Flutter views can gain focus #54985
Flutter views can gain focus #54985
Conversation
There was a problem hiding this comment.
This PR seems correct. What it does is to make the current flutter view focusable (so it can retain the focus if needed), but inaccessible by keyboard. See tabindex docs.
|
The only change I want to make is to rename the method a little bit, because all it seems to be doing is to modify the keyboard reachability of the view, but the name is more generic than what it actually does. |
|
I'm taking a quick stab at attaching the focus handlers to the flutter view root, rather than the body. |
|
Looks like I broke something in Safari with my latest changes, reviewing: |
lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart
Outdated
Show resolved
Hide resolved
lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart
Outdated
Show resolved
Hide resolved
| @@ -68,28 +68,6 @@ void testMain() { | |||
| expect(view2.dom.rootElement.getAttribute('tabindex'), '0'); | |||
| }); | |||
|
|
|||
| test('never marks the views as focusable with semantincs enabled', () async { | |||
There was a problem hiding this comment.
Should we also add some tests? :)
There was a problem hiding this comment.
Well, there's tests already (this file is just showing the removed test, but there's others to verify the rest of the functionality!)
|
One last update and applying autosubmit. |
|
auto label is removed for flutter/engine/54985, due to - The status or check suite Linux mac_android_aot_engine has failed. Please fix the issues identified (or deflake) before re-applying this label. |
auto-submit
bot
removed
the
autosubmit
I am unsure why the
tabindexwas removed when semantics were enabled. It seems this change was made without a clear explanation (by me). This PR shouldn't cause any issues as Flutter Views already have a tabindex, we're not adding a new one. This change is necessary because the semantics text strategy refocuses the view on deactivation, requiring the Flutter view to be focusable.ThIs PR is a requirement to enable #54966.
flutter/flutter#153022
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.