* 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 https://github.com/google/blockly-keyboard-experimentation/issues/578
Fixes part of #8915
### Proposed Changes
Ensures fields update focus to the next field when tabbing between field editors. The old behavior can be observed in [#578](https://github.com/google/blockly-keyboard-experimentation/issues/578) and the new behavior can be observed here:
[Screen recording 2025-06-25 1.39.28 PM.webm](https://github.com/user-attachments/assets/e00fcb55-5c20-4d5c-81a8-be9049cc0d7e)
### Reason for Changes
Having focus reset back to the original field editor that was opened is an unexpected experience for users. This approach is better.
Note that there are some separate changes added here, as well:
- Connections and fields now check if their block IDs contain their indicator prefixes since this will all-but-guarantee focus breaks for those nodes. This is an excellent example of why #9171 is needed.
- Some minor naming updates for `FieldInput`: it was incorrectly implying that key events are sent for changes to the `input` element used by the field editor, but they are actually `InputEvent`s per https://developer.mozilla.org/en-US/docs/Web/API/Element/input_event.
### Test Coverage
New tests were added for field editing in general (since this seems to be missing), including tabbing support to ensure the fixes originally introduced in #9049.
One new test has been added specifically for verifying that focus updates with tabbing. This has been verified to fail with the fix removed (as have all tabbing tests with the tabbing code from #9049 removed).
Some specific notes for the test changes:
- There's a slight test dependency inversion happening here. `FieldInput` is being tested with a specific `FieldNumber` class via real block loading. This isn't ideal, but it seems fine given the circumstances (otherwise a lot of extra setup would be necessary for the tests).
- The workspace actually needs to be made visible during tests in order for focus to work correctly (though it's reset at the end of each test, but this may cause some flickering while the tests are running).
- It's the case that a bunch of tests were actually setting up blocks incorrectly (i.e. not defining a must-have `id` property which caused some issues with the new field and connection ID validation checks). These tests have been corrected, but it's worth noting that the blocks are likely still technically wrong since they are not conforming to their TypeScript contracts.
- Similar to the previous point, one test was incorrectly setting the first ID to be returned by the ID generator as `undefined` since (presumably due to a copy-and-paste error when the test was introduced) it was referencing a `TEST_BLOCK_ID` property that hadn't been defined for that test suite. This has been corrected as, without it, there are failures due to the new validation checks.
- For the connection database checks, a new ID is generated instead of fixing the block ID to ensure that it's always unique even if called multiple times (otherwise a block ID would need to be piped through from the calling tests, or an invalid situation would need to be introduced where multiple blocks shared an ID; the former seemed unnecessary and the latter seemed nonideal).
- There are distinct Geras/Zelos tests to validate the case where a full-block field should have its parent block, rather than the field itself, focused on tabbing. See this conversation for more context: https://github.com/google/blockly/pull/9173#discussion_r2172921455.
### Documentation
No documentation changes should be needed here.
### 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#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.
* refactor(interfaces): Use typeof ... === 'function' to test for methods
Testing for
'name' in object
or
obj.name !== undefined
only checks for the existence of the property (and in the latter
case that the property is not set to undefined). That's fine if
the interface specifies a property of indeterminate type, but in
the usual case that the interface member is a method we can do
one better and check to make sure the property's value is
callable.
* refactor(interfaces): Always check obj is not null/undefined
Since most type predicates take an argument of type any but then
check for the existence of certain properties, explicitly check
that the argument is not null or undefined (or check implicitly
by calling another type predicate that does so first, which
necessitates adding a few casts because tsc infers the type of
the argument too narrowly).
* fix(interfaces): Add missing check to hasBubble type predicate
This appears to have inadvertently been omitted in PR #9004.
* fix(interfaces): Fix misplaced typeof
* fix: Fix typos in JSDocs
* fix(tests): Make Mocks conform to corresponding interfaces
Introduce a new MockFocusable, and add methods to MockIcon,
MockBubbleIcon and MockComment, so that they fulfil the
IFocusableNode, IIcon, IHasBubble and ICommentIcon interfaces
respectively.
* chore(tests): Add assertions verifying mocks conform to predicates
Add (test) runtime assertions that:
- isFocusableNode(MockFocusable) returns true
- isIcon(MockIcon) returns true
- hasBubble(MockBubbleIcon) returns true
- isCommentIcon(MockCommentIcon) returns true
(The latter is currently failing because Blockly is undefined when
isCommentIcon calls the MockCommentIcon's getType method.)
* fix(tests): Don't rely on Blockly being set in Mock methods
For some reason the global Blockly binding is not visible at the
time when isCommentIcon calls MockCommentIcon's getType method,
and presumably this problem would apply to getBubbleSize too,
so directly import the required items.
* refactor(tests): Make MockCommentIcon a MockBubbleIcon
This slightly simplifies it and makes it less likely to accidentally
stop conforming to IHasBubble.
* fix(interfaces): Fix incorrect check in isSelectable
Fix an error which caused ISelectable instances to fail
isSelectable() checks, one of the results of which is that
Blockly.common.getSelected() would generally return null.
Whoops!
## 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.
* fix: Fix bug that prevented using keyboard shortcuts when the DropDownDiv is open.
* chore: Remove obsolete comment.
* Refactor: Remove unreachable null check.
* chore: Add tests for handling Escape to dismiss the Widget/DropDownDivs.
* chore: Satisfy the linter.
* fix: Fix post-merge test failure.
* WIP on line by line navigation
Doesn't work, likely due to isValid check.
* Add all inputs to the list of siblings
* Fix formatting
* Add tests
* Remove dupe keys
* fix: Make blocks with display: none not focusable
* Undo changes to canBeFocused
* Don't traverse inputs that are invisible
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes#9078
Fixes part of #8915 (new tests)
### Proposed Changes
Exposes the ability to disable ephemeral focus management for drop-down divs that are shown using `showPositionedByBlock` or `showPositionedByField`. Previously, this was only supported via `show`, but the former methods are also used externally.
This allows the underlying issue reported by #9078 to be fixed downstream for cases when both the widget and drop-down divs are opened simultaneously.
This PR also introduces tab indexes for both widget and drop-down divs (which were noticed missing when adding tests). This is because, currently, taking ephemeral focus on for a node that doesn't have a tab index will do nothing. This fix is useful for future screen reader work, and doesn't have obvious impacts on existing core or keyboard navigation behaviors (per testing and reasoning).
### Reason for Changes
Exposing the ability to disable ephemeral focus management for all public API entrypoints for showing the divs is crucial for providing the maximum flexibility when downstream apps use both the widget and drop-down divs together. This should ensure that all of these cases can be correctly managed in the same way as https://github.com/google/blockly-samples/pull/2521.
### Test Coverage
This introduces a bunch of new tests that were missing originally for both widget and drop-down div (including specifically verifying ephemeral focus). As part of the drop-down div tests, it also introduces actual positioning logic. This isn't great, but it's somewhat reasonable and robust against page changes (since the actual mocha results can move where the elements will end up on the page).
These changes have also been manually tested with both the core simple playground and the keyboard experiment plugin's test environment with no noticed regressions in either. The plugin's tests have also been run against these changes to ensure no new breakages have been introduced.
### Documentation
No documentation changes beyond the code ones introduced in this PR should be needed.
### Additional Information
The new tests may actually act as a basis for avoiding the test backdoor that's used today for the positioning tests for drop-down div tests. This doesn't replace those existing tests nor does it cover other behaviors and entrypoints that would be worth testing, but testing ephemeral focus is a nice improvement (especially in the context of what this PR is fixing).
## 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.
* WIP on line by line navigation
Doesn't work, likely due to isValid check.
* Add all inputs to the list of siblings
* Fix formatting
* Add tests
* Remove dupe keys
* Move block into view before clicking
* fix right click test failures
* Fix drag three blocks test
dragAndDrop is relative to the start and the test window is very small.
* Fix a few more tests
- Switch to using clickBlock instead of getting the block and clicking it
- Update drag positions for some tests so they don't snap and change size
* Add a pause between right clicking a block and waiting for the menu
* Fix mutator test by finding the dragged out elseif block
* Make disable test less flakey
## 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.
* 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.
