* 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.
* 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.
_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.
* Revert "feat: Make toolbox and flyout focusable (#8920)"
This reverts commit 5bc83808bf.
* Revert "feat: Make WorkspaceSvg and BlockSvg focusable (#8916)"
This reverts commit d7680cf32e.
## 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 #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`.
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).
### 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 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.
Introduces the necessary base interfaces for representing different
focusable contexts within Blockly. The actual logic for utilizing and
implementing these interfaces will come in later PRs.
* chore: rename .eslintrc.js to eslint.config.js
* chore: Rename eslint.config.js to eslint.config.mjs.
* refactor: Migrate ESLint config to new flat format.
* chore: Remove old per-directory and global ignore ESLint config files.
* fix: Allowlist JSDoc tag aliases.
* fix: Don't require @license in tests/*.
* fix: Add NodeJS globals to several files that run under Node.
* chore: Remove now-unneeded ESLint directives in core.
* chore: Remove invalid/unneeded ESLint directives.
* fix: Fix invalid use of `await` outside of an `async` function.
* fix: Improve screenshot error message.
* fix: Update ESLint config file to not warn on existing violations.
* chore: Remove suppressions of rules that weren't triggering.
* chore: Fix package-lock.json.
* chore(deps): Add pretter-plugin-organize-imports
* chore: Remove insignificant blank lines in import sections
Since prettier-plugin-organize-imports sorts imports within
sections separated by blank lines, but preserves the section
divisions, remove any blank lines that are not dividing imports
into meaningful sections.
Do not remove blank lines separating side-effect-only imports
from main imports.
* chore: Remove unneded eslint-disable directives
* chore: Organise imports
* chore(dragging): Rename core/interfaces/i_draggable.ts
Rename core/interfaces/i_draggable.ts to
core/interfaces/i_draggable.old.ts to make room for new
IDraggable. Do not rename actual interface as it's not yet
clear that it will be necessary for both to coexist as
imports in the same file.
* feat(dragging): Introduce new IDraggable interface
* feat(dragging): Introduce new IDragger interface
---------
Co-authored-by: Beka Westberg <bwestberg@google.com>
