## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes#9304
### Proposed Changes
Fixes non-visual parent roles and rendered connection navigation policy.
### Reason for Changes
Other PRs have made progress on removing extraneous accessibility nodes with #9446 being essentially the last of these. Ensuring that parent/child relationships are correct is the last step in ensuring that the entirety of the accessibility node graph is correctly representing the DOM and navigational structure of Blockly.
This can have implications and (ideally) improvements for certain screen reader modes that provide higher-level summarization and sometimes navigation (bypassing Blockly's keyboard navigation) since it avoids an incorrect flat node structure and instead ensures correct hierarchy and ordering.
It was discovered during the development of the PR that setting `aria-owns` properties to ensure that all focusable accessibility nodes have the correct parent/child relationships (particularly for blocks) isn't actually viable per the analysis summarized in this comment: https://github.com/RaspberryPiFoundation/blockly/pull/9449#issuecomment-3663234767. At a high level introducing these relationships seems to actually cause problems in both ChromeVox and Voiceover.
Part of the analysis discovered that nodes set with the `presentation` role aren't going to behave correctly due to the spec ignoring that role if any children of such elements are focusable, so this PR does change those over to `generic` which is more correct. They are still missing in Chrome's accessibility node viewer, and `generic` _seems_ to introduce slightly better `group` behaviors on VoiceOver (that is, it seems to reduce some of the `group` announcements which VoiceOver is known for over-specifying).
Note that some tests needed to be updated to ensure that they were properly rendering blocks (in order for `RenderedConnection.canBeFocused()` to behave correctly) in the original implementation of the PR. Only one test actually changed in behavior because it seemed like it was incorrect before--the particular connection being tested wasn't actually navigable and the change to `canBeFocused` actually enforces that. These changes were kept even though the behaviors weren't needed anymore since it's still a bit more correct than before.
Overall, #9304 is closed here because the tree seems to be about as good as it can get with current knowledge (assuming no other invalid roles need to be fixed, but that can be addressed in separate issues as needed).
### Test Coverage
No automated tests are needed for this since it's experimental but it has been manually tested with both ChromeVox and Voiceover.
### Documentation
No documentation changes are needed for these experimental changes.
### Additional Information
Note that there are some limitations with this approach: text editors and listboxes (e.g. for comboboxes) are generally outside of the hierarchy represented by the Blockly workspace. This is an existing issue that remains unaffected by these changes, and fixing it to be both ARIA compliant and consistent with the DOM may not be possible (though it doesn't seem like there's a strong requirement to maintain DOM and accessibility node tree hierarchical relationships).
The analysis linked above also considered introducing a top-level `application` role which might change some of the automated behaviors of certain roles but this only seemed to worsen local testing with ChromeVox so it was excluded.
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes#9495
### Proposed Changes
Changes a bunch of ARIA role & label management to ensure that `Flyout` acts like a list rather than a tree.
### Reason for Changes
`Flyout`s are always hierarchically flat so it doesn't make sense to model them as a tree. Instead, a menu is likely a better fit per https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/menu_role:
> A `menu` generally represents a grouping of common actions or functions that the user can invoke. The `menu` role is appropriate when a list of menu items is presented in a manner similar to a menu on a desktop application. Submenus, also known as pop-up menus, also have the role `menu`.
However, there are important caveats that need to be considered and addressed:
- As discussed below, menus introduce some unexpected compatibility issues with VoiceOver so this PR presently uses `list` and `listitem`s as a slightly more generic enumerating alternative for menus.
- Menus (and to some extent lists) are stricter\* than trees in that they seem to impose a requirement that `menuitem`s cannot contain interactive elements (they are expected to be interactive themselves). This has led to a few specific changes:
- Block children are now hidden when in the flyout (since they aren't navigable anyway).
- Flyout buttons are themselves now the `menuitem` rather than their container parent, and they no longer use the role button.
- Menus aren't really expected to contain labels but it isn't inherently disallowed. This is less of an issue with lists.
- Because everything must be a `listitem` (or a few more specific alternatives) both blocks and buttons lack some context. Since not all `Flyout` items can be expected to be interactive, buttons and blocks have both had their labels updated to include an explicit indicator that they are buttons and blocks, respectively. Note that this does possibly go against convention for buttons in particular but it seems fine since this is an unusual (but seemingly correct) utilization of a `list` element.
- To further provide context on blocks, the generated label for blocks in the `Flyout` is now its verbose label rather than the more compact form.
\* This is largely a consequence of a few specific attributes of `menuitem` and `menu`s as a whole and very likely also applies to `tree`s and `treeitem`s (and `list`s and `listitems`s). However, now seemed like a good time to fix this especially in case some screen readers get confused rather than ignore nested interactive controls/follow semantic cloaking per the spec.
Demo of it working on VoiceOver (per @gonfunko -- note this was the `menu` variant rather than the `list` variant of the PR):
### Test Coverage
This has been manually tested with ChromeVox. No automated tests are needed as part of this experimental change.
### Documentation
No new documentation changes are needed for this experimental change.
### Additional Information
None.
* feat: Use custom ARIA roledescriptions for different block types
* fix: Don't include 'begin stack' in block ARIA labels in the flyout
* fix: Fix visibility of `computeAriaLabel`
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes part of https://github.com/RaspberryPiFoundation/blockly-keyboard-experimentation/issues/764
Fixes part of #9450 (infrastructure needs)
### Proposed Changes
Introduces support for two new "where am I?" shortcuts for helping to provide location context for users:
- `I`: re-reads the current selected block with full verbosity (i.e. also includes the block's field types with their values in the readout).
- `shift+I`: reads the current selected block's parent with full verbosity.
Note that this includes some functional changes to `Field` to allow for more powerful customization of a field's ARIA representation (by splitting up value and type), though a field's value defaults potentially to null which will be ignored in the final ARIA computed label. This seems necessary per the discussion here: https://github.com/RaspberryPiFoundation/blockly/pull/9470/files#r2541508565 but more consideration may be needed here as part of #9307.
Some limitations in the new shortcuts:
- They will not read out anything if a block is not selected (e.g. for fields and icons).
- They read out input blocks when the input block is selected.
- They cannot read out anything while in move mode (due to the behavior here in the plugin which automatically cancels moves if an unknown shortcut is pressed: a36f3662b0/src/actions/mover.ts (L166-L191)).
- The readout is limited by the problems of dynamic ARIA announcements (per #9460).
### Reason for Changes
https://github.com/RaspberryPiFoundation/blockly-keyboard-experimentation/issues/764 provides context on the specific needs addressed here.
### Test Coverage
Self tested. No new automated tests needed for experimental work.
### Documentation
No new documentation needed for experimental work.
### Additional Information
This was spun out of #9470 with the intent of getting shortcuts initially working checked in even if the entirety of the experience is incomplete.
## 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.
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes#9301Fixes#9312Fixes#9313
Fixes part of #9304
### Proposed Changes
This introduces a variety of specific changes to resolve several issues for screen reader work, including introducing fundamental support for field labeling.
Specifically:
- Field labels have been simplified to only use their custom defined ARIA name otherwise they are null (and thus should be ignored for readout purposes) which wraps up the remaining high-level work for #9301 (#9450 tracks more specific follow-up work to improve upon what's been established at this point). The PR also introduces an ARIA override for number inputs in math blocks so that the readout is correct for them.
- Bubble labeling is more explicit now which is useful for mutators (#9312), warnings, and comments. The general improvement for bubbles wraps up the remaining work for #9313 as well since the core issue was resolved in #9351. By default a bubble has no ARIA label.
- #9304 is partly being addressed here with the change to field images: they are no longer being added to the accessibility node tree unless they are actually navigable (that is, clickable). Part of #9304's goal is to remove extraneous nodes.
- Finally, a typo was fixed for 'replaceable blocks' since these were not reading out correctly. This was noticed in passing and isn't directly related to the other issues.
### Reason for Changes
This PR is largely being used as a basis for one particularly significant issue: #9301. Field labeling has undergone several iterations over the past few months and the team seems comfortable sticking with a "do as little as possible" approach when determining the label, thus justifying the need for expecting more specific customization (i.e. #9450). To this end it's important to be clear that getting fields to a good state is not actually "done" but the need to track it as a large incomplete thing has ended. Note that one important part of #9301 was updating field plugins to be accessible--this largely seems unnecessary as-is as it will be completely dependent on the needs of future user tests. The long-term plan will need to account for making all fields in `blockly-samples` accessible (per #9307).
Some of the terminology used here (e.g. for bubbles) will likely need to change after user testing, but it's important to establish that _something_ correct is communicated even if the terminology may require scaffolding and/or refinement.
It's important to note that while non-clickable field images are no longer in the node graph, their ARIA presence still exists as part of the fluent block labeling solution. That is, `FieldImage`'s alt text is used as part of constructing a fluent block label (sometimes to confusing effect--see #9452).
### Test Coverage
No tests needed since these are experimental changes and do not change existing test behaviors.
### Documentation
No documentation changes are needed for these experimental changes.
### Additional Information
None.
* fix: Miscellaneous improvements for screenreader support.
* fix: Include field name in ARIA label.
* fix: Update block ARIA labels when inputs are shown/hidden.
* fix: Make field row label generation more robust.
* 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.
Read value-inputs and fields in place and recursively.
Announce block shape, number of inputs and number of children
where appropriate.
Co-authored-by: Matt Hillsdon <matt.hillsdon@microbit.org>
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes#9386
Fixes part of #9293
### Proposed Changes
Addresses #9386 through a number of changes:
- Ensures the flyout contents are reevaluated for ARIA changes whenever they themselves change (since previously `WorkspaceSvg` only recomputed its ARIA properties when one of its blocks self-registered which doesn't account for labels or buttons).
- Collapsible categories are now correctly wrapped by a group (since groups of tree items must be in a parent group).
- Updates toolbox ARIA computations to be on content changes rather than having items self-specify recomputing the ARIA properties. This mitigates an issue with collapsible categories not updating the toolbox contents and thus being omitted.
- Introduced a separate pathway for computing tree info for flyout workspaces (vs. for the main workspace) in order to account for `FlyoutButton`s.
- Updated `FlyoutButton` to use a nested structure of `treeitem` then `button` in order to actually count correctly in the tree and still be an interactive button. The readout experience is actually better now on ChromeVox, as well, since it reads out _both_ 'Button' and 'Tree item' which is interesting. It seems screen readers are designed to look for this pattern but it only works if set up in a very particular way.
### Reason for Changes
Most of the changes here fixed incidental problems noticed while trying to address #9386 (such as the variables category not correctly accounting for the 'Create variable' button in the count, or not having the correct labels). Much of the count issues in the original issue were caused by a combination of missing some flyout items, and trying to compute the labels too early (i.e. before the categories were fully populated).
### Test Coverage
Since this is an experimental change, no new tests were added.
### Documentation
No documentation changes are directly needed here.
### Additional Information
None.
* 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.
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes part of #8207
Fixes part of #3370
### Proposed Changes
This introduces initial broad ARIA integration in order to enable at least basic screen reader support when using keyboard navigation.
Largely this involves introducing ARIA roles and labels in a bunch of places, sometimes done in a way to override normal built-in behaviors of the accessibility node tree in order to get a richer first-class output for Blockly (such as for blocks and workspaces).
### Reason for Changes
ARIA is the fundamental basis for configuring how focusable nodes in Blockly are represented to the user when using a screen reader. As such, all focusable nodes requires labels and roles in order to correctly communicate their contexts.
The specific approach taken in this PR is to simply add labels and roles to all nodes where obvious with some extra work done for `WorkspaceSvg` and `BlockSvg` in order to represent blocks as a tree (since that seems to be the best fitting ARIA role per those available: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles). The custom work specifically for blocks includes:
- Overriding the role description to be 'block' rather than 'tree item' (which is the default).
- Overriding the position, level, and number of sibling counts since those are normally determined based on the DOM tree and blocks are not laid out in the tree the same way they are visually or logically (so these computations were incorrect). This is also the reason for a bunch of extra computation logic being introduced.
One note on some of the labels being nonsensical (e.g. 'DoNotOverride?'): this was done intentionally to try and ensure _all_ focusable nodes (that can be focused) have labels, even when the specifics of what that label should be aren't yet clear. More components had these temporary labels until testing revealed how exactly they would behave from a screen reader perspective (at which point their roles and labels were updated as needed). The temporary labels act as an indicator when navigating through the UI, and some of the nodes can't easily be reached (for reasons) and thus may never actually need a label. More work is needed in understanding both what components need labels and what those labels should be, but that will be done beyond this PR.
### Test Coverage
No tests are added to this as it's experimental and not a final implementation.
The keyboard navigation tests are failing due to a visibility expansion of `connectionCandidate` in `BlockDragStrategy`. There's no way to avoid this breakage, unfortunately. Instead, this PR will be merged and then https://github.com/google/blockly-keyboard-experimentation/pull/684 will be finalized and merged to fix it. There's some additional work that will happen both in that branch and in a later PR in core Blockly to integrate the two experimentation branches as part of #9283 so that CI passes correctly for both branches.
### Documentation
No documentation is needed at this time.
### Additional Information
This work is experimental and is meant to serve two purposes:
- Provide a foundation for testing and iterating the core screen reader experience in Blockly.
- Provide a reference point for designing a long-term solution that accounts for all requirements collected during user testing.
This code should never be merged into `develop` as it stands. Instead, it will be redesigned with maintainability, testing, and correctness in mind at a future date (see https://github.com/google/blockly-keyboard-experimentation/discussions/673).
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes#9043
Fixes https://github.com/google/blockly-samples/issues/2512
### Proposed Changes
This replaces using BlockSvg's own ID for focus management since that's not guaranteed to be unique across all workspaces on the page.
### Reason for Changes
Both https://github.com/google/blockly-samples/issues/2512 covers the user-facing issue in more detail, but from a technical perspective it's possible for blocks to share IDs across workspaces. One easy demonstration of this is the flyout: the first block created from the flyout to the main workspace will share an ID. The workspace minimap plugin just makes the underlying problem more obvious.
The reason this introduces a breakage is due to the inherent ordering that `FocusManager` uses when trying to find a matching tree for a given DOM element that has received focus. These trees are iterated in the order of their registration, so it's quite possible for some cases (like main workspace vs. flyout) to resolve such that the behavior looks correct to users, vs. others (such as the workspace minimap) not behaving as expected.
Guaranteeing ID uniqueness across all workspaces fixes the problem entirely.
### Test Coverage
This has been manually tested in core Blockly's simple test playground and in Blockly samples' workspace minimap plugin test environment (linked against this change). See the new behavior for the minimap plugin:
[Screen recording 2025-05-13 4.31.31 PM.webm](https://github.com/user-attachments/assets/d2ec3621-6e86-4932-ae85-333b0e7015e1)
Note that this is a regression to v11 behavior in that the blocks in the minimap now show as selected.
This has been verified as working with the latest version of the keyboard navigation plugin (tip-of-tree). Keyboard-based block operations and movement seem to work as expected.
For automated testing this is expected to largely be covered by future tests added as part of resolving #8915.
### Documentation
No public documentation changes should be needed, though `IFocusableNode`'s documentation has been refined to be clearer on the uniqueness property for focusable element IDs.
### Additional Information
There's a separate open design question here about whether `BlockSvg`'s descendants should use the new focus ID vs. the block ID. Here is what I consider to be the trade-off analysis in this decision:
| | Pros | Cons |
|------------------------|-------------------------------------------------|------------------------------------------------------------------------------|
| Use `BlockSvg.id` | Can use fast `WorkspaceSvg.getBlockById`. | `WorkspaceSvg.lookUpFocusableNode` now uses 2 different IDs. |
| Use `BlockSvg.focusId` | Consistency in IDs use for block-related focus. | Requires more expensive block look-up in `WorkspaceSvg.lookUpFocusableNode`. |
* 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#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.
* docs: Make JSDoc @deprecated usage more consistent
* refactor(VariableMap)!: Refresh toolbox when map modified
Delete the following methods from WorkspaceSvg:
- renameVariableById
- deleteVariableById
- createVariable
Modify the following methods on VariableMap to call
this.workspace.refreshToolboxSelection() if this.workspace
is a WorkspaceSvg, replicating the behaviour of the
aforementioned deleted methods and additionally ensuring
that that method is called following any change to the
variable map:
- renameVariable
- changeVariableType
- renameVariableAndUses
- createVariable
- addVariable
- deleteVariable
BREAKING CHANGE:
This change ensures that the toolbox will be refreshed regardless
of what route the VaribleMap was updated, rather than only being
refreshed when it is updated via calls to methods on WorkspaceSvg.
Overall this is much more likely to fix a bug (where the toolbox
wasn't being refreshed when it should have been) than cause one
(by refreshing the toolbox when it shouldn't be), but this is
still a behaviour change which could _conceivably_ result an
unexpected regression.
* refactor(VariableMap): Remove calls to deprecated getVariableUsesById
Also refactor to use named imports core/variables.ts methods.
* refactor(Workspace): Use named imports for core/variables.ts methods
* refactor(FieldVariable): Remove call to deprecated getVariablesOfType
* refactor(variables): Remove calls to deprecated methods
* refactor(variables_dynamic): Remove call to deprecated getAllVariables
* refactor(xml): Remove calls to deprecated createVariable
* refactor(Events.VarCreate): Remove calls to deprecated methods
* refactor(Events.VarDelete): Remove calls to deprecated methods
* refactor(Events.VarRename): Remove calls to deprecated methods
_Note: This is a roll forward of #8916 that was reverted in #8933. See Additional Information below._
## The basics
- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)
## The details
### Resolves
Fixes#8913Fixes#8914
Fixes part of #8771
### Proposed Changes
This updates `WorkspaceSvg` and `BlockSvg` to be focusable, that is, it makes the workspace a `IFocusableTree` and blocks `IFocusableNode`s.
Some important details:
- While this introduces focusable tree support for `Workspace` it doesn't include two other components that are obviously needed by the keyboard navigation plugin's playground: fields and connections. These will be introduced in subsequent PRs.
- Blocks are set up to automatically synchronize their selection state with their focus state. This will eventually help to replace `LineCursor`'s responsibility for managing selection state itself.
- The tabindex property for the workspace and its ARIA label have been moved down to the `.blocklyWorkspace` element itself rather than its wrapper. This helps address some tab stop issues that are already addressed in the plugin (via monkey patches), but also to ensure that the workspace's main SVG group interacts correctly with `FocusManager`.
- `WorkspaceSvg` is being initially set up to default to its first top block when being focused for the first time. This is to match parity with the keyboard navigation plugin, however the latter also has functionality for defaulting to a position when no blocks are present. It's not clear how to actually support this under the new focus-based system (without adding an ephemeral element on which to focus), or if it's even necessary (since the workspace root can hold focus).
- `css.ts` was updated to remove `blocklyActiveFocus` and `blocklyPassiveFocus` since these have unintended highlighting consequences that aren't actually desirable yet. Instead, the exact styling for active/passive focus will be iterated in the keyboard navigation plugin project and moved to Core once finalized.
### Reason for Changes
This is part of an ongoing effort to ensure key components of Blockly are focusable so that they can be keyboard-navigable (with other needed changes yet both in Core Blockly and the keyboard navigation plugin).
### Test Coverage
No new tests have been added. It's certainly possible to add unit tests for the focusable configurations being introduced in this PR, but it may not be highly beneficial. It's largely assumed that the individual implementations should work due to a highly tested FocusManager, and it may be the case that the interactions of the components working together is far more important to verify (that is, the end user flows). The latter is planned to be tackled as part of #8915.
### Documentation
No documentation changes should be needed here.
### Additional Information
This includes changes that have been pulled from #8875.
This was originally merged in #8916 but was reverted in #8933 due to https://github.com/google/blockly-keyboard-experimentation/issues/481. This actually contains no differences from the original PR except for `css.ts` which are documented above. It does employ a new merge strategy: all of the necessary PRs to move both Core and the plugin over to using `FocusManager` will be staged and merged in quick succession as ensuring the plugin works for each constituent change (vs. the final one) is quite complex. Thus, this PR *does* break the plugin, and won't be merged until its subsequent PRs are approved and also ready for merging.
Edit: See https://github.com/google/blockly/pull/8938#issuecomment-2843589525 for why this actually is being merged a bit sooner than originally planned. Keeping the original reasoning above for context.
* test(BlockSvg): Add tests for setParent(null) when dragging
Add tests for scenarios where block(s) unrelated to the block
being disconnected has/have been marked as as being dragged.
Due to a bug in BlockSvg.prototype.setParent, one of these
fails in the case that the dragging block is not a top
block.
Also add a check to assertNonParentAndOrphan to check that the
orphan block's SVG root's parent is the workspace's canvass
(i.e., that orphan is a top-level block in the DOM too).
* fix(BlockSvg): Fix bug in setParent re: dragging block
Fix an incorrect assumption in setParent: the topmost block
whose root SVG element has the blocklyDragging class may not
actually be a top-level block.
* refactor(BlockDragStrategy): Hide connection preview earlier
* chore(BlockDragStrategy): prefer ?. to !.
Per nit on PR #8934.
* fix(BlockSvg): Spelling: "canvass" -> "canvas"
* 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#8913Fixes#8914
Fixes part of #8771
### Proposed Changes
This updates `WorkspaceSvg` and `BlockSvg` to be focusable, that is, it makes the workspace a `IFocusableTree` and blocks `IFocusableNode`s.
Some important details:
- While this introduces focusable tree support for `Workspace` it doesn't include two other components that are obviously needed by the keyboard navigation plugin's playground: fields and connections. These will be introduced in subsequent PRs.
- Blocks are set up to automatically synchronize their selection state with their focus state. This will eventually help to replace `LineCursor`'s responsibility for managing selection state itself.
- The tabindex property for the workspace and its ARIA label have been moved down to the `.blocklyWorkspace` element itself rather than its wrapper. This helps address some tab stop issues that are already addressed in the plugin (via monkey patches), but also to ensure that the workspace's main SVG group interacts correctly with `FocusManager`.
- `WorkspaceSvg` is being initially set up to default to its first top block when being focused for the first time. This is to match parity with the keyboard navigation plugin, however the latter also has functionality for defaulting to a position when no blocks are present. It's not clear how to actually support this under the new focus-based system (without adding an ephemeral element on which to focus), or if it's even necessary (since the workspace root can hold focus).
### 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.
* Fix: Remove the collapsed block warning when expanding a block
* Add tests and make warnings appear as soon as a block collapses
* Combine if checks to simplify code
