-
-
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
Fixes #855 Update Slate plugins to drop data arg #856
Conversation
@@ -20,10 +21,9 @@ function changeHistory(change, type) { | |||
*/ | |||
const next = historyOfType.first(); | |||
const historyOfTypeIsValid = historyOfType.size > 1 | |||
|| next.length > 1 | |||
|| next[0].type !== 'set_selection'; | |||
|| ( next && next.length > 1 && next[0].type !== 'set_selection' ); |
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.
This changes the logic - should evaluate true if next.length > 1
OR next[0].type !== 'set_selection'
. I also prefer multiline for readability. If there was no bug here, let's just remove the changes and leave as is.
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.
Oh I see, you're pulling the next
check up - you can just prepend it:
const historyOfTypeIsValid = next && historyOfType.size > 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 changed that because there was a bug, sometimes next
is undefined
.
But I don't understand this logic : next.length > 1
OR next[0].type
.
It will throw an error if next.length === 0
.
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 pulled this logic from Slate's core - we could make it safe by changing the last line to: || (next[0] && next[0].type !== 'set_selection');
Deploy preview ready! Built with commit 3a76293 |
Deploy preview ready! Built with commit 6901088 |
Deploy preview ready! Built with commit 6901088 |
b9c7203
to
6901088
Compare
- Summary
Fixes #855 Update Slate plugins to drop data arg
- Test plan
cmd + z
andcmd + y
to undo and redo.cmd + ( b, i, s, u )
.- Description for the changelog
cmd + u
( but doesn't work either it throws an error ).