Commit Graph

13 Commits

Author SHA1 Message Date
Ben Henning
bbd97eab67 fix: Synchronize gestures and focus (#8981)
## The basics

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

## The details
### Resolves

Fixes #8952
Fixes #8950
Fixes #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.
2025-05-05 10:29:20 -07:00
RoboErikG
b343a13bbe fix: Fixes #8764 by moving the event grouping calls up to dragger.ts (#8781) 2025-02-20 08:56:57 -08: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
Beka Westberg
6e4ba00be5 fix: dragging and disposing of shadows (#8172)
* Revert "fix: dragging blocks by shadows to delete (#8138)"

This reverts commit 3fd749205f.

* fix: dragging shadows
2024-05-28 10:28:16 -07:00
Beka Westberg
7c4b0bd703 fix: delete area animation (#8149)
* fix: delete area animation

* chore: format

* Update core/dragging/dragger.ts

Co-authored-by: Christopher Allen <cpcallen+github@gmail.com>

---------

Co-authored-by: Christopher Allen <cpcallen+github@gmail.com>
2024-05-20 09:58:48 -07:00
Beka Westberg
75d81620ab fix: undoing block delete over trashcan creates block in wrong place (#8126) 2024-05-17 12:26:10 -07:00
Beka Westberg
a0f821a239 fix: revert dragger name (#8020) 2024-04-11 13:22:17 -07:00
Beka Westberg
7491a50239 fix: compatibility with scroll plugin (#8016) 2024-04-08 13:17:03 -07:00
Beka Westberg
abfbbbc299 fix!: various drag-adjacent interfaces (#7975)
* fix: ISelectable and IDeletable interfaces

* fix: switch everything over to use new draggable interface

* fix: exports
2024-03-29 21:00:56 +00:00
Beka Westberg
da8a83b925 feat: have the gesture use a dragger for blocks (#7972)
* feat: have block use drag strategy

* fix: gesture to use dragger for blocks

* chore: register dragger

* chore: remove getInsertionMarkers and pull logic into workspace
2024-03-28 17:38:58 -07:00
Beka Westberg
da79a120f2 fix: remove parameter from wouldDelete (#7968) 2024-03-28 15:39:48 -07:00
Beka Westberg
6fe080bc6e fix: draggable interface and concrete dragger (#7967)
* fix: interface to match design

* fix: dragger implementation

* fix: rename moveToStart to revertDrag

* fixup
2024-03-28 15:11:30 -07:00
Christopher Allen
0588dfade8 feat(dragging): Introduce Dragger, implementing IDragger (#7956)
* feat(dragging): Introduce Dragger, implementing iDragger

* docs(dragging): JSDoc for Dragger's .startLoc
2024-03-22 19:03:57 +00:00