## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes https://github.com/google/blockly-keyboard-experimentation/issues/499
### Proposed Changes
This ensures that non-blocks which hold active focus correctly update `LineCursor`'s internal state.
### Reason for Changes
This is outright a correction in how `LineCursor` has worked up until now, and is now possible after several recent changes (most notably #9004). #9004 updated selection to be more explicitly generic (and based on `IFocusableNode`) which means `LineCursor` should also properly support more than just blocks when synchronizing with focus (in place of selection), particularly since lots of non-block things can be focusable.
What's interesting is that this change isn't strictly necessary, even if it is a reasonable correction and improvement in the robustness of `LineCursor`. Essentially everywhere navigation is handled results in a call to `setCurNode` which correctly sets the cursor's internal state (with no specific correction from focus since only blocks were checked and we already ensure that selecting a block correctly focuses it).
### Test Coverage
It would be nice to add test coverage specifically for the cursor cases, but it seems largely unnecessary since:
1. The main failure cases are test-specific (as mentioned above).
2. These flows are better left tested as part of broader accessibility testing (per #8915).
This has been tested with a cursory playthrough of some basic scenarios (block movement, insertion, deletion, copy & paste, context menus, and interacting with fields).
### Documentation
No new documentation should be needed here.
### Additional Information
This is expected to only affect keyboard navigation plugin behaviors, particularly plugin tests.
It may be worth updating `LineCursor` to completely reflect current focus state rather than holding an internal variable. This, in turn, may end up simplifying solving issues like #8793 (but not necessarily).
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes#9043
Fixes https://github.com/google/blockly-samples/issues/2512
### Proposed Changes
This replaces using BlockSvg's own ID for focus management since that's not guaranteed to be unique across all workspaces on the page.
### Reason for Changes
Both https://github.com/google/blockly-samples/issues/2512 covers the user-facing issue in more detail, but from a technical perspective it's possible for blocks to share IDs across workspaces. One easy demonstration of this is the flyout: the first block created from the flyout to the main workspace will share an ID. The workspace minimap plugin just makes the underlying problem more obvious.
The reason this introduces a breakage is due to the inherent ordering that `FocusManager` uses when trying to find a matching tree for a given DOM element that has received focus. These trees are iterated in the order of their registration, so it's quite possible for some cases (like main workspace vs. flyout) to resolve such that the behavior looks correct to users, vs. others (such as the workspace minimap) not behaving as expected.
Guaranteeing ID uniqueness across all workspaces fixes the problem entirely.
### Test Coverage
This has been manually tested in core Blockly's simple test playground and in Blockly samples' workspace minimap plugin test environment (linked against this change). See the new behavior for the minimap plugin:
[Screen recording 2025-05-13 4.31.31 PM.webm](https://github.com/user-attachments/assets/d2ec3621-6e86-4932-ae85-333b0e7015e1)
Note that this is a regression to v11 behavior in that the blocks in the minimap now show as selected.
This has been verified as working with the latest version of the keyboard navigation plugin (tip-of-tree). Keyboard-based block operations and movement seem to work as expected.
For automated testing this is expected to largely be covered by future tests added as part of resolving #8915.
### Documentation
No public documentation changes should be needed, though `IFocusableNode`'s documentation has been refined to be clearer on the uniqueness property for focusable element IDs.
### Additional Information
There's a separate open design question here about whether `BlockSvg`'s descendants should use the new focus ID vs. the block ID. Here is what I consider to be the trade-off analysis in this decision:
| | Pros | Cons |
|------------------------|-------------------------------------------------|------------------------------------------------------------------------------|
| Use `BlockSvg.id` | Can use fast `WorkspaceSvg.getBlockById`. | `WorkspaceSvg.lookUpFocusableNode` now uses 2 different IDs. |
| Use `BlockSvg.focusId` | Consistency in IDs use for block-related focus. | Requires more expensive block look-up in `WorkspaceSvg.lookUpFocusableNode`. |
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes#9027
### Proposed Changes
Ensure that a block being dragged is properly focused mid-drag.
### Reason for Changes
Focus seems to be lost due to the block being moved to the drag layer, so re-focusing the block ensures that it remains both actively focused and selected while dragging.
The regression was likely caused when block selection was moved to be fully synced based on active focus.
### Test Coverage
This has been manually verified in Core's simple playground. At the time of the PR being opened, this couldn't be tested in the test environment for the experimental keyboard navigation plugin since there's a navigation connection issue there that needs to be resolved to test movement.
It would be helpful to add a new test case for the underlying problem (i.e. ensuring that the block holds focus mid-drag) as part of resolving #8915.
### Documentation
No new documentation should need to be added.
### Additional Information
This was found during the development of https://github.com/google/blockly-keyboard-experimentation/pull/511.
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes https://github.com/google/blockly-keyboard-experimentation/issues/515
### Proposed Changes
This adds `canBeFocused()` checks to all the places that could currently cause problems if a node were to return `false`.
### Reason for Changes
This can't introduce a problem in current Core and, in fact, most of these classes can never return `false` even through subclasses. However, this adds better robustness and fixes the underlying issue by ensuring that `getFocusableElement()` isn't called for a node that has indicated it cannot be focused.
### Test Coverage
I've manually tested this through the keyboard navigation plugin. However, there are clearly additional tests that would be nice to add both for the traverser and for `WorkspaceSvg`, both likely as part of resolving #8915.
### Documentation
No new documentation changes should be needed here.
### Additional Information
This is fixing theoretical issues in Core, but a real issue tracked by the keyboard navigation plugin repository.
* feat!: Make bubbles, comments, and icons focusable
* feat!: Make ISelectable and ICopyable focusable.
* feat: Consolidate selection calls.
Now everything is based on focus with selection only being used as a
proxy.
* feat: Invert responsibility for setSelected().
Now setSelected() is only for quasi-external use.
* feat: Push up shadow check to getters.
Needed new common-level helper.
* chore: Lint fixes.
* feat!: Allow IFocusableNode to disable focus.
* chore: post-merge lint fixes
* fix: Fix tests + text bubble focusing.
This fixed then regressed a circular dependency causing the node and
advanced compilation steps to fail. This investigation is ongoing.
* fix: Clean up & fix imports.
This ensures the node and advanced compilation test steps now pass.
* fix: Lint fixes + revert commented out logic.
* chore: Remove unnecessary cast.
Addresses reviewer comment.
* fix: Some issues and a bunch of clean-ups.
This addresses a bunch of review comments, and fixes selecting workspace
comments.
* chore: Lint fix.
* fix: Remove unnecessary shadow consideration.
* chore: Revert import.
* chore: Some doc updates & added a warn statement.
* chore(deps): bump @microsoft/api-documenter from 7.26.7 to 7.26.26
Bumps [@microsoft/api-documenter](https://github.com/microsoft/rushstack/tree/HEAD/apps/api-documenter) from 7.26.7 to 7.26.26.
- [Changelog](https://github.com/microsoft/rushstack/blob/main/apps/api-documenter/CHANGELOG.md)
- [Commits](https://github.com/microsoft/rushstack/commits/@microsoft/api-documenter_v7.26.26/apps/api-documenter)
---
updated-dependencies:
- dependency-name: "@microsoft/api-documenter"
dependency-version: 7.26.26
dependency-type: direct:development
update-type: version-update:semver-patch
...
Signed-off-by: dependabot[bot] <support@github.com>
* fix(deps): Update patch for api-documenter
The patch was failing to apply because the contents of the file
had moved too much. Fixed by manually applying changes to the
file in question then running
node_modules/.bin/patch-package @microsoft/api-documenter
to recreate the patch, then diffing old patch vs. new to verify
that contents of patch were unchanged except for chunk positions,
then deleting old patch.
---------
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Christopher Allen <cpcallen+git@google.com>
* feat: Add interfaces for keyboard navigation.
* feat: Add the Navigator.
* feat: Make core types conform to INavigable.
* feat: Require FlyoutItems elements to be INavigable.
* feat: Add navigation policies for built-in types.
* refactor: Convert Marker and LineCursor to operate on INavigables instead of ASTNodes.
* chore: Delete dead code in ASTNode.
* fix: Fix the tests.
* chore: Assuage the linter.
* fix: Fix advanced build/tests.
* chore: Restore ASTNode tests.
* refactor: Move isNavigable() validation into Navigator.
* refactor: Exercise navigation instead of ASTNode.
* chore: Rename astnode_test.js to navigation_test.js.
* chore: Enable the navigation tests.
* fix: Fix bug when retrieving the first child of an empty workspace.
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes#8994
### Proposed Changes
This removes an error that was previously thrown by `FocusManager` when attempting to focus an invalid node (such as one that's been removed from its parent).
### Reason for Changes
https://github.com/google/blockly/issues/8994#issuecomment-2855447539 goes into more detail. While this error did cover legitimately wrong cases to try and focus things (and helped to catch some real problems), fixing this 'properly' may become a leaky boat problem where we have to track down every possible asynchronous scenario that could produce such a case. One class of this is ephemeral focus which had robustness improvements itself in #8981 that, by effect, caused this issue in the first place. Holistically fixing this with enforced API contracts alone isn't simple due to the nature of how these components interact.
This change ensures that there's a sane default to fall back on if an invalid node is passed in. Note that `FocusManager` was designed specifically to disallow defocusing a node (since fallbacks can get messy and introduce unpredictable user experiences), and this sort of allows that now. However, this seems like a reasonable approach as it defaults to the behavior when focusing a tree explicitly which allows the tree to fallback to a more suitable default (such as the first item to select in the toolbox for that particular tree). In many cases this will default back to the tree's root node (such as the workspace root group) since sometimes the removed node is still the "last focused node" of the tree (and is considered valid for the purpose of determining a fallback; tree implementations could further specialize by checking whether that node is still valid).
### Test Coverage
Some new tests were added to cover this case, but more may be useful to add as part of #8910.
### Documentation
No documentation needs to be added or updated as part of this (beyond code documentation changes).
### Additional Information
This original issue was found by @RoboErikG when testing #8995. I also verified this against the keyboard navigation plugin repository.
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes#8952Fixes#8950Fixes#8971
Fixes a bunch of other keyboard + mouse synchronization issues found during the testing discussed in https://github.com/google/blockly-keyboard-experimentation/pull/482#issuecomment-2846341307.
### Proposed Changes
This introduces a number of changes to:
- Ensure that gestures which change selection state also change focus.
- Ensure that ephemeral focus is more robust against certain classes of failures.
### Reason for Changes
There are some ephemeral focus issues that can come up with certain actions (like clicking or dragging) don't properly change focus. Beyond that, some users will likely mix clicking and keyboard navigation, so it's essential that focus and selection state stay in sync when switching between these two types of navigation modalities.
Other changes:
- Drop-down div was actually incorrectly releasing ephemeral focus before animated closes finish which could reset focus afterwards, breaking focus state.
- Both drop-down and widget divs have been updated to only return focus _after_ marking the workspace as focused since the last focused node should always be the thing returned.
- In a number of gesture cases selection has been removed. This is due to `BlockSvg` self-managing its selection state based on focus, so focusing is sufficient. I've also centralized some of the focusing calls (such as putting one in `bringBlockToFront` to ensure focusing happens after potential DOM changes).
- Similarly, `BlockSvg`'s own `bringToFront` has been updated to automatically restore focus after the operation completes. Since `bringToFront` can always result in focus loss, this provides robustness to ensure focus is restored.
- Block pasting now ensures that focus is properly set, however a delay is needed due to additional rendering changes that need to complete (I didn't dig deeply into the _why_ of this).
- Dragging has been updated to always focus the moved block if it's not in the process of being deleted.
- There was some selection resetting logic removed from gesture's `doWorkspaceClick` function. As far as I can tell, this won't be needed anymore since blocks self-regulate their selection state now.
- `FocusManager` has a new extra check for ephemeral focus to catch a specific class of failure where the browser takes away focus immediately after it's returned. This can happen if there are delay timing situations (like animations) which result in a focused node being deleted (which then results in the document body receiving focus). The robustness check is possibly not needed, but it help discover the animation issue with drop-down div and shows some promise for helping to fix the variables-closing-flyout problem.
Some caveats:
- Some undo/redo steps can still result in focus being lost. This may introduce some regressions for selection state, and definitely introduces some annoyances with keyboard navigation. More work will be needed to understand how to better redirect focus (and to what) in cases when blocks disappear.
- There are many other places where focus is being forced or selection state being overwritten that could, in theory, cause issues with focus state. These may need to be fixed in the future.
- There's a lot of redundancy with `focusNode` being called more than it needs to be. `FocusManager` does avoid calling `focus()` more than once for the same node, but it's possible for focus state to switch between multiple nodes or elements even for a single gesture (for example, due to bringing the block to the front causing a DOM refresh). While the eventual consistency nature of the manager means this isn't a real problem, it may cause problems with screen reader output in the future and warrant another pass at reducing `focusNode` calls (particularly for gestures and the click event pipeline).
### Test Coverage
This PR is largely relying on existing tests for regression verification, though no new tests have been added for the specific regression cases.
#8910 is tracking improving `FocusManager` tests which could include some cases for the new ephemeral focus improvements.
#8915 is tracking general accessibility testing which could include adding tests for these specific regression cases.
### Documentation
No new documentation is expected to be needed here.
### Additional Information
These changes originate from both #8875 and from a branch @rachel-fenichel created to investigate some of the failures this PR addresses. These changes have also been verified against both Core's playground and the keyboard navigation plugin's test environment.
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes#8976
### Proposed Changes
Only auto-close the flyout if focus is being lost to a known tree.
### Reason for Changes
I noticed from testing that the system does attempt to restore focus back to the flyout after creating a variable but the auto-closing logic was kicking in due to focus being lost with the variable creation prompt open. Even though an attempt was made to restore focus, this doesn't automatically reopen the flyout (since it is primarily governed by the toolbox selection state).
One alternative might be to try and save the previously selected toolbox category and restore it, but that's tricky. This seems simpler, and also seems to largely maintain parity with pre-focus manager Blockly. Clicking outside of the toolbox with the flyout open only closes it if the click is within the toolbox itself or within the workspace.
### Test Coverage
No new tests were added. However, it may be worth considering this specific case for future tests added with #8915.
### Documentation
No new documentation seems necessary here.
### Additional Information
None.
* feat(WorkspaceSvg): Ignore gestures during keyboard moves
Modify WorkspaceSvg.prototype.getGesture to return null when
there is a keyboard-initiated move in progress.
* chore(WorkspaceSvg): Add TODOs to remove .keyboardMoveInProgress
* docs: Make JSDoc @deprecated usage more consistent
* refactor(VariableMap)!: Refresh toolbox when map modified
Delete the following methods from WorkspaceSvg:
- renameVariableById
- deleteVariableById
- createVariable
Modify the following methods on VariableMap to call
this.workspace.refreshToolboxSelection() if this.workspace
is a WorkspaceSvg, replicating the behaviour of the
aforementioned deleted methods and additionally ensuring
that that method is called following any change to the
variable map:
- renameVariable
- changeVariableType
- renameVariableAndUses
- createVariable
- addVariable
- deleteVariable
BREAKING CHANGE:
This change ensures that the toolbox will be refreshed regardless
of what route the VaribleMap was updated, rather than only being
refreshed when it is updated via calls to methods on WorkspaceSvg.
Overall this is much more likely to fix a bug (where the toolbox
wasn't being refreshed when it should have been) than cause one
(by refreshing the toolbox when it shouldn't be), but this is
still a behaviour change which could _conceivably_ result an
unexpected regression.
* refactor(VariableMap): Remove calls to deprecated getVariableUsesById
Also refactor to use named imports core/variables.ts methods.
* refactor(Workspace): Use named imports for core/variables.ts methods
* refactor(FieldVariable): Remove call to deprecated getVariablesOfType
* refactor(variables): Remove calls to deprecated methods
* refactor(variables_dynamic): Remove call to deprecated getAllVariables
* refactor(xml): Remove calls to deprecated createVariable
* refactor(Events.VarCreate): Remove calls to deprecated methods
* refactor(Events.VarDelete): Remove calls to deprecated methods
* refactor(Events.VarRename): Remove calls to deprecated methods
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
This fixes the ongoing CI failure for the conventional auto-labeling.
### Proposed Changes
This fixes the permissions in a way that should work. It may be the case that 'issues' only needs to be 'read' but I'm basically just copying what's done in https://github.com/GoogleCloudPlatform/developer-journey-app/blob/main/.github/workflows/auto-label.yml since it's working for them.
### Reason for Changes
We want this workflow working--it's preferable to avoid getting used to a failing CI workflow (ideally every PR has zero CI failures).
As for the specific changes, note that the check will still fail in this PR. Similar to #8811, it's not expected that the CI workflow will pass in this PR until the change is checked in since the workflow uses 'pull_request_target'. While I haven't verified this change directly, I'm fairly confident it will work given the project linked above and our successes with fixing the auto assigner workflow.
Finally, the 'contents: read' bit is unnecessary since that's the default permission for `GITHUB_TOKEN` per https://docs.github.com/en/actions/security-for-github-actions/security-guides/automatic-token-authentication#permissions-for-the-github_token.
Edit: It seems that the check actually is passing with these changes--that's a bit surprising to me.
### Test Coverage
N/A
### Documentation
N/A
### Additional Information
None.
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes#8940Fixes#8954Fixes#8955
### Proposed Changes
This updates `LineCursor` to use `FocusManager` rather than selection (principally) as the source of truth.
### Reason for Changes
Ensuring that keyboard navigation works correctly with eventual screen reader support requires ensuring that ever navigated component is focused, and this is primarily what `FocusManager` has been designed to do. Since these nodes are already focused, `FocusManager` can be used as the primary source of truth for determining where the user currently has navigated, and where to go next.
Previously, `LineCursor` relied on selection for this purpose, but selection is now automatically updated (for blocks) using focus-controlled `focus` and `blur` callbacks. Note that the cursor will still fall back to synchronizing with selection state, though this will be removed once the remaining work to eliminate `MarkerSvg` has concluded (which requires further consideration on the keyboard navigation side viz-a-viz styling and CSS decisions) and once mouse clicks are synchronized with focus management.
Note that the changes in this PR are closely tied to https://github.com/google/blockly-keyboard-experimentation/pull/482 as both are necessary in order for the keyboard navigation plugin to correctly work with `FocusManager`.
Some other noteworthy changes:
- Some special handling exists for flyouts to handle navigating across stacks (per the current cursor design).
- `FocusableTreeTraverser` is needed by the keyboard navigation plugin (in https://github.com/google/blockly-keyboard-experimentation/pull/482) so it's now being exported.
- `FocusManager` had one bug that's now patched and tested in this PR: it didn't handle the case of the browser completely forcing focus loss. It would continue to maintain active focus even though no tracked elements now hold focus. One such case is the element being deleted, but there are other cases where this can happen (such as with dialog prompts).
- `FocusManager` had some issues from #8909 wherein it would overeagerly call tree focus callbacks and slightly mismanage the passive node. Since tests haven't yet been added for these lifecycle callbacks, these cases weren't originally caught (per #8910).
- `FocusManager` was updated to move the tracked manager into a static function so that it can be replaced in tests. This was done to facilitate changes to setup_teardown.js to ensure that a unique `FocusManager` exists _per-test_. It's possible for DOM focus state to still bleed across tests, but `FocusManager` largely guarantees eventual consistency. This change prevents a class of focus errors from being possible when running tests.
- A number of cursor tests needed to be updated to ensure that a connections are properly rendered (as this is a requirement for focusable nodes, and cursor is now focusing nodes). One test for output connections was changed to use an input connection, instead, since output connections can no longer be navigated to (and aren't rendered, thus are not focusable). It's possible this will need to be changed in the future if we decide to reintroduce support for output connections in cursor, but it seems like a reasonable stopgap. Huge thanks to @rachel-fenichel for helping investigate and providing an alternative for the output connection test.
**Current gaps** to be fixed after this PR is merged:
- The flyout automatically closes when creating a variable with with keyboard or mouse (I think this is only for the keyboard navigation plugin). I believe this is a regression from previous behavior due to how the navigation plugin is managing state. It would know the flyout should be open and thus ensure it stays open even when things like dialog prompts try to close it with a blur event. However, the new implementation in https://github.com/google/blockly-keyboard-experimentation/pull/482 complicates this since state is now inferred from `FocusManager`, and the flyout _losing_ focus will force it closed. There was a fix introduced in this PR to fix it for keyboard navigation, but fails for clicks because the flyout never receives focus when the create variable button is clicked. It also caused the advanced compilation tests to fail due to a subtle circular dependency from importing `WorkspaceSvg` directly rather than its type.
- The flyout, while it stays open, does not automatically update past the first variable being created without closing and reopening it. I'm actually not at all sure why this particular behavior has regressed.
### Test Coverage
No new non-`FocusManager` tests have been added. It's certainly possible to add unit tests for the focusable configurations being introduced in this PR, but it may not be highly beneficial. It's largely assumed that the individual implementations should work due to a highly tested FocusManager, and it may be the case that the interactions of the components working together is far more important to verify (that is, the end user flows). The latter is planned to be tackled as part of #8915.
Some new `FocusManager` tests were added, but more are still needed and this is tracked as part of #8910.
### Documentation
No new documentation should be needed for these changes.
### Additional Information
This includes changes that have been pulled from #8875.
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes#8930
Fixes part of #8771
### Proposed Changes
This PR introduces support for connections to be focusable (and thus navigable with keyboard navigation when paired with downstream changes to `LineCursor` and the keyboard navigation plugin). This is a largely isolated change in how it fundamentally works:
- `RenderedConnection` has been updated to be an `IFocusableNode` using a new unique ID maintained by `Connection` and automatically enabling/disabling the connection highlight based on whether it's focused (per keyboard navigation).
- The way that rendering works here has changed: rather than recreating the connection's highlight SVG each time, it's only created once and updated thereafter to ensure that it correctly fits block resizes or movements. Visibility of the highlight is controlled entirely through display visibility and can now be done synchronously (which was a requirement for focusability as only displayed elements can be focused).
- This employs the same type of ID schema strategy as fields in #8923.
### Reason for Changes
This is part of an ongoing effort to ensure key components of Blockly are focusable so that they can be keyboard-navigable (with other needed changes yet both in Core Blockly and the keyboard navigation plugin).
### Test Coverage
No new tests have been added. It's certainly possible to add unit tests for the focusable configurations being introduced in this PR, but it may not be highly beneficial. It's largely assumed that the individual implementations should work due to a highly tested FocusManager, and it may be the case that the interactions of the components working together is far more important to verify (that is, the end user flows). The latter is planned to be tackled as part of #8915.
### Documentation
No documentation changes should be needed here.
### Additional Information
This includes changes that have been pulled from #8875.
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes#8922Fixes#8929
Fixes part of #8771
### Proposed Changes
This PR introduces support for fields to be focusable (and thus navigable with keyboard navigation when paired with downstream changes to `LineCursor` and the keyboard navigation plugin). This is a largely isolated change in how it fundamentally works:
- `Field` was updated to become an `IFocusableNode`. Note that it uses a specific string-based ID schema in order to ensure that it can be properly linked back to its unique block (which helps make the search for the field in `WorkspaceSvg` a bit more efficient). This could be done with a globally unique ID, instead, but all fields would need to be searched vs. just those for the field's parent block.
- The drop-down and widget divs have been updated to manage ephemeral focus with `FocusManager` when they're open for non-system dialogs (ephemeral focus isn't needed for system dialogs/prompts since those already take/restore focus in a native way that `FocusManager` will respond to--this may require future work, however, if the restoration causes unexpected behavior for users). This approach was done due to a suggestion from @maribethb as the alternative would be a more complicated breaking change (forcing `Field` subclasses to properly manage ephemeral focus). It may still be the case that certain cases will need to do so, but widget and drop-down divs seem to address the majority of possibilities.
**Important**: `Input`s are not explicitly being supported here. As far as I can tell, we can't run into a case where `LineCursor` tries to set an input node, though perhaps I simply haven't come across this case. Supporting `Fields` and `Connections` (per #8928) seems to cover the main needed cases, though making `Input`s focusable may be a future requirement.
### Reason for Changes
This is part of an ongoing effort to ensure key components of Blockly are focusable so that they can be keyboard-navigable (with other needed changes yet both in Core Blockly and the keyboard navigation plugin).
Note that #8929 isn't broadly addressed since making widget & drop down divs manage ephemeral focus directly addresses a large class of cases. Additional cases may arise where a plugin or Blockly integration may require additional effort to make keyboard navigation work for their field--this may be best addressed with documentation and guidance.
### Test Coverage
No new tests have been added. It's certainly possible to add unit tests for the focusable configurations being introduced in this PR, but it may not be highly beneficial. It's largely assumed that the individual implementations should work due to a highly tested FocusManager, and it may be the case that the interactions of the components working together is far more important to verify (that is, the end user flows). The latter is planned to be tackled as part of #8915.
### Documentation
No new documentation is planned, however it may be prudent to update the field documentation in the future to explain how to utilize ephemeral focus when specifically building compatibility for keyboard navigation.
### Additional Information
This includes changes that have been pulled from #8875.
_Note: This is a roll forward of #8920 that was reverted in #8933. See Additional Information below._
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes#8918Fixes#8919
Fixes part of #8943
Fixes part of #8771
### Proposed Changes
This updates several classes in order to make toolboxes and flyouts focusable:
- `IFlyout` is now an `IFocusableTree` with corresponding implementations in `FlyoutBase`.
- `IToolbox` is now an `IFocusableTree` with corresponding implementations in `Toolbox`.
- `IToolboxItem` is now an `IFocusableNode` with corresponding implementations in `ToolboxItem`.
- As the primary toolbox items, `ToolboxCategory` and `ToolboxSeparator` were updated to have -1 tab indexes and defined IDs to help `ToolboxItem` fulfill its contracted for `IFocusableNode.getFocusableElement`.
- `FlyoutButton` is now an `IFocusableNode` (with corresponding ID generation, tab index setting, and ID matching for retrieval in `WorkspaceSvg`).
Each of these two new focusable trees have specific noteworthy behaviors behaviors:
- `Toolbox` will automatically indicate that its first item should be focused (if one is present), even overriding the ability to focus the toolbox's root (however there are some cases where that can still happen).
- `Toolbox` will automatically synchronize its selection state with its item nodes being focused.
- `FlyoutBase`, now being a focusable tree, has had a tab index of 0 added. Normally a tab index of -1 is all that's needed, but the keyboard navigation plugin specifically uses 0 for flyout so that the flyout is tabbable. This is a **new** tab stop being introduced.
- `FlyoutBase` holds a workspace (for rendering blocks) and, since `WorkspaceSvg` is already set up to be a focusable tree, it's represented as a subtree to `FlyoutBase`. This does introduce some wonky behaviors: the flyout's root will have passive focus while its contents have active focus. This could be manually disabled with some CSS if it ends up being a confusing user experience.
- Both `FlyoutBase` and `WorkspaceSvg` have built-in behaviors for detecting when a user tries navigating away from an open flyout to ensure that the flyout is closed when it's supposed to be. That is, the flyout is auto-hideable and a non-flyout, non-toolbox node has then been focused. This matches parity with the `T`/`Esc` flows supported in the keyboard navigation plugin playground.
One other thing to note: `Toolbox` had a few tests to update that were trying to reinit a toolbox without first disposing of it (which was caught by one of `FocusManager`'s state guardrails).
This only addresses part of #8943: it adds support for `FlyoutButton` which covers both buttons and labels. However, a longer-term solution may be to change `FlyoutItem` itself to force using an `IFocusableNode` as its element.
### Reason for Changes
This is part of an ongoing effort to ensure key components of Blockly are focusable so that they can be keyboard-navigable (with other needed changes yet both in Core Blockly and the keyboard navigation plugin).
### Test Coverage
No new tests have been added. It's certainly possible to add unit tests for the focusable configurations being introduced in this PR, but it may not be highly beneficial. It's largely assumed that the individual implementations should work due to a highly tested FocusManager, and it may be the case that the interactions of the components working together is far more important to verify (that is, the end user flows). The latter is planned to be tackled as part of #8915.
### Documentation
No documentation changes should be needed here.
### Additional Information
This includes changes that have been pulled from #8875.
This was originally merged in #8916 but was reverted in #8933 due to https://github.com/google/blockly-keyboard-experimentation/issues/481. Note that this does contain a number of differences from the original PR (namely, changes in `WorkspaceSvg` and `FlyoutButton` in order to make `FlyoutButton`s focusable). Otherwise, this has the same caveats as those noted in #8938 with regards to the experimental keyboard navigation plugin.
_Note: This is a roll forward of #8916 that was reverted in #8933. See Additional Information below._
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes#8913Fixes#8914
Fixes part of #8771
### Proposed Changes
This updates `WorkspaceSvg` and `BlockSvg` to be focusable, that is, it makes the workspace a `IFocusableTree` and blocks `IFocusableNode`s.
Some important details:
- While this introduces focusable tree support for `Workspace` it doesn't include two other components that are obviously needed by the keyboard navigation plugin's playground: fields and connections. These will be introduced in subsequent PRs.
- Blocks are set up to automatically synchronize their selection state with their focus state. This will eventually help to replace `LineCursor`'s responsibility for managing selection state itself.
- The tabindex property for the workspace and its ARIA label have been moved down to the `.blocklyWorkspace` element itself rather than its wrapper. This helps address some tab stop issues that are already addressed in the plugin (via monkey patches), but also to ensure that the workspace's main SVG group interacts correctly with `FocusManager`.
- `WorkspaceSvg` is being initially set up to default to its first top block when being focused for the first time. This is to match parity with the keyboard navigation plugin, however the latter also has functionality for defaulting to a position when no blocks are present. It's not clear how to actually support this under the new focus-based system (without adding an ephemeral element on which to focus), or if it's even necessary (since the workspace root can hold focus).
- `css.ts` was updated to remove `blocklyActiveFocus` and `blocklyPassiveFocus` since these have unintended highlighting consequences that aren't actually desirable yet. Instead, the exact styling for active/passive focus will be iterated in the keyboard navigation plugin project and moved to Core once finalized.
### Reason for Changes
This is part of an ongoing effort to ensure key components of Blockly are focusable so that they can be keyboard-navigable (with other needed changes yet both in Core Blockly and the keyboard navigation plugin).
### Test Coverage
No new tests have been added. It's certainly possible to add unit tests for the focusable configurations being introduced in this PR, but it may not be highly beneficial. It's largely assumed that the individual implementations should work due to a highly tested FocusManager, and it may be the case that the interactions of the components working together is far more important to verify (that is, the end user flows). The latter is planned to be tackled as part of #8915.
### Documentation
No documentation changes should be needed here.
### Additional Information
This includes changes that have been pulled from #8875.
This was originally merged in #8916 but was reverted in #8933 due to https://github.com/google/blockly-keyboard-experimentation/issues/481. This actually contains no differences from the original PR except for `css.ts` which are documented above. It does employ a new merge strategy: all of the necessary PRs to move both Core and the plugin over to using `FocusManager` will be staged and merged in quick succession as ensuring the plugin works for each constituent change (vs. the final one) is quite complex. Thus, this PR *does* break the plugin, and won't be merged until its subsequent PRs are approved and also ready for merging.
Edit: See https://github.com/google/blockly/pull/8938#issuecomment-2843589525 for why this actually is being merged a bit sooner than originally planned. Keeping the original reasoning above for context.
