-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Fix list item reordering #437 #493
Conversation
Thanks for this!
My thoughts exactly. The visual editor is coming to the end of a rewrite, so I have no problem merging this for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that this is a temporary fix, this LGTM. My only suggestion might be to add a comment in the code explaining why this is being done, so it's not refactored away by a well-meaning contributor. That's a low risk, though, so IMO it's fine either way.
That's a good point - @Dammmien would you mind adding a comment in the new lifecycle method explaining it's purpose? Then we're good to merge. |
Done @erquhart ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
- Summary
Fix list item reordering. As explain in the issue #437.
- Test plan
In the example project:
( editor have to be on visual mode, markdown disabled )
- Description for the changelog
After component did update, check the editor content is the same that the state value.
If it isn't the same, reset the state using a mutualized
createState
method.I think it isn't the cleanest way to fix this bug.
It's better than nothing but if you have any other idea ...