Commit Graph

34 Commits

Author SHA1 Message Date
Ben Henning
bb342f9644 feat: Make Flyout an ARIA list (experimental) (#9528)
## 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):

![Screen Recording 2025-12-11 at 2 50 30 PM](https://github.com/user-attachments/assets/24c4389f-73c7-4cb5-96ce-d9666841cdd8)

### 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.
2025-12-15 10:13:23 -08:00
Ben Henning
d0a79faf7d Merge branch 'develop' into merge-develop-into-experimental-branch 2025-11-04 18:47:23 +00:00
Ben Henning
76c734598b fix: Toolbox & Flyout ARIA positions (experimental) (#9394)
## 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.
2025-10-01 15:52:07 -07:00
Maribeth Moffatt
059cd249d2 feat: make FlyoutButton.callbackKey public (#9381) 2025-09-23 13:42:50 -07:00
Aaron Dodson
d349a0c0b0 fix: Improve representation of FlyoutButtons in screenreaders. (#9356)
* fix: Improve representation of `FlyoutButtons` in screenreaders.

* fix: Remove aria-selected.
2025-09-10 11:44:08 -07:00
Maribeth Moffatt
41b7e9399e chore: merge develop into add-screen-reader
chore: merge develop into add-screen-reader
Merge pull request #9352 from google/develop
2025-09-09 09:37:54 -07:00
Aaron Dodson
47307a9e53 refactor: Make focusable elements responsible for scrolling themselves into bounds. (#9288)
* 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.
2025-08-28 11:28:40 -07:00
Ben Henning
d0ad9343f0 feat: Add initial support for screen readers (experimental) (#9280)
## 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).
2025-08-06 15:28:45 -07:00
Ben Henning
3cbca8e4b6 feat: Automatically manage focus tree tab indexes (#9079)
## The basics

- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)

## The details
### Resolves

Fixes #8965
Fixes #8978
Fixes #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.
2025-05-29 12:09:59 -07:00
Aaron Dodson
ae22165cbe refactor: Remove INavigable in favor of IFocusableNode. (#9037)
* refactor: Remove INavigable in favor of IFocusableNode.

* chore: Fix JSDoc.

* chore: Address review feedback.
2025-05-13 15:04:49 -07:00
Aaron Dodson
a1be83bad8 refactor: Make INavigable extend IFocusableNode. (#9033) 2025-05-12 15:46:27 -07:00
Ben Henning
4074cee31b feat!: Make everything ISelectable focusable (#9004)
* 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.
2025-05-09 08:16:14 -07:00
Rachel Fenichel
92cad53cfe fix!: delete IASTNodeLocation and friends (#9015) 2025-05-08 12:47:39 -07:00
Rachel Fenichel
bb76d6e12c fix!: remove MarkerSvg and uses (#8991)
* fix: delete MarkerSvg (marker drawer)

* fix: delete marker and cursor SVG elements

* chore: format

* chore: lint
2025-05-07 09:28:51 -07:00
Aaron Dodson
acdad98653 refactor!: Use navigation rulesets instead of ASTNode to control keyboard navigation. (#8992)
* 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.
2025-05-07 08:47:52 -07:00
Ben Henning
cac8f0116c feat: Make toolbox and flyout focusable (roll forward) (#8939)
_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 #8918
Fixes #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.
2025-04-30 15:49:29 -07:00
Aaron Dodson
9fcd5a3037 release: Merge branch 'rc/v11.2.0' into rc/v12.0.0 2024-12-04 12:06:12 -08:00
Aaron Dodson
489aded31d feat: Add inflaters for flyout labels and buttons. (#8593)
* feat: Add inflaters for flyout labels and buttons.

* chore: Temporarily re-add createDom().

* chore: fix JSDoc.

* chore: Add license.

* chore: Add TSDoc.
2024-09-27 13:22:36 -07:00
Aaron Dodson
14d119b204 fix: improve prompting when deleting variables (#8529)
* fix: improve variable deletion behaviors.

* fix: don't prompt about deletion of only 1 variable block when triggered programmatically.

* fix: include the triggering block in the count of referencing blocks

* fix: only count the triggering block as a referencing block if it's not in the flyout
2024-08-19 15:47:00 -07:00
Christopher Allen
ce22f42868 chore: Organise imports (#8527)
* 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
2024-08-15 03:16:14 +01:00
Maribeth Moffatt
7c6fbec23c Merge branch 'osd' into merge-osd 2024-05-23 10:59:53 -07:00
Mike Harvey
9e05d69d2a feat: support keyboard navigation of flyout buttons (#7852)
* feat: support keyboard navigation of flyout buttons

* fix: use FlyoutItem type for flyout contents, rework navigateBetweenStacks for flyouts
2024-04-02 13:40:58 -07:00
Sayali Kandarkar
e4d8f160aa fix: removed underscore from isLabel_ in flyout_button.ts (#7533)
* Removed underscore from isLabel_ in flyout_button.ts

* chore: rename isLabel_ to isFlyoutLabel in flyout_button.ts

* fix: format

---------

Co-authored-by: sayali-kandarkar <skandark@andrew.cmu.edu>
2023-09-22 15:32:39 -07:00
Christopher Allen
b0a7c004a9 refactor(build): Delete Closure Library (#7415)
* fix(build): Restore erroneously-deleted filter function

  This was deleted in PR #7406 as it was mainly being used to
  filter core/ vs. test/mocha/ deps into separate deps files -
  but it turns out also to be used for filtering error
  messages too.  Oops.

* refactor(tests): Migrate advanced compilation test to ES Modules

* refactor(build): Migrate main.js to TypeScript

  This turns out to be pretty straight forward, even if it would
  cause crashing if one actually tried to import this module
  instead of just feeding it to Closure Compiler.

* chore(build): Remove goog.declareModuleId calls

  Replace goog.declareModuleId calls with a comment recording the
  former module ID for posterity (or at least until we decide
  how to reformat the renamings file.

* chore(tests): Delete closure/goog/*

  For the moment we still need something to serve as base.js for
  the benefit of closure-make-deps, so we keep a vestigial
  base.js around, containing only the @provideGoog declaration.

* refactor(build): Remove vestigial base.js

  By changing slightly the command line arguments to
  closure-make-deps and closure-calculate-chunks the need to have
  any base.js is eliminated.

* chore: Typo fix for PR #7415
2023-08-31 00:24:47 +01:00
dependabot[bot]
2546b01d70 chore(deps): Bump prettier from 2.8.8 to 3.0.0 (#7322)
* chore(deps): Bump prettier from 2.8.8 to 3.0.0

Bumps [prettier](https://github.com/prettier/prettier) from 2.8.8 to 3.0.0.
- [Release notes](https://github.com/prettier/prettier/releases)
- [Changelog](https://github.com/prettier/prettier/blob/main/CHANGELOG.md)
- [Commits](https://github.com/prettier/prettier/compare/2.8.8...3.0.0)

---
updated-dependencies:
- dependency-name: prettier
  dependency-type: direct:development
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

* chore: Reformat using Prettier v3.0 defaults

The main change is to add trailing commas to the last line of
block-formatted function calls.

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Christopher Allen <cpcallen+git@google.com>
2023-07-25 14:56:10 +00:00
Maribeth Bottorff
88ff901a72 chore: use prettier instead of clang-format (#7014)
* chore: add and configure prettier

* chore: remove clang-format

* chore: remove clang-format config

* chore: lint additional ts files

* chore: fix lint errors in blocks

* chore: add prettier-ignore where needed

* chore: ignore js blocks when formatting

* chore: fix playground html syntax

* chore: fix yaml spacing from merge

* chore: convert text blocks to use arrow functions

* chore: format everything with prettier

* chore: fix lint unused imports in blocks
2023-05-10 16:01:39 -07:00
Rachel Fenichel
48bdeb3bdb chore: remove underscores from private properties and methods for flyout (#6960)
* chore: remove underscores in flyout files

* chore: remove underscores in flyout_button
2023-04-07 15:40:56 -07:00
Neil Fraser
42fde0f81b chore: Reduce delta on ports to blockly-samples (#6886)
* Reduce usage of obsolete .keyCode property.
* Rename private properties/methods which violate eslint rules.
* Use arrays of bound events rather than individual properties.
* Improve typing info.
* Also fix O(n^2) recursive performance issue in theme's getComponentStyle function.
* And replace String(...) with '${...}' (smaller, faster).
* .toString() is considered harmful.
2023-03-15 13:28:57 -07:00
Blake Thomas Williams
7439a70864 feat: updated flyout button to set border radius via static variable (#6838) 2023-02-10 20:17:29 +01:00
Rachel Fenichel
1d1a927628 chore: remove alias comments (#6816)
* chore: remove alias comments

* chore: format

* chore: remove extra newlines

* chore: fix bad replaces
2023-02-06 10:08:55 -08:00
Aaron Dodson
90cb965e7c refactor: Migrate to PointerEvents (#6598)
* refactor: Remove checks for PointerEvent support.

* refactor: Deprecate and remove calls to splitEventByTouches.

* refactor: Deprecate and remove calls to setClientFromTouch().

* refactor: Use PointerEvent in place of Event/MouseEvent/TouchEvent/PseudoEvent.

* refactor: Update references to mouse/touch events in code and documentation to reference pointer events.

* refactor: Merge Gesture and TouchGesture

* chore: clang-format changed files

* refactor: Bind and expect PointerEvents instead of MouseEvents.

* refactor: Rename TouchGesture to Gesture.

* fix: Fix test failures.

* chore: clang-format changed files.

* fix: Fix errant _ from merging

* refactor: Clean up dead code in browser_events.ts.

* chore: Update version in deprecation notices to reflect release schedule

* fix: Fixed a bug that caused the browser context menu to not be suppressed in Chrome.

* fix: Re-export Gesture as TouchGesture for backwards compatibility.

* refactor: Deprecate and remove uses of opt_noPreventDefault.

* chore: Fix error message in gesture.ts.

* chore: Removed obsolete todo.
2022-12-05 11:27:52 -08:00
Maribeth Bottorff
037eb59b89 chore: Lint TsDoc. (#6353)
* chore: add linting for tsdoc

* chore: don't require types on return

* chore: remove redundant fileoverview from ts

* chore: change return to returns and add some newlines

* chore: remove license tag

* chore: don't require params/return docs

* chore: remove spurious struct tags

* Revert "chore: change return to returns and add some newlines"

This reverts commit d6d8656a45.

* chore: don't auto-add param names

* chore: disable require-param bc it breaks on this

* return to returns and add line breaks

* chore: configure additional jsdoc rules

* chore: run format

* Revert "chore: remove license tag"

This reverts commit 173455588a.

* chore: allow license tag format

* chore: only require jsdoc on exported items

* chore: add missing jsdoc or silence where needed

* chore: run format

* chore: lint fixes
2022-08-23 14:27:22 -07:00
Beka Westberg
21d90696d1 chore: Migrate core/ to Typescript, actually (#6299)
* fix: convert files to typescript

* fix: add alias for AnyDuringMigration so that tsc will run

* chore: format

* chore: enable ts for the clang-format workflow (#6233)

* chore: Restore @fileoverview comment locations (#6237)

* chore: add declareModuleId (#6238)

* fix: Revert comment change to app_controller.js (#6241)

* fix: Add missing import goog statements (#6240)

I've added the import statement immediately before the
goog.declareModuleId calls that depend on it.

There is an argument to be made that we should put the import
statement in their normal place amongst any other imports, and
move the declareModuleId statement to below the double blank
line below the imports, but as these are so tightly coupled,
replace the previous goog.module calls, and will both be deleted
at the same time once the transition to TypeScript is fully complete
I think it's fine (and certainly much easier) to do it this way.

* chore: Fix whitespace (#6243)

* fix: Remove spurious blank lines

  Remove extraneous blank lines introduced by deletion of
  'use strict'; pragmas.

  Also fix the location of the goog.declareModuleId call in
  core/utils/array.ts.

* fix: Add missing double-blank-line before body of modules

  Our convention is to have two blank lines between the imports (or
  module ID, if there are no imports) and the beginning of the body
  of the module.  Enforce this.

* fix: one addition format error for PR #6243

* fix(build): Skip npm prepare when running in CI (#6244)

Have npm prepare do nothing when running in CI.

We don't need to do any building, because npm test will build
everything needed in the workflows in which it is run, and we
don't want to build anything in other workflows because a tsc
error would prevent those workflows from completing.

* fix: re-add `@package` annotations as `@internal` annotations (#6232)

* fix: add ~70% of internal attributes

* fix: work on manually adding more @internal annotations

* fix: add more manual internal annotations

* fix: rename package typos to internal

* fix: final manual fixes for internal annotations

* chore: format

* chore: make unnecessary multiline jsdoc a single line

* fix: fix internal tags in serialization exceptions

* fix: tsc errors picked up from develop (#6224)

* fix: relative path for deprecation utils

* fix: checking if properties exist in svg_math

* fix: set all timeout PIDs to AnyDuringMigration

* fix: make nullability errors explicity in block drag surface

* fix: make null check in events_block_change explicit

* fix: make getEventWorkspace_ internal so we can access it from CommentCreateDeleteHelper

* fix: rename DIV -> containerDiv in tooltip

* fix: ignore backwards compat check in category

* fix: set block styles to AnyDuringMigration

* fix: type typo in KeyboardShortcut

* fix: constants name in row measurables

* fix: typecast in mutator

* fix: populateProcedures type of flattened array

* fix: ignore errors related to workspace comment deserialization

* chore: format files

* fix: renaming imports missing file extensions

* fix: remove check for sound.play

* fix: temporarily remove bad requireType.

All `export type` statements are stripped when tsc is run. This means
that when we attempt to require BlockDefinition from the block files, we
get an error because it does not exist.

We decided to temporarily remove the require, because this will no
longer be a problem when we conver the blocks to typescript, and
everything gets compiled together.

* fix: bad jsdoc in array

* fix: silence missing property errors

Closure was complaining about inexistant properties, but they actually
do exist, they're just not being transpiled by tsc in a way that closure
understands.

I.E. if things are initialized in a function called by the constructor,
rather than in a class field or in the custructor itself, closure would
error.

It would also error on enums, because they are transpiled to a weird
IIFE.

* fix: context menu action handler not knowing the type of this.

this: TypeX information gets stripped when tsc is run, so closure could
not know that this was not global. Fixed this by reorganizing to use the
option object directly instead of passing it to onAction to be bound to
this.

* fix: readd getDeveloperVars checks (should not be part of migration)

This was found because ALL_DEVELOPER_VARS_WARNINGS_BY_BLOCK_TYPE was no
longer being accessed.

* fix: silence closure errors about overriding supertype props

We propertly define the overrides in typescript, but these get removed
from the compiled output, so closure doesn't know they exist.

* fix: silence globalThis errors

this: TypeX annotations get stripped from the compiled output, so
closure can't know that we're accessing the correct things. However,
typescript makes sure that this always has the correct properties, so
silencing this should be fine.

* fix: bad jsdoc name

* chore: attempt compiling with blockly.js

* fix: attempt moving the import statement above the namespace line

* chore: add todo comments to block def files

* chore: remove todo from context menu

* chore: add comments abotu disabled errors

* chore: move comments back to their correct positions (#6249)

* fix: work on fixing comments

* chore: finish moving all comments

* chore: format

* chore: move some other messed up comments

* chore: format

* fix: Correct enum formatting, use merged `namespace`s for types that are class static members (#6246)

* fix: formatting of enum KeyCodes

* fix: Use merged namespace for ContextMenuRegistry static types

  - Create a namespace to be merged with the ContextMenuRegistry
    class containing the types that were formerly declared as static
    properties on that class.

  - Use type aliases to export them individually as well, for
    compatibility with the changes made by MigranTS (and/or
    @gonfunko) to how other modules in core/ now import these
    types.

  - Update renamings.json5 to reflect the availability of the
    direct exports for modules that import this module directly
    (though they are not available to, and will not be used by,
    code that imports only via blockly.js/blockly.ts.)

* fix: Use merged namespace for Input.Align

  - Create a merged namespace for the Input.Align enum.

  - Use type/const aliases to export it as Input too.

  - Update renamings.json5 to reflect the availability of the
    direct export.

* fix: Use merged namespace for Names.NameType

  - Create a merged namespace for the Names.NameType enum.

  - Use type/const aliases to export it as NameType too.

  - Update renamings.json5 to reflect the availability of the
    direct export.  (This ought to have happened in an earlier
    version as it was already available by both routes.)

* chore: Fix minor issues for PR #6246

  - Use `Align` instead of `Input.Align` where possible.

* fix(build): Suppress irrelevant JSC_UNUSED_LOCAL_ASSIGNMENT errors

  tsc generates code for merged namespaces that looks like:

      (function (ClassName) {
          let EnumName;
          (function (EnumName) {
              EnumName[EnumNameAlign["v1"] = 0] = "v1";
              // etc.
          })(EnumName = ClassName.EnumName || (ClassName.EnumName = {}));
      })(ClassName || (ClassName = {}));

  and Closure Compiler complains about the fact that the EnumName let
  binding is initialised but never used.  (It exists so that any other
  code that was in the namespace could see the enum.)

  Suppress this message, since it is not actionable and lint and/or tsc
  should tell us if we have actual unused variables in our .ts files.

* chore(build): Suppress spurious warnings from closure-make-deps (#6253)

A little bit of an ugly hack, but it works: pipe stderr through
grep -v to suppress error output starting with "WARNING in".

* fix: remaining enums that weren't properly exported (#6251)

* fix: remaining enums that weren't properly exported

* chore: format

* fix: add enum value exports

* chore: format

* fix: properly export interfaces that were typedefs (#6250)

* fix: properly export interfaces that were typedefs

* fix: allowCollsion -> allowCollision

* fix: convert unconverted enums

* fix: enums that were/are instance properties

* fix: revert changes to property enums

* fix: renamed protected parameter properties (#6252)

* fix: bad protected parameter properties

* chore:format

* fix: gesture constructor

* fix: overridden properties that were renamed

* refactor: Migrate `blockly.js` to TypeScript (#6261)

* chore: Apply changes to blockly.js to blockly.ts

* fix: Build using core/blockly.ts instead of .js

  Compiles and runs in compressed mode correctly!

* fix(build): Don't depend on execSync running bash (#6262)

For some reason on Github CI servers execSync uses /bin/sh, which
is (on Ubuntu) dash rather than bash, and does not understand
the pipefail option.

So remove the grep pipe on stderr and just discard all error output
at all.

This is not ideal as errors in test deps will go unreported AND
not even cause test failure, but it's not clear that it's worth
investing more time to fix this at the moment.

* chore: use `import type` where possible (#6279)

* chore: automatically change imports to import types

* chore: revert changes that actually need to be imports

* chore: format

* chore: add more import type statements based on importsNotUsedAsValues

* chore: fix tsconfig

* chore: add link to compiler issue

* fix: add type information to blockly options (#6283)

* fix: add type information to blockly options

* chore: format

* chore: remove erroneous comment

* fix: bugs revealed by getting the built output working (#6282)

* fix: types of compose and decompose in block

* fix: workspace naming in toolbox

* chore: add jsdoc

* chore: restore registry comments to better positions

* chore: pr comments'

* fix(variables): Revert inadvertent change to allDeveloperVariables (#6290)

It appears that a function call got modified incorrectly (probably
in an effort to fix a typing issue).  This fix trivially reverts
the line in question to match the original JS version from develop.

This causes the generator tests to pass.

* fix: circular dependencies (#6281)

* chore: fix circular dependencies w/ static workspace funcs

* remove preserved imports that aren't currently necessary (probably)

* fix circular dependency with workspaces and block using stub

* fix dependency between variables and xml by moving function to utils

* add stub for trashcan as well

* fix line endings from rebase

* fix goog/base order

* add trashcan patch

* fix: types of compose and decompose in block

* fix: workspace naming in toolbox

* chore: add jsdoc

* chore: restore registry comments to better positions

* chore: remove implementations in goog.js

* chore: fix types of stubs

* chore: remove added AnyDuringMigration casts

* chore: remove modifications to xml and variables

* chore: format

* chore: remove event requirements in workspace comments

* chore: fix circular dependency with xml and workspace comments

* fixup remove ContextMenu import

* chore: fix dependency between mutator and workspace

* chore: break circular dependency between names and procedures

* chore: get tests to run?

* chore: pr comments'

* chore: fix stubbing field registry fromJson

* chore: fix spying on fire

* chore: fix stubbing parts of connection checker

* chore: fix stubbing dialog

* chore: fix stubbing style

* chore: fix spying on duplicate

* chore: fix stubbing variables

* chore: fix stubbing copy

* chore: fix stubbing in workspace

* chore: remove unnecessary stubs

* chore: fix formatting

* chore: fix other formatting

* chore: add backwards compatible static properties to workspace

* chore: move static type properties

* chore: move and comment stubs

* chore: add newlines at EOF

* chore: improve errors for monkey patched functions

* chore: update comment with a pointer to the doc

* chore: update comment with a pointer to the doc

* chore: format

* chore: revert changes to playground used for testing (#6292)

* chore: get mocha tests to pass. (#6291)

* chore: fix undo and empty code blocks

* chore: skip IE test

* chore: fix gesture test

* chore: fix replace message references test

* chore: fix string table interpolation

* chore: skip getById tests

* chore: fix field tests

* chore: fix console errors by making workspace nullable

* chore: format

* chore: fix definition overwrite warning

* chore: update metadata

* chore: temporarily modify the the advanced compile test

* chore: fix gestures by fixing test instead

Co-authored-by: Neil Fraser <fraser@google.com>
Co-authored-by: Christopher Allen <cpcallen+git@google.com>
2022-08-02 10:30:13 -07:00
Beka Westberg
27d6760ec6 chore: rename all JS files to TS files (#6297) 2022-08-01 13:09:51 -07:00