-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
fixes issue #681 with pasteHtml incorrectly re-hydrating tabs. #684
Conversation
Thanks for the PR. The string vs number difference is indeed the root issue but I'd prefer if we went the other direction in converting to number so numeric comparisons can still be used. Can you update the PR to do this instead? I think just adding a custom |
@jhchen please check it now. |
Bleargh. there is a coercion problem in the toolbar module, updating that now. |
@@ -3,8 +3,8 @@ import Parchment from 'parchment'; | |||
class IdentAttributor extends Parchment.Attributor.Class { | |||
add(node, value) { | |||
if (value === '+1' || value === '-1') { |
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.
I actually think this can and should be removed... the +1/-1 is really a function of the toolbar and is handled as such in modules/toolbar.js, with the debugger attached, this method never got hit with a value that was a string === "+1" || "-1" it was always the existing indent incremented or decremented.
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.
The keyboard also uses this +1/-1 functionality
I'll merge this to avoid the back and forth but I'm probably going to change the === back as I think it will cause a regression. |
the === being set to == breaks the +1 button, i checked... the button active state detection with === is broken for the header buttons in the two examples above (pre-fix it works, post fix it doesn't). perhaps the right solution is not checking |
I guess I should have said that I will not be keeping === rather than I will be changing it back. |
Resolves #681
Current version of quill 1.0 beta demonstrating problem: http://codepen.io/jonnolen/pen/GZaaaX
Version of quill 1.0 beta built with changes demonstrating problem resolved: http://codepen.io/jonnolen/pen/XdLaGO