Use CSS for selection state #8942
Labels
issue: feature request
Describes a new feature and why it should be added
Comments
BenHenning
added a commit
to google/blockly-keyboard-experimentation
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.
|
See https://github.com/google/blockly-keyboard-experimentation/pull/511/files#r2085299185 specifically since there's a specific case around connection displaying that can probably be solved as part of this issue to simplify some of the CSS that needs to move over from the keyboard navigation plugin. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Check for duplicates
Problem
Selection is currently done using a custom SVG filter:
blockly/core/renderers/zelos/constants.ts
Line 675 in ada01da
This makes customization difficult especially when combined with other possible states (such as active focus).
Request
It would be preferable for selection to be styled using pure CSS such that it can be combined with other possible states.
Alternatives considered
No response
Additional context
This has been explored in #8875 and it's not obvious exactly how to solve it. Some possibilities that have been explored as well as discussed with @rachel-fenichel:
blocklySelectedcan then apply a stroke property and control visibility of this special highlight element. It's still tricky, however, as this element requires custom lifecycle management and it can't be a child of the block'sgelement (which meansblocklySelectedhas to get applied to a different element tree than it does today).The text was updated successfully, but these errors were encountered: