From deaf50b88d417fac28704a9f4cf52ef94595da9a Mon Sep 17 00:00:00 2001 From: Juan Tugores Date: Thu, 22 Feb 2024 11:46:19 -0800 Subject: [PATCH 1/9] Marks the flutter views as focusable. When a given flutter view is focused its tabindex will be -1 When a given flutter view is not focused its tabindex will be 0 --- .../lib/src/engine/platform_dispatcher.dart | 9 +- .../view_focus_binding.dart | 105 +++++---- .../engine/view_embedder/style_manager.dart | 5 + .../view_focus_binding_test.dart | 219 +++++++++--------- .../view_embedder/style_manager_test.dart | 18 ++ 5 files changed, 202 insertions(+), 154 deletions(-) diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index 277fe73018ade..d5f17fdedae8c 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -79,7 +79,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { _addLocaleChangedListener(); registerHotRestartListener(dispose); AppLifecycleState.instance.addListener(_setAppLifecycleState); - ViewFocusBinding.instance.addListener(invokeOnViewFocusChange); + _viewFocusBinding.init(); domDocument.body?.append(accessibilityPlaceholder); _onViewDisposedListener = viewManager.onViewDisposed.listen((_) { // Send a metrics changed event to the framework when a view is disposed. @@ -123,7 +123,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { _removeLocaleChangedListener(); HighContrastSupport.instance.removeListener(_updateHighContrast); AppLifecycleState.instance.removeListener(_setAppLifecycleState); - ViewFocusBinding.instance.removeListener(invokeOnViewFocusChange); + _viewFocusBinding.dispose(); accessibilityPlaceholder.remove(); _onViewDisposedListener.cancel(); viewManager.dispose(); @@ -228,6 +228,11 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { } } + late final ViewFocusBinding _viewFocusBinding = ViewFocusBinding( + viewManager: viewManager, + onViewFocusChange: invokeOnViewFocusChange, + ); + @override ui.ViewFocusChangeCallback? get onViewFocusChange => _onViewFocusChange; ui.ViewFocusChangeCallback? _onViewFocusChange; diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart b/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart index db0cd066da42b..0e2661d65a49a 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart @@ -2,47 +2,41 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; import 'package:ui/src/engine.dart'; import 'package:ui/ui.dart' as ui; /// Tracks the [FlutterView]s focus changes. final class ViewFocusBinding { /// Creates a [ViewFocusBinding] instance. - ViewFocusBinding._(); + ViewFocusBinding({ + required FlutterViewManager viewManager, + required ui.ViewFocusChangeCallback onViewFocusChange, + }): + _viewManager = viewManager, + _onViewFocusChange = onViewFocusChange; - /// The [ViewFocusBinding] singleton. - static final ViewFocusBinding instance = ViewFocusBinding._(); + final FlutterViewManager _viewManager; + final ui.ViewFocusChangeCallback _onViewFocusChange; - final List _listeners = []; int? _lastViewId; ui.ViewFocusDirection _viewFocusDirection = ui.ViewFocusDirection.forward; + StreamSubscription? _onViewCreatedListener; - /// Subscribes the [listener] to [ui.ViewFocusEvent] events. - void addListener(ui.ViewFocusChangeCallback listener) { - if (_listeners.isEmpty) { - domDocument.body?.addEventListener(_keyDown, _handleKeyDown); - domDocument.body?.addEventListener(_keyUp, _handleKeyUp); - domDocument.body?.addEventListener(_focusin, _handleFocusin); - domDocument.body?.addEventListener(_focusout, _handleFocusout); - } - _listeners.add(listener); - } - - /// Removes the [listener] from the [ui.ViewFocusEvent] events subscription. - void removeListener(ui.ViewFocusChangeCallback listener) { - _listeners.remove(listener); - if (_listeners.isEmpty) { - domDocument.body?.removeEventListener(_keyDown, _handleKeyDown); - domDocument.body?.removeEventListener(_keyUp, _handleKeyUp); - domDocument.body?.removeEventListener(_focusin, _handleFocusin); - domDocument.body?.removeEventListener(_focusout, _handleFocusout); - } + void init() { + domDocument.body?.addEventListener(_keyDown, _handleKeyDown); + domDocument.body?.addEventListener(_keyUp, _handleKeyUp); + domDocument.body?.addEventListener(_focusin, _handleFocusin); + domDocument.body?.addEventListener(_focusout, _handleFocusout); + _onViewCreatedListener = _viewManager.onViewCreated.listen(_markViewAsFocusable); } - void _notify(ui.ViewFocusEvent event) { - for (final ui.ViewFocusChangeCallback listener in _listeners) { - listener(event); - } + void dispose() { + domDocument.body?.removeEventListener(_keyDown, _handleKeyDown); + domDocument.body?.removeEventListener(_keyUp, _handleKeyUp); + domDocument.body?.removeEventListener(_focusin, _handleFocusin); + domDocument.body?.removeEventListener(_focusout, _handleFocusout); + _onViewCreatedListener?.cancel(); } late final DomEventListener _handleFocusin = createDomEventListener((DomEvent event) { @@ -67,36 +61,57 @@ final class ViewFocusBinding { }); void _handleFocusChange(DomElement? focusedElement) { + final int? lastViewId = _lastViewId; final int? viewId = _viewId(focusedElement); - if (viewId == _lastViewId) { + if (viewId == lastViewId) { return; } + final ui.ViewFocusEvent event; - if (viewId == null) { - event = ui.ViewFocusEvent( - viewId: _lastViewId!, - state: ui.ViewFocusState.unfocused, - direction: ui.ViewFocusDirection.undefined, - ); - } else { + if (viewId != null) { event = ui.ViewFocusEvent( viewId: viewId, state: ui.ViewFocusState.focused, direction: _viewFocusDirection, ); + } else { + event = ui.ViewFocusEvent( + viewId: lastViewId!, + state: ui.ViewFocusState.unfocused, + direction: ui.ViewFocusDirection.undefined, + ); } _lastViewId = viewId; - _notify(event); + _markViewAsFocusable(lastViewId); + _markViewAsFocusable(viewId, reachableByKeyboard: false); + _onViewFocusChange(event); } - static int? _viewId(DomElement? element) { - final DomElement? viewElement = element?.closest( - DomManager.flutterViewTagName, - ); - final String? viewIdAttribute = viewElement?.getAttribute( - GlobalHtmlAttributes.flutterViewIdAttributeName, - ); - return viewIdAttribute == null ? null : int.tryParse(viewIdAttribute); + int? _viewId(DomElement? element) { + final DomElement? viewElement = element?.closest(DomManager.flutterViewTagName); + for (final EngineFlutterView view in _viewManager.views) { + if (view.dom.rootElement == viewElement) { + return view.viewId; + } + } + return null; + } + + void _markViewAsFocusable( + int? viewId, { + bool reachableByKeyboard = true, + }) { + if (viewId == null) { + return; + } + // A tabindex with value zero means the DOM element can be reached by using + // the keyboard (tab, shift + tab). When its value is -1 it is still focusable + // but can't be focused by the result of keyboard events This is specially + // important when the semantics tree is enabled as it puts DOM nodes inside + // the flutter view and having it with a zero tabindex messes the focus + // traversal order when pressing tab or shift tab. + final int tabIndex = reachableByKeyboard ? 0 : -1; + _viewManager[viewId]?.dom.rootElement.setAttribute('tabindex', tabIndex); } static const String _focusin = 'focusin'; diff --git a/lib/web_ui/lib/src/engine/view_embedder/style_manager.dart b/lib/web_ui/lib/src/engine/view_embedder/style_manager.dart index 3cdff41224b97..5080b852c1d4a 100644 --- a/lib/web_ui/lib/src/engine/view_embedder/style_manager.dart +++ b/lib/web_ui/lib/src/engine/view_embedder/style_manager.dart @@ -116,6 +116,11 @@ void applyGlobalCssRulesToSheet( // Hide placeholder text '$cssSelectorPrefix .flt-text-editing::placeholder {' ' opacity: 0;' + '}' + + // Hide outline when the flutter-view root element is focused. + '$cssSelectorPrefix:focus {' + ' outline: none;' '}', ); diff --git a/lib/web_ui/test/engine/platform_dispatcher/view_focus_binding_test.dart b/lib/web_ui/test/engine/platform_dispatcher/view_focus_binding_test.dart index d220ed7eff19a..e5c43b0999bde 100644 --- a/lib/web_ui/test/engine/platform_dispatcher/view_focus_binding_test.dart +++ b/lib/web_ui/test/engine/platform_dispatcher/view_focus_binding_test.dart @@ -12,140 +12,145 @@ void main() { void testMain() { group(ViewFocusBinding, () { - late EnginePlatformDispatcher platformDispatcher; + late List dispatchedViewFocusEvents; + late EnginePlatformDispatcher dispatcher; setUp(() { - platformDispatcher = EnginePlatformDispatcher.instance; domDocument.activeElement?.blur(); + + dispatcher = EnginePlatformDispatcher.instance; + dispatchedViewFocusEvents = []; + dispatcher.onViewFocusChange = dispatchedViewFocusEvents.add; + }); + + test('The view is focusable and reachable by keyboard when registered', () async { + final EngineFlutterView view = createAndRegisterView(dispatcher); + + + // The root element should have a tabindex="0" to make the flutter view + // focusable and reachable by the keyboard. + expect(view.dom.rootElement.getAttribute('tabindex'), '0'); + }); + + test('The view is focusable but not reachable by keyboard when focused', () async { + final EngineFlutterView view = createAndRegisterView(dispatcher); + + view.dom.rootElement.focus(); + + // The root element should have a tabindex="-1" to make the flutter view + // focusable but not reachable by the keyboard. + expect(view.dom.rootElement.getAttribute('tabindex'), '-1'); + }); + + test('Correctly marks the focusable views as reachable by keyboard or not', () async { + final EngineFlutterView view1 = createAndRegisterView(dispatcher); + final EngineFlutterView view2 = createAndRegisterView(dispatcher); + + expect(view1.dom.rootElement.getAttribute('tabindex'), '0'); + expect(view2.dom.rootElement.getAttribute('tabindex'), '0'); + + view1.dom.rootElement.focus(); + expect(view1.dom.rootElement.getAttribute('tabindex'), '-1'); + expect(view2.dom.rootElement.getAttribute('tabindex'), '0'); + + view2.dom.rootElement.focus(); + expect(view1.dom.rootElement.getAttribute('tabindex'), '0'); + expect(view2.dom.rootElement.getAttribute('tabindex'), '-1'); + + view2.dom.rootElement.blur(); + expect(view1.dom.rootElement.getAttribute('tabindex'), '0'); + expect(view2.dom.rootElement.getAttribute('tabindex'), '0'); }); test('fires a focus event - a view was focused', () async { - final List viewFocusEvents = []; - final DomElement div = createDomElement('div'); - final EngineFlutterView view = EngineFlutterView(platformDispatcher, div); - final DomElement focusableViewElement = div - .querySelector(DomManager.flutterViewTagName)! - ..setAttribute('tabindex', 0); - - platformDispatcher.onViewFocusChange = viewFocusEvents.add; - domDocument.body!.append(div); - focusableViewElement.focus(); - - expect(viewFocusEvents, hasLength(1)); - - expect(viewFocusEvents[0].viewId, view.viewId); - expect(viewFocusEvents[0].state, ui.ViewFocusState.focused); - expect(viewFocusEvents[0].direction, ui.ViewFocusDirection.forward); + final EngineFlutterView view = createAndRegisterView(dispatcher); + + view.dom.rootElement.focus(); + + expect(dispatchedViewFocusEvents, hasLength(1)); + + expect(dispatchedViewFocusEvents[0].viewId, view.viewId); + expect(dispatchedViewFocusEvents[0].state, ui.ViewFocusState.focused); + expect(dispatchedViewFocusEvents[0].direction, ui.ViewFocusDirection.forward); }); test('fires a focus event - a view was unfocused', () async { - final List viewFocusEvents = []; - final DomElement div = createDomElement('div'); - final EngineFlutterView view = EngineFlutterView(platformDispatcher, div); - final DomElement focusableViewElement = div - .querySelector(DomManager.flutterViewTagName)! - ..setAttribute('tabindex', 0); - - platformDispatcher.onViewFocusChange = viewFocusEvents.add; - domDocument.body!.append(div); - focusableViewElement.focus(); - focusableViewElement.blur(); - - expect(viewFocusEvents, hasLength(2)); - - expect(viewFocusEvents[0].viewId, view.viewId); - expect(viewFocusEvents[0].state, ui.ViewFocusState.focused); - expect(viewFocusEvents[0].direction, ui.ViewFocusDirection.forward); - - expect(viewFocusEvents[1].viewId, view.viewId); - expect(viewFocusEvents[1].state, ui.ViewFocusState.unfocused); - expect(viewFocusEvents[1].direction, ui.ViewFocusDirection.undefined); + final EngineFlutterView view = createAndRegisterView(dispatcher); + + view.dom.rootElement.focus(); + view.dom.rootElement.blur(); + + expect(dispatchedViewFocusEvents, hasLength(2)); + + expect(dispatchedViewFocusEvents[0].viewId, view.viewId); + expect(dispatchedViewFocusEvents[0].state, ui.ViewFocusState.focused); + expect(dispatchedViewFocusEvents[0].direction, ui.ViewFocusDirection.forward); + + expect(dispatchedViewFocusEvents[1].viewId, view.viewId); + expect(dispatchedViewFocusEvents[1].state, ui.ViewFocusState.unfocused); + expect(dispatchedViewFocusEvents[1].direction, ui.ViewFocusDirection.undefined); }); test('fires a focus event - focus transitions between views', () async { - final List viewFocusEvents = []; - final DomElement div1 = createDomElement('div'); - final DomElement div2 = createDomElement('div'); - final EngineFlutterView view1 = - EngineFlutterView(platformDispatcher, div1); - final EngineFlutterView view2 = - EngineFlutterView(platformDispatcher, div2); - final DomElement focusableViewElement1 = div1 - .querySelector(DomManager.flutterViewTagName)! - ..setAttribute('tabindex', 0); - final DomElement focusableViewElement2 = div2 - .querySelector(DomManager.flutterViewTagName)! - ..setAttribute('tabindex', 0); - - domDocument.body!.append(div1); - domDocument.body!.append(div2); - - platformDispatcher.onViewFocusChange = viewFocusEvents.add; - - focusableViewElement1.focus(); - focusableViewElement2.focus(); + final EngineFlutterView view1 = createAndRegisterView(dispatcher); + final EngineFlutterView view2 = createAndRegisterView(dispatcher); + + view1.dom.rootElement.focus(); + view2.dom.rootElement.focus(); // The statements simulate the user pressing shift + tab in the keyboard. // Synthetic keyboard events do not trigger focus changes. domDocument.body!.pressTabKey(shift: true); - focusableViewElement1.focus(); + view1.dom.rootElement.focus(); domDocument.body!.releaseTabKey(); - expect(viewFocusEvents, hasLength(3)); + expect(dispatchedViewFocusEvents, hasLength(3)); - expect(viewFocusEvents[0].viewId, view1.viewId); - expect(viewFocusEvents[0].state, ui.ViewFocusState.focused); - expect(viewFocusEvents[0].direction, ui.ViewFocusDirection.forward); + expect(dispatchedViewFocusEvents[0].viewId, view1.viewId); + expect(dispatchedViewFocusEvents[0].state, ui.ViewFocusState.focused); + expect(dispatchedViewFocusEvents[0].direction, ui.ViewFocusDirection.forward); - expect(viewFocusEvents[1].viewId, view2.viewId); - expect(viewFocusEvents[1].state, ui.ViewFocusState.focused); - expect(viewFocusEvents[1].direction, ui.ViewFocusDirection.forward); + expect(dispatchedViewFocusEvents[1].viewId, view2.viewId); + expect(dispatchedViewFocusEvents[1].state, ui.ViewFocusState.focused); + expect(dispatchedViewFocusEvents[1].direction, ui.ViewFocusDirection.forward); - expect(viewFocusEvents[2].viewId, view1.viewId); - expect(viewFocusEvents[2].state, ui.ViewFocusState.focused); - expect(viewFocusEvents[2].direction, ui.ViewFocusDirection.backward); + expect(dispatchedViewFocusEvents[2].viewId, view1.viewId); + expect(dispatchedViewFocusEvents[2].state, ui.ViewFocusState.focused); + expect(dispatchedViewFocusEvents[2].direction, ui.ViewFocusDirection.backward); }); test('fires a focus event - focus transitions on and off views', () async { - final List viewFocusEvents = []; - final DomElement div1 = createDomElement('div'); - final DomElement div2 = createDomElement('div'); - final EngineFlutterView view1 = - EngineFlutterView(platformDispatcher, div1); - final EngineFlutterView view2 = - EngineFlutterView(platformDispatcher, div2); - final DomElement focusableViewElement1 = div1 - .querySelector(DomManager.flutterViewTagName)! - ..setAttribute('tabindex', 0); - final DomElement focusableViewElement2 = div2 - .querySelector(DomManager.flutterViewTagName)! - ..setAttribute('tabindex', 0); - - domDocument.body!.append(div1); - domDocument.body!.append(div2); - - platformDispatcher.onViewFocusChange = viewFocusEvents.add; - - focusableViewElement1.focus(); - focusableViewElement2.focus(); - focusableViewElement2.blur(); - - expect(viewFocusEvents, hasLength(3)); - - expect(viewFocusEvents[0].viewId, view1.viewId); - expect(viewFocusEvents[0].state, ui.ViewFocusState.focused); - expect(viewFocusEvents[0].direction, ui.ViewFocusDirection.forward); - - expect(viewFocusEvents[1].viewId, view2.viewId); - expect(viewFocusEvents[1].state, ui.ViewFocusState.focused); - expect(viewFocusEvents[1].direction, ui.ViewFocusDirection.forward); - - expect(viewFocusEvents[2].viewId, view2.viewId); - expect(viewFocusEvents[2].state, ui.ViewFocusState.unfocused); - expect(viewFocusEvents[2].direction, ui.ViewFocusDirection.undefined); + final EngineFlutterView view1 = createAndRegisterView(dispatcher); + final EngineFlutterView view2 = createAndRegisterView(dispatcher); + + view1.dom.rootElement.focus(); + view2.dom.rootElement.focus(); + view2.dom.rootElement.blur(); + + expect(dispatchedViewFocusEvents, hasLength(3)); + + expect(dispatchedViewFocusEvents[0].viewId, view1.viewId); + expect(dispatchedViewFocusEvents[0].state, ui.ViewFocusState.focused); + expect(dispatchedViewFocusEvents[0].direction, ui.ViewFocusDirection.forward); + + expect(dispatchedViewFocusEvents[1].viewId, view2.viewId); + expect(dispatchedViewFocusEvents[1].state, ui.ViewFocusState.focused); + expect(dispatchedViewFocusEvents[1].direction, ui.ViewFocusDirection.forward); + + expect(dispatchedViewFocusEvents[2].viewId, view2.viewId); + expect(dispatchedViewFocusEvents[2].state, ui.ViewFocusState.unfocused); + expect(dispatchedViewFocusEvents[2].direction, ui.ViewFocusDirection.undefined); }); }); } +EngineFlutterView createAndRegisterView(EnginePlatformDispatcher dispatcher) { + final DomElement div = createDomElement('div'); + final EngineFlutterView view = EngineFlutterView(dispatcher, div); + domDocument.body!.append(div); + dispatcher.viewManager.registerView(view); + return view; +} + extension on DomElement { void pressTabKey({bool shift = false}) { dispatchKeyboardEvent(type: 'keydown', key: 'Tab', shiftKey: shift); diff --git a/lib/web_ui/test/engine/view_embedder/style_manager_test.dart b/lib/web_ui/test/engine/view_embedder/style_manager_test.dart index bff6acc86b1e2..97634fef7c006 100644 --- a/lib/web_ui/test/engine/view_embedder/style_manager_test.dart +++ b/lib/web_ui/test/engine/view_embedder/style_manager_test.dart @@ -14,6 +14,24 @@ void main() { void doTests() { group('StyleManager', () { + test('attachGlobalStyles hides the outline when focused', () { + final DomElement flutterViewElement = createDomElement(DomManager.flutterViewTagName); + + domDocument.body!.append(flutterViewElement); + StyleManager.attachGlobalStyles( + node: flutterViewElement, + styleId: 'testing', + styleNonce: 'testing', + cssSelectorPrefix: DomManager.flutterViewTagName, + ); + final String expected = browserEngine == BrowserEngine.firefox + ? 'rgb(0, 0, 0) 0px' + : 'rgb(0, 0, 0) none 0px'; + final String got = domWindow.getComputedStyle(flutterViewElement, 'focus').outline; + + expect(got, expected); + }); + test('styleSceneHost', () { expect( () => StyleManager.styleSceneHost(createDomHTMLDivElement()), From 6f69db8870f7f088d7959a1c09e8ef863fe9101b Mon Sep 17 00:00:00 2001 From: Juan Tugores Date: Thu, 22 Feb 2024 19:02:51 -0800 Subject: [PATCH 2/9] Apply reviewer suggestions --- .../lib/src/engine/platform_dispatcher.dart | 18 ++++-- .../view_focus_binding.dart | 60 ++++++++++--------- .../view_embedder/flutter_view_manager.dart | 10 ++++ .../flutter_view_manager_test.dart | 10 ++++ 4 files changed, 64 insertions(+), 34 deletions(-) diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index d5f17fdedae8c..6fa60c2d10168 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -77,9 +77,9 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { HighContrastSupport.instance.addListener(_updateHighContrast); _addFontSizeObserver(); _addLocaleChangedListener(); + _addViewFocusChangeListener(); registerHotRestartListener(dispose); AppLifecycleState.instance.addListener(_setAppLifecycleState); - _viewFocusBinding.init(); domDocument.body?.append(accessibilityPlaceholder); _onViewDisposedListener = viewManager.onViewDisposed.listen((_) { // Send a metrics changed event to the framework when a view is disposed. @@ -121,6 +121,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { _removeBrightnessMediaQueryListener(); _disconnectFontSizeObserver(); _removeLocaleChangedListener(); + _removeViewFocusChangeListener(); HighContrastSupport.instance.removeListener(_updateHighContrast); AppLifecycleState.instance.removeListener(_setAppLifecycleState); _viewFocusBinding.dispose(); @@ -228,10 +229,17 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { } } - late final ViewFocusBinding _viewFocusBinding = ViewFocusBinding( - viewManager: viewManager, - onViewFocusChange: invokeOnViewFocusChange, - ); + late final ViewFocusBinding _viewFocusBinding = ViewFocusBinding(viewManager); + StreamSubscription? _onViewFocusChangeSubscription; + + void _addViewFocusChangeListener() { + _onViewFocusChangeSubscription = _viewFocusBinding.onViewFocusChange.listen( + invokeOnViewFocusChange, + ); + } + void _removeViewFocusChangeListener() { + _onViewFocusChangeSubscription?.cancel(); + } @override ui.ViewFocusChangeCallback? get onViewFocusChange => _onViewFocusChange; diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart b/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart index 0e2661d65a49a..ea9efefe4e2a5 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart @@ -9,26 +9,27 @@ import 'package:ui/ui.dart' as ui; /// Tracks the [FlutterView]s focus changes. final class ViewFocusBinding { /// Creates a [ViewFocusBinding] instance. - ViewFocusBinding({ - required FlutterViewManager viewManager, - required ui.ViewFocusChangeCallback onViewFocusChange, - }): - _viewManager = viewManager, - _onViewFocusChange = onViewFocusChange; + ViewFocusBinding(this._viewManager) { + _init(); + } final FlutterViewManager _viewManager; - final ui.ViewFocusChangeCallback _onViewFocusChange; + + Stream get onViewFocusChange => _onViewFocusChangeController.stream; + final StreamController _onViewFocusChangeController = + StreamController.broadcast(sync: true); + + StreamSubscription? _onViewCreatedListener; int? _lastViewId; ui.ViewFocusDirection _viewFocusDirection = ui.ViewFocusDirection.forward; - StreamSubscription? _onViewCreatedListener; - void init() { + void _init() { domDocument.body?.addEventListener(_keyDown, _handleKeyDown); domDocument.body?.addEventListener(_keyUp, _handleKeyUp); domDocument.body?.addEventListener(_focusin, _handleFocusin); domDocument.body?.addEventListener(_focusout, _handleFocusout); - _onViewCreatedListener = _viewManager.onViewCreated.listen(_markViewAsFocusable); + _onViewCreatedListener = _viewManager.onViewCreated.listen(_handleViewCreated); } void dispose() { @@ -61,45 +62,46 @@ final class ViewFocusBinding { }); void _handleFocusChange(DomElement? focusedElement) { - final int? lastViewId = _lastViewId; final int? viewId = _viewId(focusedElement); - if (viewId == lastViewId) { + if (viewId == _lastViewId) { return; } final ui.ViewFocusEvent event; - if (viewId != null) { + if (viewId == null) { event = ui.ViewFocusEvent( - viewId: viewId, - state: ui.ViewFocusState.focused, - direction: _viewFocusDirection, + viewId: _lastViewId!, + state: ui.ViewFocusState.unfocused, + direction: ui.ViewFocusDirection.undefined, ); } else { event = ui.ViewFocusEvent( - viewId: lastViewId!, - state: ui.ViewFocusState.unfocused, - direction: ui.ViewFocusDirection.undefined, + viewId: viewId, + state: ui.ViewFocusState.focused, + direction: _viewFocusDirection, ); } - _lastViewId = viewId; - _markViewAsFocusable(lastViewId); + _markViewAsFocusable(_lastViewId, reachableByKeyboard: true); _markViewAsFocusable(viewId, reachableByKeyboard: false); - _onViewFocusChange(event); + _lastViewId = viewId; + _onViewFocusChangeController.add(event); } int? _viewId(DomElement? element) { - final DomElement? viewElement = element?.closest(DomManager.flutterViewTagName); - for (final EngineFlutterView view in _viewManager.views) { - if (view.dom.rootElement == viewElement) { - return view.viewId; - } + final DomElement? rootElement = element?.closest(DomManager.flutterViewTagName); + if (rootElement == null) { + return null; } - return null; + return _viewManager.viewIdForRootElement(rootElement); + } + + void _handleViewCreated(int viewId) { + _markViewAsFocusable(viewId, reachableByKeyboard: true); } void _markViewAsFocusable( int? viewId, { - bool reachableByKeyboard = true, + required bool reachableByKeyboard, }) { if (viewId == null) { return; diff --git a/lib/web_ui/lib/src/engine/view_embedder/flutter_view_manager.dart b/lib/web_ui/lib/src/engine/view_embedder/flutter_view_manager.dart index e93956f0325a1..684e5ea667c22 100644 --- a/lib/web_ui/lib/src/engine/view_embedder/flutter_view_manager.dart +++ b/lib/web_ui/lib/src/engine/view_embedder/flutter_view_manager.dart @@ -96,6 +96,16 @@ class FlutterViewManager { return _jsViewOptions[viewId]; } + /// Returns the [viewId] if [rootElement] corresponds to any of the [views]. + int? viewIdForRootElement(DomElement rootElement) { + for(final EngineFlutterView view in views) { + if (view.dom.rootElement == rootElement) { + return view.viewId; + } + } + return null; + } + void dispose() { // We need to call `toList()` in order to avoid concurrent modification // inside the loop. diff --git a/lib/web_ui/test/engine/view_embedder/flutter_view_manager_test.dart b/lib/web_ui/test/engine/view_embedder/flutter_view_manager_test.dart index 04fb618e9dc6c..5dbec513268d0 100644 --- a/lib/web_ui/test/engine/view_embedder/flutter_view_manager_test.dart +++ b/lib/web_ui/test/engine/view_embedder/flutter_view_manager_test.dart @@ -101,5 +101,15 @@ Future doTests() async { reason: 'Should fire dispose event for view'); }); }); + + group('viewIdForRootElement', () { + test('works', () { + final EngineFlutterView view = EngineFlutterView(platformDispatcher, createDomElement('div')); + final int viewId = view.viewId; + viewManager.registerView(view); + + expect(viewManager.viewIdForRootElement(view.dom.rootElement), viewId); + }); + }); }); } From 87ddd10a7872b87dbf0d6a21d343ba610f46c0d7 Mon Sep 17 00:00:00 2001 From: Juan Tugores Date: Fri, 23 Feb 2024 13:47:53 -0800 Subject: [PATCH 3/9] Apply reviewer suggestions --- .../lib/src/engine/platform_dispatcher.dart | 18 +++--------------- .../view_focus_binding.dart | 17 ++++++----------- 2 files changed, 9 insertions(+), 26 deletions(-) diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index 6fa60c2d10168..2f5208edae438 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -77,9 +77,9 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { HighContrastSupport.instance.addListener(_updateHighContrast); _addFontSizeObserver(); _addLocaleChangedListener(); - _addViewFocusChangeListener(); registerHotRestartListener(dispose); AppLifecycleState.instance.addListener(_setAppLifecycleState); + _viewFocusBinding.init(); domDocument.body?.append(accessibilityPlaceholder); _onViewDisposedListener = viewManager.onViewDisposed.listen((_) { // Send a metrics changed event to the framework when a view is disposed. @@ -121,7 +121,6 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { _removeBrightnessMediaQueryListener(); _disconnectFontSizeObserver(); _removeLocaleChangedListener(); - _removeViewFocusChangeListener(); HighContrastSupport.instance.removeListener(_updateHighContrast); AppLifecycleState.instance.removeListener(_setAppLifecycleState); _viewFocusBinding.dispose(); @@ -229,17 +228,8 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { } } - late final ViewFocusBinding _viewFocusBinding = ViewFocusBinding(viewManager); - StreamSubscription? _onViewFocusChangeSubscription; - - void _addViewFocusChangeListener() { - _onViewFocusChangeSubscription = _viewFocusBinding.onViewFocusChange.listen( - invokeOnViewFocusChange, - ); - } - void _removeViewFocusChangeListener() { - _onViewFocusChangeSubscription?.cancel(); - } + late final ViewFocusBinding _viewFocusBinding = + ViewFocusBinding(viewManager, invokeOnViewFocusChange); @override ui.ViewFocusChangeCallback? get onViewFocusChange => _onViewFocusChange; @@ -261,7 +251,6 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { ); } - @override void requestViewFocusChange({ required int viewId, @@ -271,7 +260,6 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { // TODO(tugorez): implement this method. At the moment will be a no op call. } - /// A set of views which have rendered in the current `onBeginFrame` or /// `onDrawFrame` scope. Set? _viewsRenderedInCurrentFrame; diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart b/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart index ea9efefe4e2a5..ba32e3c922e8e 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart @@ -9,22 +9,17 @@ import 'package:ui/ui.dart' as ui; /// Tracks the [FlutterView]s focus changes. final class ViewFocusBinding { /// Creates a [ViewFocusBinding] instance. - ViewFocusBinding(this._viewManager) { - _init(); - } + ViewFocusBinding(this._viewManager, this._onViewFocusChange); final FlutterViewManager _viewManager; - - Stream get onViewFocusChange => _onViewFocusChangeController.stream; - final StreamController _onViewFocusChangeController = - StreamController.broadcast(sync: true); - - StreamSubscription? _onViewCreatedListener; + final ui.ViewFocusChangeCallback _onViewFocusChange; int? _lastViewId; ui.ViewFocusDirection _viewFocusDirection = ui.ViewFocusDirection.forward; - void _init() { + StreamSubscription? _onViewCreatedListener; + + void init() { domDocument.body?.addEventListener(_keyDown, _handleKeyDown); domDocument.body?.addEventListener(_keyUp, _handleKeyUp); domDocument.body?.addEventListener(_focusin, _handleFocusin); @@ -84,7 +79,7 @@ final class ViewFocusBinding { _markViewAsFocusable(_lastViewId, reachableByKeyboard: true); _markViewAsFocusable(viewId, reachableByKeyboard: false); _lastViewId = viewId; - _onViewFocusChangeController.add(event); + _onViewFocusChange(event); } int? _viewId(DomElement? element) { From db97ed82b67c0564817feca31ea2f29993a41b26 Mon Sep 17 00:00:00 2001 From: Juan Tugores Date: Fri, 23 Feb 2024 13:47:59 -0800 Subject: [PATCH 4/9] Apply reviewer suggestions --- lib/web_ui/lib/src/engine/platform_dispatcher.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index 2f5208edae438..cf56f332102cf 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -228,7 +228,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { } } - late final ViewFocusBinding _viewFocusBinding = + late final ViewFocusBinding _viewFocusBinding = ViewFocusBinding(viewManager, invokeOnViewFocusChange); @override From e40ddd6af7255312a3ef74ae7d927f85301c027a Mon Sep 17 00:00:00 2001 From: Juan Tugores Date: Fri, 23 Feb 2024 14:14:51 -0800 Subject: [PATCH 5/9] Make it a mixin --- .../lib/src/engine/platform_dispatcher.dart | 11 +++++----- .../view_focus_binding.dart | 21 ++++++++----------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index cf56f332102cf..d0806480c3f39 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -69,7 +69,7 @@ class HighContrastSupport { /// /// This is the central entry point for platform messages and configuration /// events from the platform. -class EnginePlatformDispatcher extends ui.PlatformDispatcher { +class EnginePlatformDispatcher extends ui.PlatformDispatcher with ViewFocusBinding { /// Private constructor, since only dart:ui is supposed to create one of /// these. EnginePlatformDispatcher() { @@ -79,7 +79,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { _addLocaleChangedListener(); registerHotRestartListener(dispose); AppLifecycleState.instance.addListener(_setAppLifecycleState); - _viewFocusBinding.init(); + initViewFocusBindings(); domDocument.body?.append(accessibilityPlaceholder); _onViewDisposedListener = viewManager.onViewDisposed.listen((_) { // Send a metrics changed event to the framework when a view is disposed. @@ -123,7 +123,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { _removeLocaleChangedListener(); HighContrastSupport.instance.removeListener(_updateHighContrast); AppLifecycleState.instance.removeListener(_setAppLifecycleState); - _viewFocusBinding.dispose(); + disposeViewFocusBindings(); accessibilityPlaceholder.remove(); _onViewDisposedListener.cancel(); viewManager.dispose(); @@ -153,6 +153,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { EngineFlutterDisplay.instance, ]; + @override late final FlutterViewManager viewManager = FlutterViewManager(this); /// The current list of windows. @@ -228,9 +229,6 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { } } - late final ViewFocusBinding _viewFocusBinding = - ViewFocusBinding(viewManager, invokeOnViewFocusChange); - @override ui.ViewFocusChangeCallback? get onViewFocusChange => _onViewFocusChange; ui.ViewFocusChangeCallback? _onViewFocusChange; @@ -243,6 +241,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { // Engine code should use this method instead of the callback directly. // Otherwise zones won't work properly. + @override void invokeOnViewFocusChange(ui.ViewFocusEvent viewFocusEvent) { invoke1( _onViewFocusChange, diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart b/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart index ba32e3c922e8e..0c792f17628a3 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart @@ -7,27 +7,24 @@ import 'package:ui/src/engine.dart'; import 'package:ui/ui.dart' as ui; /// Tracks the [FlutterView]s focus changes. -final class ViewFocusBinding { - /// Creates a [ViewFocusBinding] instance. - ViewFocusBinding(this._viewManager, this._onViewFocusChange); - - final FlutterViewManager _viewManager; - final ui.ViewFocusChangeCallback _onViewFocusChange; +mixin ViewFocusBinding { + FlutterViewManager get viewManager; + void invokeOnViewFocusChange(ui.ViewFocusEvent viewFocusEvent); int? _lastViewId; ui.ViewFocusDirection _viewFocusDirection = ui.ViewFocusDirection.forward; StreamSubscription? _onViewCreatedListener; - void init() { + void initViewFocusBindings() { domDocument.body?.addEventListener(_keyDown, _handleKeyDown); domDocument.body?.addEventListener(_keyUp, _handleKeyUp); domDocument.body?.addEventListener(_focusin, _handleFocusin); domDocument.body?.addEventListener(_focusout, _handleFocusout); - _onViewCreatedListener = _viewManager.onViewCreated.listen(_handleViewCreated); + _onViewCreatedListener = viewManager.onViewCreated.listen(_handleViewCreated); } - void dispose() { + void disposeViewFocusBindings() { domDocument.body?.removeEventListener(_keyDown, _handleKeyDown); domDocument.body?.removeEventListener(_keyUp, _handleKeyUp); domDocument.body?.removeEventListener(_focusin, _handleFocusin); @@ -79,7 +76,7 @@ final class ViewFocusBinding { _markViewAsFocusable(_lastViewId, reachableByKeyboard: true); _markViewAsFocusable(viewId, reachableByKeyboard: false); _lastViewId = viewId; - _onViewFocusChange(event); + invokeOnViewFocusChange(event); } int? _viewId(DomElement? element) { @@ -87,7 +84,7 @@ final class ViewFocusBinding { if (rootElement == null) { return null; } - return _viewManager.viewIdForRootElement(rootElement); + return viewManager.viewIdForRootElement(rootElement); } void _handleViewCreated(int viewId) { @@ -108,7 +105,7 @@ final class ViewFocusBinding { // the flutter view and having it with a zero tabindex messes the focus // traversal order when pressing tab or shift tab. final int tabIndex = reachableByKeyboard ? 0 : -1; - _viewManager[viewId]?.dom.rootElement.setAttribute('tabindex', tabIndex); + viewManager[viewId]?.dom.rootElement.setAttribute('tabindex', tabIndex); } static const String _focusin = 'focusin'; From f3ba20d1f5668ab967c3e974035086d5e089768f Mon Sep 17 00:00:00 2001 From: Juan Tugores Date: Fri, 23 Feb 2024 14:37:32 -0800 Subject: [PATCH 6/9] Revert mixin changes --- .../lib/src/engine/platform_dispatcher.dart | 11 +++++----- .../view_focus_binding.dart | 20 ++++++++++--------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index d0806480c3f39..cf56f332102cf 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -69,7 +69,7 @@ class HighContrastSupport { /// /// This is the central entry point for platform messages and configuration /// events from the platform. -class EnginePlatformDispatcher extends ui.PlatformDispatcher with ViewFocusBinding { +class EnginePlatformDispatcher extends ui.PlatformDispatcher { /// Private constructor, since only dart:ui is supposed to create one of /// these. EnginePlatformDispatcher() { @@ -79,7 +79,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher with ViewFocusBindi _addLocaleChangedListener(); registerHotRestartListener(dispose); AppLifecycleState.instance.addListener(_setAppLifecycleState); - initViewFocusBindings(); + _viewFocusBinding.init(); domDocument.body?.append(accessibilityPlaceholder); _onViewDisposedListener = viewManager.onViewDisposed.listen((_) { // Send a metrics changed event to the framework when a view is disposed. @@ -123,7 +123,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher with ViewFocusBindi _removeLocaleChangedListener(); HighContrastSupport.instance.removeListener(_updateHighContrast); AppLifecycleState.instance.removeListener(_setAppLifecycleState); - disposeViewFocusBindings(); + _viewFocusBinding.dispose(); accessibilityPlaceholder.remove(); _onViewDisposedListener.cancel(); viewManager.dispose(); @@ -153,7 +153,6 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher with ViewFocusBindi EngineFlutterDisplay.instance, ]; - @override late final FlutterViewManager viewManager = FlutterViewManager(this); /// The current list of windows. @@ -229,6 +228,9 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher with ViewFocusBindi } } + late final ViewFocusBinding _viewFocusBinding = + ViewFocusBinding(viewManager, invokeOnViewFocusChange); + @override ui.ViewFocusChangeCallback? get onViewFocusChange => _onViewFocusChange; ui.ViewFocusChangeCallback? _onViewFocusChange; @@ -241,7 +243,6 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher with ViewFocusBindi // Engine code should use this method instead of the callback directly. // Otherwise zones won't work properly. - @override void invokeOnViewFocusChange(ui.ViewFocusEvent viewFocusEvent) { invoke1( _onViewFocusChange, diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart b/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart index 0c792f17628a3..441816a602ec1 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart @@ -7,24 +7,26 @@ import 'package:ui/src/engine.dart'; import 'package:ui/ui.dart' as ui; /// Tracks the [FlutterView]s focus changes. -mixin ViewFocusBinding { - FlutterViewManager get viewManager; - void invokeOnViewFocusChange(ui.ViewFocusEvent viewFocusEvent); +final class ViewFocusBinding { + ViewFocusBinding(this._viewManager, this._onViewFocusChange); + + final FlutterViewManager _viewManager; + final ui.ViewFocusChangeCallback _onViewFocusChange; int? _lastViewId; ui.ViewFocusDirection _viewFocusDirection = ui.ViewFocusDirection.forward; StreamSubscription? _onViewCreatedListener; - void initViewFocusBindings() { + void init() { domDocument.body?.addEventListener(_keyDown, _handleKeyDown); domDocument.body?.addEventListener(_keyUp, _handleKeyUp); domDocument.body?.addEventListener(_focusin, _handleFocusin); domDocument.body?.addEventListener(_focusout, _handleFocusout); - _onViewCreatedListener = viewManager.onViewCreated.listen(_handleViewCreated); + _onViewCreatedListener = _viewManager.onViewCreated.listen(_handleViewCreated); } - void disposeViewFocusBindings() { + void dispose() { domDocument.body?.removeEventListener(_keyDown, _handleKeyDown); domDocument.body?.removeEventListener(_keyUp, _handleKeyUp); domDocument.body?.removeEventListener(_focusin, _handleFocusin); @@ -76,7 +78,7 @@ mixin ViewFocusBinding { _markViewAsFocusable(_lastViewId, reachableByKeyboard: true); _markViewAsFocusable(viewId, reachableByKeyboard: false); _lastViewId = viewId; - invokeOnViewFocusChange(event); + _onViewFocusChange(event); } int? _viewId(DomElement? element) { @@ -84,7 +86,7 @@ mixin ViewFocusBinding { if (rootElement == null) { return null; } - return viewManager.viewIdForRootElement(rootElement); + return _viewManager.viewIdForRootElement(rootElement); } void _handleViewCreated(int viewId) { @@ -105,7 +107,7 @@ mixin ViewFocusBinding { // the flutter view and having it with a zero tabindex messes the focus // traversal order when pressing tab or shift tab. final int tabIndex = reachableByKeyboard ? 0 : -1; - viewManager[viewId]?.dom.rootElement.setAttribute('tabindex', tabIndex); + _viewManager[viewId]?.dom.rootElement.setAttribute('tabindex', tabIndex); } static const String _focusin = 'focusin'; From 78731aab8da065756161033c9b562a19050ff988 Mon Sep 17 00:00:00 2001 From: Juan Tugores Date: Wed, 28 Feb 2024 08:56:19 -0800 Subject: [PATCH 7/9] Prepend the accesibility placeholder --- lib/web_ui/lib/src/engine/platform_dispatcher.dart | 2 +- .../engine/platform_dispatcher/platform_dispatcher_test.dart | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index cf56f332102cf..20588af1054f7 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -80,7 +80,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { registerHotRestartListener(dispose); AppLifecycleState.instance.addListener(_setAppLifecycleState); _viewFocusBinding.init(); - domDocument.body?.append(accessibilityPlaceholder); + domDocument.body?.prepend(accessibilityPlaceholder); _onViewDisposedListener = viewManager.onViewDisposed.listen((_) { // Send a metrics changed event to the framework when a view is disposed. // View creation/resize is handled by the `_didResize` handler in the diff --git a/lib/web_ui/test/engine/platform_dispatcher/platform_dispatcher_test.dart b/lib/web_ui/test/engine/platform_dispatcher/platform_dispatcher_test.dart index 09d64757b5324..c9f9e69f5b4ad 100644 --- a/lib/web_ui/test/engine/platform_dispatcher/platform_dispatcher_test.dart +++ b/lib/web_ui/test/engine/platform_dispatcher/platform_dispatcher_test.dart @@ -409,8 +409,9 @@ void testMain() { }); }); - test('appends an accesibility placeholder', () { + test('adds the accesibility placeholder', () { expect(dispatcher.accessibilityPlaceholder.isConnected, isTrue); + expect(domDocument.body!.children.first, dispatcher.accessibilityPlaceholder); }); test('removes the accesibility placeholder', () { From 6905458346c0303ae064562abbf62fe95b76faa4 Mon Sep 17 00:00:00 2001 From: Juan Tugores Date: Wed, 28 Feb 2024 11:02:49 -0800 Subject: [PATCH 8/9] Do not mark the view as focusable when semantics are enabled --- .../view_focus_binding.dart | 29 +++++++++++-------- .../view_focus_binding_test.dart | 27 +++++++++++++++-- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart b/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart index 441816a602ec1..9f9852bd48ed8 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart @@ -75,8 +75,8 @@ final class ViewFocusBinding { direction: _viewFocusDirection, ); } - _markViewAsFocusable(_lastViewId, reachableByKeyboard: true); - _markViewAsFocusable(viewId, reachableByKeyboard: false); + _maybeMarkViewAsFocusable(_lastViewId, reachableByKeyboard: true); + _maybeMarkViewAsFocusable(viewId, reachableByKeyboard: false); _lastViewId = viewId; _onViewFocusChange(event); } @@ -90,24 +90,29 @@ final class ViewFocusBinding { } void _handleViewCreated(int viewId) { - _markViewAsFocusable(viewId, reachableByKeyboard: true); + _maybeMarkViewAsFocusable(viewId, reachableByKeyboard: true); } - void _markViewAsFocusable( + void _maybeMarkViewAsFocusable( int? viewId, { required bool reachableByKeyboard, }) { if (viewId == null) { return; } - // A tabindex with value zero means the DOM element can be reached by using - // the keyboard (tab, shift + tab). When its value is -1 it is still focusable - // but can't be focused by the result of keyboard events This is specially - // important when the semantics tree is enabled as it puts DOM nodes inside - // the flutter view and having it with a zero tabindex messes the focus - // traversal order when pressing tab or shift tab. - final int tabIndex = reachableByKeyboard ? 0 : -1; - _viewManager[viewId]?.dom.rootElement.setAttribute('tabindex', tabIndex); + + final DomElement? rootElement = _viewManager[viewId]?.dom.rootElement; + if (EngineSemantics.instance.semanticsEnabled) { + rootElement?.removeAttribute('tabindex'); + } else { + // A tabindex with value zero means the DOM element can be reached by using + // the keyboard (tab, shift + tab). When its value is -1 it is still focusable + // but can't be focused by the result of keyboard events This is specially + // important when the semantics tree is enabled as it puts DOM nodes inside + // the flutter view and having it with a zero tabindex messes the focus + // traversal order when pressing tab or shift tab. + rootElement?.setAttribute('tabindex', reachableByKeyboard ? 0 : -1); + } } static const String _focusin = 'focusin'; diff --git a/lib/web_ui/test/engine/platform_dispatcher/view_focus_binding_test.dart b/lib/web_ui/test/engine/platform_dispatcher/view_focus_binding_test.dart index e5c43b0999bde..c26c3a219ec09 100644 --- a/lib/web_ui/test/engine/platform_dispatcher/view_focus_binding_test.dart +++ b/lib/web_ui/test/engine/platform_dispatcher/view_focus_binding_test.dart @@ -1,4 +1,4 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. +// Copyright 2014 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. import 'package:test/bootstrap/browser.dart'; @@ -17,6 +17,7 @@ void testMain() { setUp(() { domDocument.activeElement?.blur(); + EngineSemantics.instance.semanticsEnabled = false; dispatcher = EnginePlatformDispatcher.instance; dispatchedViewFocusEvents = []; @@ -42,7 +43,7 @@ void testMain() { expect(view.dom.rootElement.getAttribute('tabindex'), '-1'); }); - test('Correctly marks the focusable views as reachable by keyboard or not', () async { + test('marks the focusable views as reachable by the keyboard or not', () async { final EngineFlutterView view1 = createAndRegisterView(dispatcher); final EngineFlutterView view2 = createAndRegisterView(dispatcher); @@ -62,6 +63,28 @@ void testMain() { expect(view2.dom.rootElement.getAttribute('tabindex'), '0'); }); + test('never marks the views as focusable with semantincs enabled', () async { + EngineSemantics.instance.semanticsEnabled = true; + + final EngineFlutterView view1 = createAndRegisterView(dispatcher); + final EngineFlutterView view2 = createAndRegisterView(dispatcher); + + expect(view1.dom.rootElement.getAttribute('tabindex'), isNull); + expect(view2.dom.rootElement.getAttribute('tabindex'), isNull); + + view1.dom.rootElement.focus(); + expect(view1.dom.rootElement.getAttribute('tabindex'), isNull); + expect(view2.dom.rootElement.getAttribute('tabindex'), isNull); + + view2.dom.rootElement.focus(); + expect(view1.dom.rootElement.getAttribute('tabindex'), isNull); + expect(view2.dom.rootElement.getAttribute('tabindex'), isNull); + + view2.dom.rootElement.blur(); + expect(view1.dom.rootElement.getAttribute('tabindex'), isNull); + expect(view2.dom.rootElement.getAttribute('tabindex'), isNull); + }); + test('fires a focus event - a view was focused', () async { final EngineFlutterView view = createAndRegisterView(dispatcher); From 314e4b6da1bf8e78337e627bbd0a99a2f6a0e58b Mon Sep 17 00:00:00 2001 From: Juan Tugores Date: Wed, 28 Feb 2024 13:51:36 -0800 Subject: [PATCH 9/9] Fix license file --- .../engine/platform_dispatcher/view_focus_binding_test.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/web_ui/test/engine/platform_dispatcher/view_focus_binding_test.dart b/lib/web_ui/test/engine/platform_dispatcher/view_focus_binding_test.dart index c26c3a219ec09..8610d394fff90 100644 --- a/lib/web_ui/test/engine/platform_dispatcher/view_focus_binding_test.dart +++ b/lib/web_ui/test/engine/platform_dispatcher/view_focus_binding_test.dart @@ -1,6 +1,7 @@ -// Copyright 2014 The Flutter Authors. All rights reserved. +// Copyright 2013 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. + import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; import 'package:ui/src/engine.dart';