feat: Edit option in block context menu #274
Conversation
Rename the main directional shortcuts to match the names of corresponding keys. Not renaming the corresponding methods on LineCursor yet as that seems like a riskier change.
If there are no inline nodes to the right of the cursor on the
'current line' of the program, hide the Edit option.
This means the menu item is shown any time the cursor is on a
block and not in the rightmost position 'on the current line';
consequently sometimes the description ("Edit block contents")
is possibly misleading, because it might not be the contents of
the _current_ block that's being navigated to, but rather that
of a sibling or parent block.
|
Marking as draft, as somehow I lost a big chunk of code in last commit! |
|
Turns out this PR is fine; it was my local branch that I had somehow got in to some werid state that was not building. |
Co-authored-by: Matt Hillsdon <[email protected]>
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
|
@BenHenning: this remains ready for review. |
BenHenning
left a comment
BenHenning
left a comment
There was a problem hiding this comment.
Thanks @cpcallen! No major concerns, just some clarification questions. PTAL.
| * This action registers itself only as a context menu item, as there | ||
| * is already a corresponding "right" shortcut item. |
There was a problem hiding this comment.
Does it perhaps make sense to consolidate this with the shortcut for workspace contexts? It seems to be doing roughly the same thing, and it feels a bit odd to have an inconsistency here as compared to other action classes (where those manage both context menu and shortcut triggers).
It wouldn't seem odd, I think, for there to be a workspace-only shortcut handled here either since the context menu callback and precondition are already assuming workspace only. I suppose the only aspects I'm unsure of is having multiple actions bound for the same key (RIGHT) or whether splitting that into multiple locations is wise.
There was a problem hiding this comment.
I think this code could be merged in to actions/ws_movement.ts, now that has been separated out in #281.
The precondition for showing the "edit" option is a bit different from the precondition for allowing the right arrow to work, though: the right arrow key is practically always enabled, wheras the edit option is only shown in some places.
(And the edit option will need to be updated once the cursor properly supports RTL, since in that case it will do the same thing as the left arrow key, rather than the right arrow key.)
Unfortunately I won't have time to do such a refactor before I leave today, and I won't be back for a couple of weeks.
There was a problem hiding this comment.
I think it's fine to merge this as-is for the benefits of the clean-up. Will approve, thanks!
BenHenning
left a comment
BenHenning
left a comment
There was a problem hiding this comment.
PR LGTM for the clean-up. Any additional cleaning can be done later.
| * This action registers itself only as a context menu item, as there | ||
| * is already a corresponding "right" shortcut item. |
There was a problem hiding this comment.
I think it's fine to merge this as-is for the benefits of the clean-up. Will approve, thanks!
|
@rachel-fenichel since Chris is out how do you want to handle merging this? |
|
I fixed some merge conflicts/bad imports and am merging now. |
Introduce an "Edit block contents (→)" block context menu option. This is shown any time the cursor is on a block and not in the rightmost position 'on the current line'; consequently the description may sometimes be slightly misleading, because it might not be the contents of the current block that's being navigated to, but rather that of a sibling or parent block. The menu option weights are adjusted to put edit between insert and delete.
Also:
LineCursorare not renamed, as that seems like a riskier change.)navigation_controller.ts.Part of #132.