* fix: Make left and right arrow keys only navigate within the current block/comment
* fix: Fix tests
* fix: Remove .only from test suite
* test: Reenable statement block navigation tests with modifications
* chore: Clarify comments
## The basics
- [ ] ~I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)~ (I can't really due to the nature of this change--I have to rely on tests and external testing)
## The details
### Resolves
Fixes part of #9456 (remainder will be in a change in the keyboard navigation plugin)
### Proposed Changes
Introduce and use new `Block` function for retrieving a configurably constrained singleton field for a given block. The constraints allow for some level of configuring (such as whether to isolate to only full or editable blocks). The existing simple reporter function has been retrofitted to use this new function, instead.
### Reason for Changes
This expanded support fixes the underlying use case.
Separately, this change reveals two noteworthy details:
1. There's inconsistency in the codebase as to when the singleton field needs to be editable, a full-block field, both, and neither. It would be ideal to make this consistent. Interestingly, the documentation for `isSimpleReporter` seems to have been wrong since it wasn't actually fulfilling its contract of returning an editable field (this has been retained for callsites except where the check was already happening).
2. There's a possible recursion case now possible between `getSingletonFullBlockField` and `isFullBlockField` due to `FieldInput`'s `isFullBlockField` depending on `isSimpleReporter`. Ideally this would be changed in the future to avoid that potential recursion risk (possibly as part of #9307).
### Test Coverage
No new automated tests are needed for this experimental work. Manual testing mainly comprised of cursory navigation and readout checks for single-field blocks to make sure nothing breaks. More thorough testing is difficult in core since the specific situation of multiple fields don't have a corresponding block to use in the playground to verify.
Automated tests are also being heavily relied on for correctness since all of the nuance behind the simple reporter cases would require a deeper testing pass.
### Documentation
No new documentation needed for this experimental work.
### Additional Information
None.
* fix: Improve narration and navigation of C-shaped blocks.
* chore: Satisfy the linter.
* chore: Refactor and comment `getBlockNavigationCandidates()`.
* refactor: Reduce code duplication in `LineCursor`.
* fix: Add missing case when labeling connections.
* refactor: Make focusable elements responsible for scrolling themselves into bounds.
* chore: Add tests for scrolling focused elements into view.
* fix: Removed inadvertent `.only`.
* fix: Scroll parent block of connections into bounds on focus.
* refactor: Update `TextInputBubble` to use `CommentEditor` for text editing.
* feat: Designate `Bubble` as implementing `IFocusableNode`.
* feat: Dismiss focused bubbles on Escape.
* feat: Add support for keyboard navigation to block comments.
* fix: Scroll comment editors rather than zooming the workspace.
* chore: Add param to docstring.
* feat: Enhance the Rect API.
* feat: Add support for sorting IBoundedElements in general.
* fix: Improve typings of getTopElement/Comment methods.
* feat: Add classes to represent comment icons.
* refactor: Use comment icons in comment view.
* feat: Update navigation policies to support workspace comments.
* feat: Make the navigator and workspace handle workspace comments.
* feat: Visit workspace comments when navigating with the up/down arrows.
* chore: Make the linter happy.
* chore: Rename comment icons to bar buttons.
* refactor: Rename CommentIcons to CommentBarButtons.
* chore: Improve docstrings.
* chore: Clarify unit type.
* refactor: Remove workspace argument from `navigateStacks()`.
* fix: Fix errant find and replace in CSS.
* fix: Fix issue that could cause delete button to become misaligned.
* 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
* 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
## 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/526
### Proposed Changes
This limits synchronizing in `LineCursor` from `FocusManager` to just nodes that have a corresponding block.
### Reason for Changes
Limiting the synchronizing in this way ensures that navigation can't enter a bad state. The reason for why this is needed is explained in https://github.com/google/blockly-keyboard-experimentation/issues/526#issuecomment-2885117998.
Longer term it would maybe be ideal to do one or both of the following:
- Figure out ways of making navigation a bit more robust (perhaps on the keyboard navigation side) such that if cursor _is_ in a bad state there's some way to recover (rather than ending up permanently broken).
- Remove `Marker`'s internal state in favor of always relying on `FocusManager`'s state to cover the cases where there can be automatic focus shifting.
### Test Coverage
This was manually tested with the keyboard navigation plugin and verified to ensure that both https://github.com/google/blockly-keyboard-experimentation/issues/526 and https://github.com/google/blockly-keyboard-experimentation/issues/499 are (still) working as expected. Some basic testing was done with the core simple playground with the developer console open to ensure there weren't any expected failures.
Automated testing cases would be better addressed as part of resolving #8915.
### Documentation
No new documentation is needed here.
### Additional Information
This behavior is expected to only affect the keyboard navigation plugin.
## 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).
* 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#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.
* chore: tests for cursor getNextNode
* chore: add tests for getPreviousNode
* feat: add looping to getPreviousNode and getNextNode
* chore: inline returns
* chore: fix test that results in a stack node
* chore: fix annotations
* refactor: Backport LineCursor to core.
* fix: Fix instantiation of LineCursor.
* fix: Fix tests.
* chore: Assauge the linter.
* chore: Fix some typos.
* feat: Make padding configurable for scrollBoundsIntoView.
* chore: Merge in the latest changes from keyboard-experimentation.
* refactor: Clarify name and docs for findSiblingOrParentSibling().
* fix: Improve scrollBoundsIntoView() behavior.
* fix: Export CursorOptions.
* refactor: Further clarify second parameter of setCurNode().
* fix: Revert change that could prevent scrolling bounds into view.
