Fix: Revert focus prs #8932
Closed
Fix: Revert focus prs #8932
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chore: Merge develop into v12
…PiFoundation#8231) * feat: Add support for preserving block comment locations. * chore: format the tests.
chore: merge develop into v12
chore: merge develop into rc/v12
…berryPiFoundation#8291) (RaspberryPiFoundation#8302) * feat!: Add a blocklyFieldText CSS class to fields' text elements (RaspberryPiFoundation#8291) * add class instead of replace Co-authored-by: Beka Westberg <bwestberg@google.com> --------- Co-authored-by: Beka Westberg <bwestberg@google.com>
…erryPiFoundation#8337) * refactor: Add `addClass` and `removeClass` methods to blockSvg * fix: lint * fix: jsdoc
…iFoundation#8369) * feat: add the IVariableMap and IVariableModel interfaces. * chore: add license headers.
RaspberryPiFoundation#8357) * fix!: RaspberryPiFoundation#8345 rename css class This commit renames the blocklyTreeRow CSS class to blocklyToolboxCategory * Update category.ts * fix: css class conflicts Rename original blocklyToolboxCategory to blocklyToolboxCategoryContainer
…undation#8381) * refactor: make VariableModel implement IVariableModel. * chore: assauge the linter.
* feat: Add blockShadow class * formatted the file
…aspberryPiFoundation#8347 (RaspberryPiFoundation#8367) * renamed blocklyTreeIcon Css class to blocklyToolboxCategoryIcon * fix!: renamed blocklyTreeIcon Css class to blocklyToolboxCategoryIcon RaspberryPiFoundation#8347 * fixed whitespace formatting
…rryPiFoundation#8339) * fix!: Replace Closure UI CSS classes with Blockly CSS classes * chore: remove comments about deprecated goog-x class * chore: remove deprecated goog-x classes * fix: correct coding format to pass CI checks
* feat: Add blocklyTrashcanFlyout class * Fixed formatting issues * fix: versioning reverted to original * fix: prettier version resolved * fix: clean installation
…ion#8377) * fix: override `jsonInit` method to add css classes * fix: lint * refactor: simplify logic
chore: develop into V12 to pin node version
…tion#8395) * refactor: make VariableMap implement IVariableMap. * chore: remove unused arrayUtils import. * chore: fix comment on variable map backing store. * chore: Added JSDoc to new VariableMap methods. * chore: Improve test descriptions.
…undation#8400) * refactor: Use IVariableModel methods instead of directly accessing properties. * refactor: replace references to VariableModel with IVariableModel.
…tion#8401) * refactor: use IVariableMap in place of VariableMap. * refactor!: move variable deletion prompting out of VariableMap. * chore: Remove unused imports.
…tions (RaspberryPiFoundation#8882) * feat!: deprecate scopeType and include focusedNode in context menu options * chore: add issue to todo
…undation#8889) * feat: Allow for HTML elements in dropdown field menus. * refactor: Use dot access.
* feat: show context menu for connections * fix: update after rebase
…ion#8904) * chore: Add messages from the keyboard experiment. * fix: Resolve duplicate message ID. * fix: Use placeholders for keyboard shortcuts.
* fix: Add some missing message strings. * fix: Prefix messages with SHORTCUTS
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 RaspberryPiFoundation#8875 and are required for later PRs that will introduce IFocusableNode and IFocusableTree implementations.
These were needed in previous versions of plugin changes, but aren't anymore.
This reverts commit 404c20e.
…yPiFoundation#8896) * feat: Allow resetting alert/prompt/confirm to defaults. * chore: Add unit tests for Blockly.dialog. * fix: Removed TEST_ONLY hack from Blockly.dialog. * feat: Add a default toast notification implementation. * feat: Add support for toasts to Blockly.dialog. * chore: Add tests for default toast implementation. * chore: Fix docstring. * refactor: Use default arguments for dialog functions. * refactor: Add 'close' to the list of messages. * chore: Add new message in several other places. * chore: clarify docstrings. * feat: Make toast assertiveness configurable.
Addresses a review comment.
…us-manager-callbacks-and-improvements feat!: Introduce new focus tree/node functions.
…#8916) ## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes RaspberryPiFoundation#8913 Fixes RaspberryPiFoundation#8914 Fixes part of RaspberryPiFoundation#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 RaspberryPiFoundation#8915. ### Documentation No documentation changes should be needed here. ### Additional Information This includes changes that have been pulled from RaspberryPiFoundation#8875.
## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes RaspberryPiFoundation#8918 Fixes RaspberryPiFoundation#8919 Fixes part of RaspberryPiFoundation#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 RaspberryPiFoundation#8915. ### Documentation No documentation changes should be needed here. ### Additional Information This includes changes that have been pulled from RaspberryPiFoundation#8875.
…#8926) * fix: loop cursor when moving to prev node * chore: add loop tests for LineCursor prev and out * chore: fix out loop test for line cursor
…undation#8916)" This reverts commit d7680cf.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
32 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The basics
The details
Resolves
Fixes problems with focus and key handling introduced by #8916
Proposed Changes
Revert two PRs that merged a portion of the focus behavior into Core.
Reason for Changes
The changes broke some aspects of keyboard navigation being worked on in keyboard experimental
Test Coverage
Documentation
Additional Information