-
Notifications
You must be signed in to change notification settings - Fork 434
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
Implements the go back and forward keyboard shortcuts #2789
Conversation
Uffizzi Preview |
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.
It looks good to me. Feel free to ship it once Jonatan approves. :)
assets/js/hooks/cell.js
Outdated
// We defer the check to happen after all focus/click events have | ||
// been processed, in case the state changes as a result | ||
setTimeout(() => { | ||
if (this.isFocused && viewUpdate.selectionSet) { |
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.
We can check if the selection changed:
!update.state.selection.eq(update.startState.selection)
I would make the event more specific liveEditor.onSelectionChange
and have that check there.
The only difference is that when the user goes into insert mode in existing cell, the selection doesn't change, so we can call sendCursorHistory
on focus separately.
Once we do that, I don't think we need the setTimeout
here (since it's only specific to the focus case?). wdyt?
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'm not sure if I understood about the focus change, because it makes the pubsub event to be dispatched twice
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.
My idea is that in onFocus we do this:
if (!this.isFocused || !this.insertMode) {
this.currentEditor().blur();
} else {
this.sendCursorHistory();
}
This way we send the cursor history when the editor is focused and when the cursor changes location within the editor (focusing alone does not change the cursor location).
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.
🚢
No description provided.