* fix: Fix bug that caused inadvertent scrolling when the `WidgetDiv` was shown.
* chore: Add test to verify that displaying the context menu does not scroll the page.
* chore: Clarify comments.
* fix: Remove errant `.only`.
* chore: Add test to verify that actively focusing a node does not scroll the page.
* fix: Remove inadvertent `.only`.
* Revert "fix: Auto-close widget divs on lost focus (#9216)"
This reverts commit bea183d85d.
* Revert "fix: Auto close drop-down divs on lost focus (reapply) (#9213)"
This reverts commit 0e16b0405a.
## 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/563
### Proposed Changes
This expands the functionality introduced in #9213 to also include widget divs.
### Reason for Changes
MakeCode makes use of widget div in several field editors, so the issues described in https://github.com/google/blockly-keyboard-experimentation/issues/563 aren't fully mitigated with #9213 alone.
This PR essentially adds the same support for auto-closing as drop-down divs now have, and enables this functionality by default.
Note the drop-down div change: it was missed in #9123 that the API change for drop-down div's `show` function is actually API-breaking, so this updates that API to be properly backward compatible (and reverts one test change that makes use of it).
The `FocusManager` change actually corrects an implementation issue from #9123: not updating the tracked focus status before calling the callback can result in focus being inadvertently restored if the callback triggers returning focus due to a lost focus situation. This was wrong for drop-down divs, too, but it's harder to notice there because the dismissal of the drop-down div happens on a timer (which means there's sufficient time for `FocusManager`'s state to correct prior to attempting to return from the ephemeral focus state).
Demonstration of fixed behavior (since the inline number editor uses a widget div):
[Screen recording 2025-07-08 2.12.31 PM.webm](https://github.com/user-attachments/assets/7c3c7c3c-224c-48f4-b4af-bde86feecfa8)
### Test Coverage
New widget div tests have been added to verify the new parameter and auto-close functionality.
The `FocusManager` test was updated to account for the new, and correct, behavior around the internal tracked ephemeral focus state.
Note that some `tabindex` state has been clarified and cleaned up in the test index page and `FocusManager`. It's fine (and preferable) for ephemeral-used elements to always be focusable rather than making them dynamically so (which avoids state bleed across test runs which was happening one of the new tests).
https://github.com/google/blockly-keyboard-experimentation/pull/649 includes additional tests for validating widget behaviors.
### Documentation
No new documentation should be needed here--the API documentation changes should be sufficient.
One documentation update was made in `dropdowndiv.ts` that corrects the documentation parameter ordering.
### Additional Information
Nothing further to add.
## 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/563
### Proposed Changes
This introduces support in `FocusManager` to receive feedback on when an ephemerally focused element entirely loses focus (that is, neither it nor any of its descendants have focus).
This also introduces a behavior change for drop-down divs using the previously mentioned functionality to automatically close themselves when they lose focus for any reason (e.g. clicking outside of the div or tab navigating away from it).
Finally, and **importantly**, this adds a case where ephemeral focus does _not_ automatically return to the previously focused node: when focus is lost to the ephemerally focused element's tree and isn't instead put on another focused node.
### Reason for Changes
Ultimately, focus is probably the best proxy for cases when a drop-down div ought to no longer be open. However, tracking focus only within the scope of the drop-down div utility is rather difficult since a lot of the same problems that `FocusManager` handles also occur here (with regards to both descendants and outside elements receiving focus). It made more sense to expand `FocusManager`'s ephemeral focus support:
- It was easier to implement this `FocusManager` and in a way that's much more robust (since it's leveraging existing event handlers).
- Using `FocusManager` trivialized the solution for drop-down divs.
- There could be other use cases where custom ephemeral focus uses might benefit from knowing when they lose focus.
This new support is enabled by default for all drop-down divs, but can be disabled by callers if they wish to revert to the previous behavior of not auto-closing.
The change for whether to restore ephemeral focus was needed to fix a drawback that arises from the automatic returning of ephemeral focus introduced in this PR: when a user clicks out of an open drop-down menu it will restore focus back to the node that held focus prior to taking ephemeral focus (since it properly hides the drop-down div and restores focus). This creates awkward behavior issues for both mouse and keyboard users:
- For mouse: trying to open a drop-down outside of Blockly will automatically close the drop-down when the Blockly drop-down finishes closing (since focus is stolen back away from the thing the user clicked on).
- For keyboard: tab navigating out of Blockly tries to force focus back to Blockly.
**New in v2 of this PR**: Commit 0363d67c18 is the main one that prevents #9203 from being reintroduced by ensuring that widget div only clears its contents after ephemeral focus has returned. This was missed in the first audit since it wasn't clear that this line, in particular, can cause a div with focus to be removed and thus focus lost: dfd565957b/core/widgetdiv.ts (L156)
### Test Coverage
New tests have been added for both the drop-down div and `FocusManager` components, and have been verified as failing without the new behaviors in place.
There may be other edge cases worth testing for `FocusManager` in particular, but the tests introduced in this PR seem to cover the most important cases.
**New in v2 of this PR**: A test was added to validate that widget div now clears its contents only after ephemeral focus has returned to avoid the desyncing scenario that led to #9203. This test has been verified to fail without the fix. There are also a few new tests being added in the keyboard navigation plugin repository that also validate this behavior at a higher level (see https://github.com/google/blockly-keyboard-experimentation/pull/649).
Demonstration of the new behavior:
[Screen recording 2025-07-01 6.28.37 PM.webm](https://github.com/user-attachments/assets/7af29fed-1ba1-4828-a6cd-65bb94509e72)
### Documentation
No new documentation changes seem needed beyond the code documentation updates.
### Additional Information
It's also possible to change the automatic restoration behavior to be conditional instead of always assuming focus shouldn't be reset if focus leaves the ephemeral element, but that's probably a better change if there's an actual user issue discovered with this approach.
This was originally introduced in #9175 but it was reverted in #9204 due to #9203.
## 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/563
### Proposed Changes
This introduces support in `FocusManager` to receive feedback on when an ephemerally focused element entirely loses focus (that is, neither it nor any of its descendants have focus).
This also introduces a behavior change for drop-down divs using the previously mentioned functionality to automatically close themselves when they lose focus for any reason (e.g. clicking outside of the div or tab navigating away from it).
Finally, and **importantly**, this adds a case where ephemeral focus does _not_ automatically return to the previously focused node: when focus is lost to the ephemerally focused element's tree and isn't instead put on another focused node.
### Reason for Changes
Ultimately, focus is probably the best proxy for cases when a drop-down div ought to no longer be open. However, tracking focus only within the scope of the drop-down div utility is rather difficult since a lot of the same problems that `FocusManager` handles also occur here (with regards to both descendants and outside elements receiving focus). It made more sense to expand `FocusManager`'s ephemeral focus support:
- It was easier to implement this `FocusManager` and in a way that's much more robust (since it's leveraging existing event handlers).
- Using `FocusManager` trivialized the solution for drop-down divs.
- There could be other use cases where custom ephemeral focus uses might benefit from knowing when they lose focus.
This new support is enabled by default for all drop-down divs, but can be disabled by callers if they wish to revert to the previous behavior of not auto-closing.
The change for whether to restore ephemeral focus was needed to fix a drawback that arises from the automatic returning of ephemeral focus introduced in this PR: when a user clicks out of an open drop-down menu it will restore focus back to the node that held focus prior to taking ephemeral focus (since it properly hides the drop-down div and restores focus). This creates awkward behavior issues for both mouse and keyboard users:
- For mouse: trying to open a drop-down outside of Blockly will automatically close the drop-down when the Blockly drop-down finishes closing (since focus is stolen back away from the thing the user clicked on).
- For keyboard: tab navigating out of Blockly tries to force focus back to Blockly.
### Test Coverage
New tests have been added for both the drop-down div and `FocusManager` components, and have been verified as failing without the new behaviors in place.
There may be other edge cases worth testing for `FocusManager` in particular, but the tests introduced in this PR seem to cover the most important cases.
Demonstration of the new behavior:
[Screen recording 2025-07-01 6.28.37 PM.webm](https://github.com/user-attachments/assets/7af29fed-1ba1-4828-a6cd-65bb94509e72)
### Documentation
No new documentation changes seem needed beyond the code documentation updates.
### Additional Information
It's also possible to change the automatic restoration behavior to be conditional instead of always assuming focus shouldn't be reset if focus leaves the ephemeral element, but that's probably a better change if there's an actual user issue discovered with this approach.
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes#9155
### Proposed Changes
In cases when an ID is missing for an element passed to `FocusableTreeTraverser.findFocusableNodeFor()`, always return `null`.
Additionally, the new short-circuit logic exposed that `Toolbox` actually wasn't being set up correctly (that is, its root element was not being configured with a valid ID). This has been fixed.
### Reason for Changes
These are cases when a valid node should never be matched (and it's technically possible to incorrectly match if an `IFocusableNode` is set up incorrectly and is providing a focusable element with an unset ID). This avoids the extra computation time of potentially calling deep into `WorkspaceSvg` and exploring all possible nodes for an ID that should never match.
Note that there is a weird quirk with `null` IDs actually being the string `"null"`. This is a side effect of how `setAttribute` and attributes in general work with HTML elements. There's nothing really that can be done here, so it's now considered invalid to also have an ID of string `"null"` just to ensure the `null` case is properly short-circuited.
Finally, the issue with toolbox being configured incorrectly was discovered with the introducing of a new hard failure in `FocusManager.registerTree()` when a tree with an invalid root element is registered. From testing there are no other such trees that need to be updated.
A new warning was also added if `focusNode()` is used on a node with an element that has an invalid ID. This isn't a hard failure to follow the convention of other invalid `focusNode()` situations. It's much more fragile for `focusNode()` to throw than `registerTree()` since the former generally happens much earlier in a page lifecycle, and is less prone to dynamic behaviors.
### Test Coverage
New tests were added to validate the various empty ID cases for `FocusableTreeTraverser.findFocusableNodeFor()`, and to validate the new error check for `FocusManager.registerTree()`.
### Documentation
No new documentation should be needed.
### Additional Information
Nothing to add.
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes#8965Fixes#8978Fixes#8970
Fixes https://github.com/google/blockly-keyboard-experimentation/issues/523
Fixes https://github.com/google/blockly-keyboard-experimentation/issues/547
Fixes part of #8910
### Proposed Changes
Fives groups of changes are included in this PR:
1. Support for automatic tab index management for focusable trees.
2. Support for automatic tab index management for focusable nodes.
3. Support for automatically hiding the flyout when back navigating from the toolbox.
4. A fix for `FocusManager` losing DOM syncing that was introduced in #9082.
5. Some cleanups for flyout and some tests for previous behavior changes to `FocusManager`.
### Reason for Changes
Infrastructure changes reasoning:
- Automatically managing tab indexes for both focusable trees and roots can largely reduce the difficulty of providing focusable nodes/trees and generally interacting with `FocusManager`. This facilitates a more automated navigation experience.
- The fix for losing DOM syncing is possibly not reliable, but there are at least now tests to cover for it. This may be a case where a `try{} finally{}` could be warranted, but the code will stay as-is unless requested otherwise.
`Flyout` changes:
- `Flyout` no longer needs to be a focusable tree, but removing that would be an API breakage. Instead, it throws for most of the normal tree/node calls as it should no longer be used as such. Instead, its workspace has been made top-level tabbable (in addition to the main workspace) which solves the extra tab stop issues and general confusing inconsistencies between the flyout, toolbox, and workspace.
- `Flyout` now correctly auto-selects the first block (#9103 notwithstanding). Technically it did before, however the extra `Flyout` tabstop before its workspace caused the inconsistency (since focusing the `Flyout` itself did not auto-select, only selecting its workspace did).
Important caveats:
- `getAttribute` is used in place of directly fetching `.tabIndex` since the latter can apparently default to `-1` (and possibly `0`) in cases when it's not actually set. This is a very surprising behavior that leads to incorrect test results.
- Sometimes tab index still needs to be introduced (such as in cases where native DOM focus is needed, e.g. via `focus()` calls or clicking). This is demonstrated both by updates to `FocusManager`'s tests as well as toolbox's category and separator. This can be slightly tricky to miss as large parts of Blockly now depend on focus to represent their state, so clicking either needs to be managed by Blockly (with corresponding `focusNode` calls) or automatic (with a tab index defined for the element that can be clicked, or which has a child that can be clicked).
Note that nearly all elements used for testing focus in the test `index.html` page have had their tab indexes removed to lean on `FocusManager`'s automatic tab management (though as mentioned above there is still some manual tab index management required for `focus()`-specific tests).
### Test Coverage
New tests were added for all of the updated behaviors to `FocusManager`, including a new need to explicitly provide (and reset) tab indexes for all `focus()`-esque tests. This also includes adding new tests for some behaviors introduced in past PRs (a la #8910).
Note that all of the new and affected conditionals in `FocusManager` have been verified as having at least 1 test that breaks when it's removed (inverted conditions weren't thoroughly tested, but it's expected that they should also be well covered now).
Additional tests to cover the actual navigation flows will be added to the keyboard experimentation plugin repository as part of https://github.com/google/blockly-keyboard-experimentation/pull/557 (this PR needs to be merged first).
For manual testing, I mainly verified keyboard navigation with some cursory mouse & click testing in the simple playground. @rachel-fenichel also performed more thorough mouse & click testing (that yielded an actual issue that was fixed--see discussion below).
The core webdriver tests have been verified to have seemingly the same existing failures with and without these changes.
All of the following new keyboard navigation plugin tests have been verified as failing without the fixes introduced in this branch (and passing with them):
- `Tab navigating to flyout should auto-select first block`
- `Keyboard nav to different toolbox category should auto-select first block`
- `Keyboard nav to different toolbox category and block should select different block`
- `Tab navigate away from toolbox restores focus to initial element`
- `Tab navigate away from toolbox closes flyout`
- `Tab navigate away from flyout to toolbox and away closes flyout`
- `Tabbing to the workspace after selecting flyout block should close the flyout`
- `Tabbing to the workspace after selecting flyout block via workspace toolbox shortcut should close the flyout`
- `Tabbing back from workspace should reopen the flyout`
- `Navigation position in workspace should be retained when tabbing to flyout and back`
- `Clicking outside Blockly with focused toolbox closes the flyout`
- `Clicking outside Blockly with focused flyout closes the flyout`
- `Clicking on toolbox category focuses it and opens flyout`
### Documentation
No documentation changes are needed beyond the code doc changes included in the PR.
### Additional Information
An additional PR will be introduced for the keyboard experimentation plugin repository to add tests there (see test coverage above). This description will be updated with a link to that PR once it exists.
## 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/87
### Proposed Changes
This updates `FocusManager.focusNode()` to automatically defocus its internal state if it detects that DOM focus (per `document.activeElement`) doesn't match its own internal focus.
It also updates `FocusManager` to avoid duplicate self calls to `focusNode()`.
### Reason for Changes
This is a robustness improvement for `focusNode` that is nice to keep as a "if all else fails" mechanism, but it's currently a hacky workaround to https://github.com/google/blockly-keyboard-experimentation/issues/87. #9081 is tracking introducing a long-term fix for the desynchronizing problem, but that's likely to be potentially much harder to solve and this at least introduces a reasonable correction.
From a stability perspective, it seems likely that there are multiple classes of failures covered by this fix. Essentially the browser behavior difference in Firefox and Safari over Chrome is that the former do not fire a focus change event when a focused element is removed from the DOM (leading to `FocusManager` getting out of sync). There may be other such cases when a focus event isn't fired, so this robustness improvement at least ensures eventual consistency so long as `focusNode()` is called (and, fortunately, that's done a lot now).
While this is a nice robustness improvement, it's not a perfect replacement for a real fix. For the time between `FocusManager` getting out of sync and `focusNode` getting called, `getFocusedNode` will _not_ match the actual element holding focus. The primary class of issues known is when a DOM element is being moved, and in these cases `focusNode` _is_ called. If there are other such unknown cases where a desync can happen, **`getFocusedNode()` will remain wrong until a later `focusNode()` call**.
Note one other change: originally implemented but removed in earlier PRs for `FocusManager`, this change also includes ensuring `focusNode()` isn't called multiple times for a single request to focus a node. Current logic results in a call to `focusNode()` calling a node's `focus()` which then processes a second call to `focusNode()` (which is fully executed because `FocusManager.focusedNode` isn't updated until after the call to `focus()`). This doesn't actually correct any state, but it's more efficient and provides some resilience against potential logic issues from calling node/tree callbacks multiple times (which was observed up to 3 times in some cases).
### Test Coverage
This has been tested via the keyboard navigation experimental plugin's test environment (with Firefox), plus new unit tests. Note the new test for directly verifying desyncing logic is contrived, but it should be perfectly testing the exact scenario that's being observed on Firefox/Safari. A separate test was added for the existing behavior of focusing a different node still correcting `FocusManager` state even if it was desynced (the bug only happens for the same node being refocused).
New tests were also added for the various lifecycle callbacks (to ensure they aren't called multiple times).
All of the new tests were verified to fail without the two fixes in place (they were verified in isolation), minus the test for focusing a second node when desynced (since that should pass regardless of the new fixes).
Some basic simple playground testing was done, as well, just to verify nothing obvious was broken around selection, gestures, and copy/paste.
### Documentation
No new documentation should be needed here.
### Additional Information
This wasn't explicitly tested in Safari since I only have access to Chrome and Firefox, but I will ask someone else on the team to verify this for me after merging if it isn't checked sooner.
* 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.
## 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#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.
This introduces new callback methods for IFocusableTree and
IFocusableNode for providing a basis of synchronizing domain state with
focus changes. It also introduces support for implementations of
IFocusableTree to better manage initial state cases, especially when a
tree is focused using tab navigation.
FocusManager has also been updated to ensure functional parity between
tab-navigating to a tree and using focusTree() on that tree. This means
that tab navigating to a tree will actually restore focus back to that
tree's previous focused node rather than the root (unless the root is
navigated to from within the tree itself). This is meant to provide
better consistency between tab and non-tab keyboard navigation.
Note that these changes originally came from #8875 and are required for
later PRs that will introduce IFocusableNode and IFocusableTree
implementations.
This adds new tests for the FocusableTreeTraverser and fixes a number of
issues with the original implementation (one of which required two new
API methods to be added to IFocusableTree). More tests have also been
added for FocusManager, and defocusing tracked nodes/trees has been
fully implemented in FocusManager.
This is the bulk of the work for introducing the central logical unit
for managing and sychronizing focus as a first-class Blockly concept
with that of DOM focus.
There's a lot to do yet, including:
- Ensuring clicks within Blockly's scope correctly sync back to focus
changes.
- Adding support for, and testing, cases when focus is lost from all
registered trees.
- Testing nested tree propagation.
- Testing the traverser utility class.
- Adding implementations for IFocusableTree and IFocusableNode
throughout Blockly.
