From eea5707bf5417bde7aeed722f6e4bad3458e97b4 Mon Sep 17 00:00:00 2001 From: Juan Tugores Date: Thu, 5 Sep 2024 15:18:39 -0700 Subject: [PATCH 1/5] Flutter view can gain focus. --- .../view_focus_binding.dart | 26 ++++++++----------- .../view_focus_binding_test.dart | 22 ---------------- 2 files changed, 11 insertions(+), 37 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 97aa20e408216..ce026e087cb88 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 @@ -115,8 +115,8 @@ final class ViewFocusBinding { direction: _viewFocusDirection, ); } - _maybeMarkViewAsFocusable(_lastViewId, reachableByKeyboard: true); - _maybeMarkViewAsFocusable(viewId, reachableByKeyboard: false); + _updateViewFocusStatus(_lastViewId, reachableByKeyboard: true); + _updateViewFocusStatus(viewId, reachableByKeyboard: false); _lastViewId = viewId; _onViewFocusChange(event); } @@ -127,10 +127,10 @@ final class ViewFocusBinding { } void _handleViewCreated(int viewId) { - _maybeMarkViewAsFocusable(viewId, reachableByKeyboard: true); + _updateViewFocusStatus(viewId, reachableByKeyboard: true); } - void _maybeMarkViewAsFocusable( + void _updateViewFocusStatus( int? viewId, { required bool reachableByKeyboard, }) { @@ -139,17 +139,13 @@ final class ViewFocusBinding { } 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); - } + // 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 fe0f9c5479ea3..7e7754836c1e8 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 @@ -68,28 +68,6 @@ 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.focusWithoutScroll(); - expect(view1.dom.rootElement.getAttribute('tabindex'), isNull); - expect(view2.dom.rootElement.getAttribute('tabindex'), isNull); - - view2.dom.rootElement.focusWithoutScroll(); - 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 b1c044d9896c7f36c141d0e30eefd3a8b3721300 Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Wed, 6 Nov 2024 10:48:10 -0800 Subject: [PATCH 2/5] Minor PR changes. --- .../view_focus_binding.dart | 36 ++++++++++++------- 1 file changed, 23 insertions(+), 13 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 ce026e087cb88..b36f2d299429b 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 @@ -27,10 +27,17 @@ final class ViewFocusBinding { StreamSubscription? _onViewCreatedListener; void init() { + // We need a global listener here to know if the user was pressing "shift" + // when the Flutter view receives focus, to move the Flutter focus to the + // *last* focusable element. domDocument.body?.addEventListener(_keyDown, _handleKeyDown); domDocument.body?.addEventListener(_keyUp, _handleKeyUp); + // Can these focus events be attached directly to the view, instead the body? domDocument.body?.addEventListener(_focusin, _handleFocusin); domDocument.body?.addEventListener(_focusout, _handleFocusout); + + // If so, update `_handleViewCreated` and add a `_handleViewDisposed` to attach + // and remove the focus/blur listener. _onViewCreatedListener = _viewManager.onViewCreated.listen(_handleViewCreated); } @@ -48,13 +55,14 @@ final class ViewFocusBinding { } final DomElement? viewElement = _viewManager[viewId]?.dom.rootElement; - if (state == ui.ViewFocusState.focused) { - // Only move the focus to the flutter view if nothing inside it is focused already. - if (viewId != _viewId(domDocument.activeElement)) { - viewElement?.focusWithoutScroll(); - } - } else { - viewElement?.blur(); + switch (state) { + case ui.ViewFocusState.unfocused: + // Only move the focus to the flutter view if nothing inside it is focused already. + if (viewId != _viewId(domDocument.activeElement)) { + viewElement?.focusWithoutScroll(); + } + case ui.ViewFocusState.focused: + viewElement?.blur(); } } @@ -115,8 +123,8 @@ final class ViewFocusBinding { direction: _viewFocusDirection, ); } - _updateViewFocusStatus(_lastViewId, reachableByKeyboard: true); - _updateViewFocusStatus(viewId, reachableByKeyboard: false); + _updateViewKeyboardReachability(_lastViewId, reachable: true); + _updateViewKeyboardReachability(viewId, reachable: false); _lastViewId = viewId; _onViewFocusChange(event); } @@ -127,12 +135,14 @@ final class ViewFocusBinding { } void _handleViewCreated(int viewId) { - _updateViewFocusStatus(viewId, reachableByKeyboard: true); + _updateViewKeyboardReachability(viewId, reachable: true); } - void _updateViewFocusStatus( + // Controls whether the Flutter view identified by [viewId] is reachable by + // keyboard. + void _updateViewKeyboardReachability( int? viewId, { - required bool reachableByKeyboard, + required bool reachable, }) { if (viewId == null) { return; @@ -145,7 +155,7 @@ final class ViewFocusBinding { // 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); + rootElement?.setAttribute('tabindex', reachable ? 0 : -1); } static const String _focusin = 'focusin'; From 8f843a75c03f0268711ee319ab444a5d0db06725 Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Mon, 11 Nov 2024 16:24:32 -0800 Subject: [PATCH 3/5] Attach focusin/out to the rootElement of the view. --- .../engine/platform_dispatcher/view_focus_binding.dart | 10 +++++----- 1 file changed, 5 insertions(+), 5 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 b36f2d299429b..5bb3bbed243f1 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 @@ -32,9 +32,6 @@ final class ViewFocusBinding { // *last* focusable element. domDocument.body?.addEventListener(_keyDown, _handleKeyDown); domDocument.body?.addEventListener(_keyUp, _handleKeyUp); - // Can these focus events be attached directly to the view, instead the body? - domDocument.body?.addEventListener(_focusin, _handleFocusin); - domDocument.body?.addEventListener(_focusout, _handleFocusout); // If so, update `_handleViewCreated` and add a `_handleViewDisposed` to attach // and remove the focus/blur listener. @@ -44,8 +41,6 @@ final class ViewFocusBinding { void dispose() { domDocument.body?.removeEventListener(_keyDown, _handleKeyDown); domDocument.body?.removeEventListener(_keyUp, _handleKeyUp); - domDocument.body?.removeEventListener(_focusin, _handleFocusin); - domDocument.body?.removeEventListener(_focusout, _handleFocusout); _onViewCreatedListener?.cancel(); } @@ -135,6 +130,11 @@ final class ViewFocusBinding { } void _handleViewCreated(int viewId) { + final DomElement? rootElement = _viewManager[viewId]?.dom.rootElement; + + rootElement?.addEventListener(_focusin, _handleFocusin); + rootElement?.addEventListener(_focusout, _handleFocusout); + _updateViewKeyboardReachability(viewId, reachable: true); } From c0964076b2523531bde4aab9350c2caaf270c6ab Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Mon, 11 Nov 2024 17:22:27 -0800 Subject: [PATCH 4/5] Use state properly in changeViewFocus. --- .../src/engine/platform_dispatcher/view_focus_binding.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 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 5bb3bbed243f1..c5ac287e91e48 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 @@ -51,12 +51,12 @@ final class ViewFocusBinding { final DomElement? viewElement = _viewManager[viewId]?.dom.rootElement; switch (state) { - case ui.ViewFocusState.unfocused: + case ui.ViewFocusState.focused: // Only move the focus to the flutter view if nothing inside it is focused already. if (viewId != _viewId(domDocument.activeElement)) { viewElement?.focusWithoutScroll(); } - case ui.ViewFocusState.focused: + case ui.ViewFocusState.unfocused: viewElement?.blur(); } } From d735a65a9736fdf1a7d274116c206e5645df54f2 Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Wed, 13 Nov 2024 13:58:54 -0800 Subject: [PATCH 5/5] Address PR comments. --- .../src/engine/platform_dispatcher/view_focus_binding.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 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 c5ac287e91e48..1a9bbb66018db 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 @@ -149,9 +149,9 @@ final class ViewFocusBinding { } final DomElement? rootElement = _viewManager[viewId]?.dom.rootElement; - // 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 + // A tabindex with value zero means the DOM element can be reached using the + // keyboard (tab, shift + tab). When its value is -1 it is still focusable + // but can't be focused as 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.