feat: Only use FocusManager for LineCursor's state#9136
Closed
BenHenning wants to merge 1 commit intoRaspberryPiFoundation:developfrom
Closed
feat: Only use FocusManager for LineCursor's state#9136BenHenning wants to merge 1 commit intoRaspberryPiFoundation:developfrom
BenHenning wants to merge 1 commit intoRaspberryPiFoundation:developfrom
Conversation
Collaborator
Author
|
NB: I validated my changes, but need to do more manual testing in both the core and plugin playgrounds before I'm fully satisfied with the fix. |
Collaborator
Author
|
Replaced by #9142. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The basics
The details
Resolves
Fixes RaspberryPiFoundation/blockly-keyboard-experimentation#584
Proposed Changes
This updates
LineCursorto only useFocusManageras its basic state forgetCurNode, essentially ignoringMarker's ability to track state.Reason for Changes
RaspberryPiFoundation/blockly-keyboard-experimentation#584 had a specific issue where the keyboard navigation plugin was explicitly focusing the workspace and updating the cursor to set the workspace as its current node when using 'W', allowing the context menu to be opened (since that's relying on the current cursor node.
There are other ways to solve this, but the easiest is to simply remove the need for
setCurNodeby entirely relying onFocusNodefor the cursor's state so that there's never an inconsistency.FocusManagerwas already correctly focusing the workspace upon tabbing, so this ensures the two are always in sync.Test Coverage
The original case for RaspberryPiFoundation/blockly-keyboard-experimentation#584 was manually tested, along with block movement (since that verifies that
Marker.getSourceBlock()is working correctly in a polymorphic way forLineCursora la the plugin'smove.tslogic, specificallygetCurrentBlock()).Automated tests still need to be added.
Documentation
No documentation changes are needed here.
Additional Information
It would be nice to fully remove the marker and cursor classes (or at least their uses), but that's not the goal of this change.