-
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
Merged
Merged
Changes from 14 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
de7c95a
Implement the logic to go to last focused editor
aleDsz d7a7b09
Add keyboard shortcut to go to last focused editor
aleDsz 6a57f70
Add tests
aleDsz e70cd0e
macOS fixes
josevalim 4882312
Update sequence mac shortcut
aleDsz 91443d8
npm run format
aleDsz ad94f59
Allow go back outside of the editor
aleDsz 65ccbfa
Apply review comments
aleDsz 56de1d3
Update lib/livebook_web/live/session_live/shortcuts_component.ex
aleDsz 9297065
Apply review comments
aleDsz 15a2f89
Focus previous/next -> Focus above/below
josevalim ef8a71a
Remove unused function
aleDsz ff655cd
Apply review comments
aleDsz cbcdd79
npm run format
aleDsz dafadcd
Apply review comments
aleDsz 38089cc
Apply review comments
aleDsz dba207a
Apply review comments
aleDsz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,123 @@ | ||
/** | ||
* Allows for recording a sequence of focused cells with the focused line | ||
* and navigate inside this stack. | ||
*/ | ||
export default class CursorHistory { | ||
/** @private */ | ||
entries = []; | ||
|
||
/** @private */ | ||
index = -1; | ||
|
||
/** | ||
* Adds a new cell to the stack. | ||
* | ||
* If the stack length is greater than the stack limit, | ||
* it will remove the oldest entries. | ||
*/ | ||
push(cellId, line, offset) { | ||
const entry = { cellId, line, offset }; | ||
|
||
if (this.isSameCell(cellId)) { | ||
this.entries[this.index] = entry; | ||
} else { | ||
if (this.entries[this.index + 1] !== undefined) { | ||
this.entries = this.entries.slice(0, this.index + 1); | ||
} | ||
|
||
this.entries.push(entry); | ||
this.index++; | ||
} | ||
|
||
if (this.entries.length > 20) { | ||
this.entries.shift(); | ||
this.index--; | ||
} | ||
} | ||
|
||
/** | ||
* Removes all matching cells with given id from the stack. | ||
*/ | ||
removeAllFromCell(cellId) { | ||
// We need to make sure the last entry from history | ||
// doesn't belong to the given cell id that we need | ||
// to remove from the entries list. | ||
let currentEntryIndex = this.index; | ||
let currentEntry = this.entries[currentEntryIndex]; | ||
|
||
while (currentEntry.cellId === cellId) { | ||
currentEntryIndex--; | ||
currentEntry = this.entries[currentEntryIndex]; | ||
} | ||
|
||
this.entries = this.entries.filter((entry) => entry.cellId !== cellId); | ||
this.index = this.entries.lastIndexOf(currentEntry); | ||
aleDsz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
/** | ||
* Checks if the current stack is available to navigate back. | ||
*/ | ||
canGoBack() { | ||
return this.canGetFromHistory(-1); | ||
} | ||
|
||
/** | ||
* Navigates back in the current stack. | ||
* | ||
* If the navigation succeeds, it will return the entry from current index. | ||
* Otherwise, returns null; | ||
*/ | ||
goBack() { | ||
return this.getFromHistory(-1); | ||
} | ||
|
||
/** | ||
* Checks if the current stack is available to navigate forward. | ||
*/ | ||
canGoForward() { | ||
return this.canGetFromHistory(1); | ||
} | ||
|
||
/** | ||
* Navigates forward in the current stack. | ||
* | ||
* If the navigation succeeds, it will return the entry from current index. | ||
* Otherwise, returns null; | ||
*/ | ||
goForward() { | ||
return this.getFromHistory(1); | ||
} | ||
|
||
/** | ||
* Gets the entry in the current stack. | ||
* | ||
* If the stack have at least one entry, it will return the entry from current index. | ||
* Otherwise, returns null; | ||
*/ | ||
getCurrent() { | ||
if (this.entries.length <= 0) return null; | ||
return this.entries[this.index]; | ||
} | ||
|
||
/** @private **/ | ||
getFromHistory(direction) { | ||
if (!this.canGetFromHistory(direction)) return null; | ||
|
||
this.index = Math.max(0, this.index + direction); | ||
return this.entries[this.index]; | ||
} | ||
|
||
/** @private **/ | ||
canGetFromHistory(direction) { | ||
if (this.entries.length === 0) return false; | ||
|
||
const index = this.index + direction; | ||
return 0 <= index && index < this.entries.length; | ||
} | ||
|
||
/** @private **/ | ||
isSameCell(cellId) { | ||
const lastEntry = this.entries[this.index]; | ||
return lastEntry !== undefined && cellId === lastEntry.cellId; | ||
} | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
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:
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).