Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changing a group of paragraphs to a numbered list only indents the paragraphs #158

Closed
Michael-Grover opened this issue Jul 23, 2020 · 13 comments · Fixed by #252
Closed

Changing a group of paragraphs to a numbered list only indents the paragraphs #158

Michael-Grover opened this issue Jul 23, 2020 · 13 comments · Fixed by #252
Assignees

Comments

@Michael-Grover
Copy link

Bug Report 🐛

If the user highlights a group of paragraphs and clicks the numbered or bulleted list icons, the paragraphs are indented, but not given a bullet or a number.

image

Expected Behavior

Highlighting a group of paragraphs and clicking the numbered or bulleted list icons should turn the highlighted text into a numbered or bulleted list

video:
https://www.loom.com/share/64f31fb19dd346ab971859ff2a28a0dc

@gatij10
Copy link
Contributor

gatij10 commented Jan 5, 2021

Is this issue still open? May I solve the bug ?

@Michael-Grover
Copy link
Author

@gatij10 I checked and it looks like this bug still exists. Thanks for volunteering, I've assigned this issue to you.

@K-Kumar-01
Copy link
Contributor

@Michael-Grover
The issue seemed inactive so I did some experiments on it and deduced the following things from it:

  1. Selection of multiple paragraphs from top to bottom causes no bug.
  2. Selection of multiple paragraphs from bottom to top causes bugs.

Selected from the top
Screenshot from 2021-02-20 19-25-04
Working as expected.
Screenshot from 2021-02-20 19-25-28
Selecting from bottom
Screenshot from 2021-02-20 19-25-55
The bug and console.logs() depicting that.
Screenshot from 2021-02-20 19-26-21

It seems from the above experiments that the 2nd elements of focus and anchor are not being properly understood by the selection, which may be the cause for the above bug.

@dselman
Copy link
Contributor

dselman commented Feb 20, 2021

@K-Kumar-01
Copy link
Contributor

Nice analysis. I suggest you look at the code inside isList:

https://github.com/accordproject/web-components/blob/master/packages/ui-markdown-editor/src/utilities/toolbarHelpers.js

@dselman
The console.logs() were made by changing the code inside the above-mentioned link.
As far as I can understand, this seems to be an issue of slate itself, though I am not sure about this.

@gatij10
Copy link
Contributor

gatij10 commented Feb 20, 2021

Nice analysis. I suggest you look at the code inside isList:
https://github.com/accordproject/web-components/blob/master/packages/ui-markdown-editor/src/utilities/toolbarHelpers.js

@dselman
The console.logs() were made by changing the code inside the above-mentioned link.
As far as I can understand, this seems to be an issue of slate itself, though I am not sure about this.

I observed that on selecting from bottom, initially the offset values of anchor and focus are swapped.
Screenshot 2021-02-20 202333

@K-Kumar-01
Copy link
Contributor

K-Kumar-01 commented Feb 20, 2021

@dselman @Michael-Grover
I have opened a pull request. Let me know if there are any changes required.

@jolanglinais
Copy link
Member

@K-Kumar-01 and @gatij10 I think it makes sense to resolve which PR you two want to use.

@K-Kumar-01
Copy link
Contributor

@gatij10
There was already a PR made by me. So I request you to please close your PR

@gatij10
Copy link
Contributor

gatij10 commented Feb 23, 2021

@K-Kumar-01 Your PR failed in the netlify build, that's why I created a new PR.

@K-Kumar-01
Copy link
Contributor

@gatij10
That is fixable. I would request you to please close down your PR.

@gatij10
Copy link
Contributor

gatij10 commented Feb 23, 2021

okay

@K-Kumar-01
Copy link
Contributor

okay

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment