fix: Limit LineCursor & focus syncing#9062
Merged
BenHenning merged 1 commit intoRaspberryPiFoundation:developfrom May 19, 2025
Merged
fix: Limit LineCursor & focus syncing#9062BenHenning merged 1 commit intoRaspberryPiFoundation:developfrom
LineCursor & focus syncing#9062BenHenning merged 1 commit intoRaspberryPiFoundation:developfrom
Conversation
This ensures that non-block cases don't actually change LineCursor state (which can cause some synchronization inconsistencies that then break navigation).
BenHenning
commented
May 15, 2025
Collaborator
Author
BenHenning
left a comment
BenHenning
left a comment
There was a problem hiding this comment.
Self-reviewed change.
BenHenning
requested review from
rachel-fenichel
and removed request for
RoboErikG
Collaborator
Author
|
Assigning over to you @rachel-fenichel mainly in case you have any high-level thoughts. |
rachel-fenichel
approved these changes
May 19, 2025
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
3 participants
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#526
Proposed Changes
This limits synchronizing in
LineCursorfromFocusManagerto just nodes that have a corresponding block.Reason for Changes
Limiting the synchronizing in this way ensures that navigation can't enter a bad state. The reason for why this is needed is explained in RaspberryPiFoundation/blockly-keyboard-experimentation#526 (comment).
Longer term it would maybe be ideal to do one or both of the following:
Marker's internal state in favor of always relying onFocusManager's state to cover the cases where there can be automatic focus shifting.Test Coverage
This was manually tested with the keyboard navigation plugin and verified to ensure that both RaspberryPiFoundation/blockly-keyboard-experimentation#526 and RaspberryPiFoundation/blockly-keyboard-experimentation#499 are (still) working as expected. Some basic testing was done with the core simple playground with the developer console open to ensure there weren't any expected failures.
Automated testing cases would be better addressed as part of resolving #8915.
Documentation
No new documentation is needed here.
Additional Information
This behavior is expected to only affect the keyboard navigation plugin.