Click and drag on initial load selects a block and drags it rather than panning #287
Milestone
Comments
microbit-matt-hillsdon
added a commit
to microbit-matt-hillsdon/blockly-keyboard-experimentation
that referenced
this issue
Mar 20, 2025
This can interfere with in-progress gestures causing them to act in unexpected ways. We now clear the current node when focus moves away. Avoid altering the selection in cursor.draw to prevent similar poorly timed selections. If the cursor position is lost, e.g. by click in the workspace, the arrow keys now default it if needed. Tabbing to the workspace always shows a cursor. Fixes: - Click in workspace blank space now doesn't select a block on mouse down - The mini-workspace for "if" and similar now behaves correctly again (no keyboard nav still though) - Also fixes google#287
microbit-matt-hillsdon
added a commit
to microbit-matt-hillsdon/blockly-keyboard-experimentation
that referenced
this issue
Mar 20, 2025
This can interfere with in-progress gestures causing them to act in unexpected ways. We now clear the current node when focus moves away. Avoid altering the selection in cursor.draw to prevent similar poorly timed selections. If the cursor position is lost, e.g. by click in the workspace, the arrow keys now default it if needed. Tabbing to the workspace always shows a cursor. Fixes: - Click in workspace blank space now doesn't select a block on mouse down - The mini-workspace for "if" and similar now behaves correctly again (no keyboard nav still though) - Also fixes google#287
BenHenning
added a commit
that referenced
this issue
May 2, 2025
Fixes #142 Fixes #481 This removes nearly all custom focus/blur management and instead leverages `FocusManager` as the source of truth and primary system for focusing/selecting elements. This largely continues the work in Core starting with google/blockly#8938 and ending with google/blockly#8941 in order to bring basic parity in the keyboard navigation plugin using `FocusManager`. Specifically: - This replaces all cases where explicit focus/opening logic is needed with `focusNode`/`focusTree` calls via `FocusManager`. - `Navigation` now infers state based on what's currently focused rather than tracking it separately. - All of the existing focus/blur response logic has been made redundant due to the inferred navigation mode and focus being tightly coupled with the components themselves. - The gesture monkey patch seems no longer necessary since, as far as I can tell, focus gets forced back to where it's supposed to be at specific lifecycle moments (which helps undo many of the problems caused by `WorkspaceSvg`'s `markFocused`). - The passive indicator is no longer necessary now that the `FocusManager`-driven `blocklyPassiveFocus` CSS class exists for styling. - Note that there's a lot of automatic state synchronizing that has been purposely pushed into Core to simplify the navigation pieces, particularly: - Blocks will auto-select/unselect themselves based on focus. - `Toolbox` will automatically synchronize its selected item state with its item that has active focus. - `Toolbox` and `Flyout` cooperate for automatically closing the `Flyout` at the correct times. - The new CSS classes are not yet final as the overall strategy for indicating active/passive focus isn't final. There's also additional work needed to ensure selection and active focus can be properly stylized as a combined state, but this is blocked on the completion of google/blockly#8942. - `Toolbox` navigation was moved to using its methods directly instead of its event since selection logic now needs to be paired with `focusNode`. - There are a number of changes in this PR that could probably go into Core, but these can wait for a future PR as they shouldn't be API breaking. Some of the changes include: - `Toolbox` item selection could auto-focus within `Toolbox` itself, or this could be done as part of `Toolbox`-specific navigation. - `Flyout` focus could automatically focus its first item if nothing is currently focused. - For regression testing, the following has been checked: - 'T' and 'I' do work as expected (ensuring google/blockly#8933 (review) is no longer an issue). - Verified #227, #200, #229, #222, and #287 have not regressed. google/blockly-samples#2498 shouldn't be possible anymore (even with the fix) due to ephemeral focus, and we seem to be matching parity with #326 per the `handleFocus*()` functions, though there may be cases from `handleFocusWorkspace` yet to check. The test updates were necessary because, at present, clicking does not force focus which means the tests need to rely purely on keys for navigation. I think these changes are actually an improvement since it closer aligns them with keyboard-only usage. **Current gaps** that may need resolution prior to this PR being mergeable: - There's a regression with clicking to create a variable closing the flyout that I'm still investigating. This is detailed more in google/blockly#8941. - At some point between the latest changes to this branch and the ones in google/blockly#8941 block movement regressed slightly: finishing a movement gesture now defocuses the workspace entirely (even though this was working earlier). - There are a lot of inconsistencies between the focus and selection styling before this PR and with this PR. It's unclear how much of this requires actual resolution. - Much more testing is needed to understand gaps as this changes a significant amount of state handling for the plugin.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This breaks a common mouse interaction when viewing larger projects.
Screen.Recording.2025-03-06.at.10.58.59.mov
The text was updated successfully, but these errors were encountered: