Repository navigation
fix(widgets): dismiss open RawTooltip on Escape key (WCAG 1.4.13) - #192966
auto-submit[bot] merged 4 commits into
Conversation
…p accessibility - Expose selected state (selected: highlight) on Autocomplete option items so screen readers announce the keyboard-highlighted option (b/464138739, b/464133587, b/464122713). - Wrap DrawerHeader in Semantics(header: true) so screen readers expose heading structure (b/462597561, b/462583677). - Register a HardwareKeyboard handler in RawTooltipState to dismiss open tooltips on LogicalKeyboardKey.escape without closing parent dialogs such as DatePickerDialog (WCAG 1.4.13 Dismissible, b/463905826). - Add ignorePointer to TooltipThemeData so applications can configure hoverable tooltips globally (TooltipThemeData(ignorePointer: false)) for WCAG 1.4.13 Hoverable compliance without regressing flutter#142465 (b/463913706).
|
This pull request contains changes to Material or Cupertino, which are currently frozen in this repository. Changes should be made in Please refer to #188444 for instructions. |
…ip in widgets Per flutter#188444, Material changes belong in packages/material_ui in flutter/packages during the design decoupling code freeze. Scope this PR strictly to RawTooltipState Escape key dismissal in packages/flutter/lib/src/widgets/raw_tooltip.dart.
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to dismiss open tooltips using the Escape key by registering a keyboard event handler in RawTooltipState, and adds a test to verify this behavior. The review feedback suggests refining the key event handler to consume all key events (down, repeat, up) for the Escape key, rather than only KeyDownEvent, to prevent these events from propagating and triggering unintended actions in the focus tree.
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Reason for revert: Breaks |
|
Ah snap, I missed that the OverlayEntry in the test is created inline, but it should be disposed, which explains the failure. |
|
Successfully created revert PR: #193183 |
….13) (flutter#193183) Reverts: [fix(widgets): dismiss open RawTooltip on Escape key (WCAG 1.4.13)](flutter#192966) Initiated by: @b-luk Reason for reverting: Breaks `Windows framework_tests_widgets_leak_tracking`. https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8669930694849114497/+/u/run_test.dart_for_framework_tests_shard_and_subshard_widgets/stdout ``` 01:23 +4311 ~114 -1: C:/b/s/w/ir/x/w/flutter/packages/flutter/test/widgets/raw_tooltip_test.dart: (tearDownAll) [E] Expected: leak free Actual: <Instance of 'Leaks'> Which: contains leaks: # The text is generated by leak_tracker. # For leak troubleshooting tips open: # https://github.com/flutter/flutter/blob/main/docs/contributing/testing/Leak-tracking.md notDisposed: total: 2 objects: ValueNotifier<_OverlayEntryWidgetState?>: test: Escape key dismisses hovered RawTooltip even when primaryFocus is on a sibling consuming Escape identityHashCode: 520722467 OverlayEntry: test: Escape key dismisses hovered RawTooltip even when primaryFocus is on a sibling consuming Escape identityHashCode: 418743465 package:matcher expect package:leak_tracker_testing/src/leak_testing.dart 69:24 LeakTesting.collectedLeaksReporter.<fn> package:leak_tracker_flutter_testing/src/testing.dart 74:37 maybeTearDownLeakTrackingForAll ===== asynchronous gap =========================== dart:async Zone.registerBinaryCallback package:flutter_test/src/test_compat.dart 302:3 _tearDownForTestFile ``` Original PR Author: @kevmoo Reviewed By: @navaronbracke The original PR description is provided below: ## Description Improves `RawTooltip` keyboard accessibility (`WCAG 1.4.13 Dismissible`, http://b/463905826): - **`RawTooltip` `Escape` key dismissal**: Registers a `HardwareKeyboard` handler in `RawTooltipState` that dismisses open tooltips on `LogicalKeyboardKey.escape` (`KeyDownEvent`) and consumes the key event when a tooltip is open, preventing `Escape` from prematurely closing parent modal dialogs (such as `DatePickerDialog`) while a tooltip is displayed. - *(Note: The accompanying Material changes for `Autocomplete`, `DrawerHeader`, and `TooltipThemeData.ignorePointer` will be submitted separately to `packages/material_ui` in `flutter/packages` per [flutter#188444](flutter#188444 Fixes http://b/463905826 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
….13)" (flutter#193192) Relands flutter#192966 (reverted in flutter#193183) with `OverlayEntry` disposal in `raw_tooltip_test.dart` and `FocusManager` early key-event interception in `raw_tooltip.dart`. ### What changed since flutter#192966 (`f8c85519a060`) 1. **Test `OverlayEntry` Leak Fix (`raw_tooltip_test.dart`)**: - In `raw_tooltip_test.dart` (`Escape key dismisses hovered RawTooltip even when primaryFocus is on a sibling consuming Escape`), `flutter#192966` passed an inline `OverlayEntry(...)` to `Overlay(initialEntries: <OverlayEntry>[...])` without calling `entry.remove()` and `entry.dispose()` in `addTearDown`. Because `Overlay` does not dispose `initialEntries` on unmount, the `OverlayEntry`'s internal `ValueNotifier<_OverlayEntryWidgetState?>` was flagged as `notDisposed` by `leak_tracker` in CI (`Linux framework_tests_widgets_leak_tracking`). - This reland captures `OverlayEntry? entry` and registers `addTearDown(() { entry?.remove(); entry?.dispose(); })`. Verified locally with `flutter test --dart-define=LEAK_TRACKING=true test/widgets/raw_tooltip_test.dart`. 2. **Stop `Escape` Propagation Before Focus-Tree Shortcuts (`raw_tooltip.dart`)**: - In `flutter#192966`, `RawTooltipState` registered `HardwareKeyboard.instance.addHandler(_handleKeyEvent)` alone. Because `KeyEventManager` runs `HardwareKeyboard` handlers *before* `FocusManager.handleKeyMessage` without stopping the focus-tree walk when returning `true`, emptying `RawTooltip._openedTooltips` in `HardwareKeyboard` caused `Escape` to still reach focused siblings (`siblingReceivedEscape`) or `WidgetsApp`'s `Shortcuts` (`DismissIntent` on enclosing `ModalRoute`s such as `DatePickerDialog`). - This reland registers a single static pair (`FocusManager.instance.addEarlyKeyEventHandler(_handleEarlyKeyEvent)` plus `HardwareKeyboard.instance.addHandler(_handleHardwareKeyEvent)` fallback when `primaryFocus == null`) when `RawTooltip._openedTooltips` transitions from empty to non-empty, and removes them when the last open tooltip closes or disposes. Returning `KeyEventResult.handled` from `_handleEarlyKeyEvent` stops `FocusManager` before walking the focus tree, and a second test (`Escape key dismisses open RawTooltip inside WidgetsApp without invoking ancestor DismissIntent`) verifies both cases. ### Original description Fixes http://b/463905826 WCAG 1.4.13 (Content on Hover or Focus) requires a mechanism to dismiss additional content triggered by pointer hover or keyboard focus (`Escape` key) without moving pointer hover or keyboard focus, and without unintended side effects such as dismissing an enclosing modal dialog (`DatePickerDialog`) when a tooltip is open. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] All existing and new tests are passing.
Description
Improves
RawTooltipkeyboard accessibility (WCAG 1.4.13 Dismissible, http://b/463905826):RawTooltipEscapekey dismissal: Registers aHardwareKeyboardhandler inRawTooltipStatethat dismisses open tooltips onLogicalKeyboardKey.escape(KeyDownEvent) and consumes the key event when a tooltip is open, preventingEscapefrom prematurely closing parent modal dialogs (such asDatePickerDialog) while a tooltip is displayed.Autocomplete,DrawerHeader, andTooltipThemeData.ignorePointerwill be submitted separately topackages/material_uiinflutter/packagesper #188444).Fixes http://b/463905826
Pre-launch Checklist
///).