-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Adds a call to the PlatformDispatcher whenever the focus changes
#151268
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds a call to the PlatformDispatcher whenever the focus changes
#151268
Conversation
| if (previousFocus != _primaryFocus) { | ||
| if (_primaryFocus != null && (_primaryFocus!.context?.mounted ?? false)) { | ||
| final int? viewId = View.maybeOf(_primaryFocus!.context!)?.viewId; | ||
| if (viewId != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If viewId is null then we need to let the engine know that the previously focused view (that's what the latest focused view bit is for) lost focus, hence it should be blurred. See https://github.com/flutter/engine/blob/main/lib/web_ui/lib/src/engine/platform_dispatcher/view_focus_binding.dart#L57
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the view id is null, then this widget isn't in a view anymore, and shouldn't be receiving focus, much less notifying the engine of its focus change. This state (the view being null) should probably never happen, since anything that is no longer part of a view should be disposed before it gets here, and thus never have a chance to become the primary focus. The only node in the focus tree that this might legitimately happen to is the root node, which doesn't have a context and so would also not report.
We could keep track of what the view used to be the last time any node reported a focus change, but then why not just use the one kept in the engine, since the engine is controlling which view is focused anyhow?
1bcc7d0 to
caf4fc4
Compare
|
Okay @tugorez, this now does what you described. I still have to write a test for it, but take a look. |
caf4fc4 to
fdada0e
Compare
| _dirtyNodes.clear(); | ||
| if (previousFocus != _primaryFocus) { | ||
| if (_primaryFocus != null && (_primaryFocus!.context?.mounted ?? false)) { | ||
| final int? viewId = View.maybeOf(_primaryFocus!.context!)?.viewId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO _lastFocusedViewId should be updated here, not by using the engine events. This is because we have two distinct concepts of "focus":
-
Flutter Focus: Managed by the framework, this represents the focus state within the Flutter widget tree.
-
Platform Focus: Managed by the platform (iOS, Android, Web), this represents the actual focus state of the "native" views.
These two can diverge, but we reconcile them using the view focus methods in the platform dispatcher. Updating _lastFocusedViewId here aligns with the idea that we're working with the Flutter focus concept.
|
This LGTM. Thanks Greg :)! |
5c8f753 to
1348f58
Compare
|
@tugorez I had to abandon the previous method of doing this because it was trying to look up deactivated elements in between frames. Instead, I'm doing all of the work in the Please take another look. Thanks! |
ec67d97 to
7e43fd7
Compare
044efe0 to
c9d65f6
Compare
431d094 to
ec3a0ef
Compare
ec3a0ef to
bed133b
Compare
Roll Flutter from 7d5c1c0 to 031dc3d (97 revisions) flutter/flutter@7d5c1c0...031dc3d 2024-07-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 342a42547822 to 354abf2800a0 (7 revisions) (flutter/flutter#152385) 2024-07-26 ryjohn@google.com Use more CORS headers for flutter run server (flutter/flutter#152249) 2024-07-26 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from 8714b54a87c0 to 342a42547822 (6 revisions) (flutter/flutter#152379) 2024-07-26 34465683+rkishan516@users.noreply.github.com feat: Add drag handle size to be configurable based on given size (flutter/flutter#152085) 2024-07-26 matanlurey@users.noreply.github.com Add and use an integration test with native(ADB) screenshots (flutter/flutter#152326) 2024-07-26 engine-flutter-autoroll@skia.org Roll Packages from 19daf6f to 3d358d9 (4 revisions) (flutter/flutter#152372) 2024-07-26 32538273+ValentinVignal@users.noreply.github.com Add test for range_slider.0.dart (flutter/flutter#152152) 2024-07-26 tessertaha@gmail.com Introduce `TabBar.indicatorAnimation` to customize tab indicator animation (flutter/flutter#151746) 2024-07-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 21629ece8b72 to 8714b54a87c0 (3 revisions) (flutter/flutter#152351) 2024-07-26 kevmoo@users.noreply.github.com Cleanup examples/api web load logic to latest (flutter/flutter#152349) 2024-07-26 gspencergoog@users.noreply.github.com Adds a call to the `PlatformDispatcher` whenever the focus changes (flutter/flutter#151268) 2024-07-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from d665bf82dc32 to 21629ece8b72 (4 revisions) (flutter/flutter#152344) 2024-07-25 31859944+LongCatIsLooong@users.noreply.github.com `docImport`s for the widgets library (flutter/flutter#152339) 2024-07-25 jacksongardner@google.com Set dart defines properly while in debug mode. (flutter/flutter#152262) 2024-07-25 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.25.13 to 3.25.14 (flutter/flutter#152342) 2024-07-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from 74785a771ae6 to d665bf82dc32 (2 revisions) (flutter/flutter#152340) 2024-07-25 leroux_bruno@yahoo.fr Cleanup InputDecoration.collapsed constructor (flutter/flutter#152165) 2024-07-25 32538273+ValentinVignal@users.noreply.github.com Add test for expansion_panel_list.expansion_panel_list_radio.0_test.dart (flutter/flutter#151730) 2024-07-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from f862a620cee4 to 74785a771ae6 (2 revisions) (flutter/flutter#152333) 2024-07-25 engine-flutter-autoroll@skia.org Roll Packages from 1c319ac to 19daf6f (3 revisions) (flutter/flutter#152327) 2024-07-25 31859944+LongCatIsLooong@users.noreply.github.com Add a more typical / concrete example to IntrinsicHeight / IntrinsicWidth (flutter/flutter#152246) 2024-07-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from f47b4d8e145a to f862a620cee4 (1 revision) (flutter/flutter#152320) 2024-07-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from 74737820a8ee to f47b4d8e145a (7 revisions) (flutter/flutter#152314) 2024-07-25 42016383+DBowen33@users.noreply.github.com Flutter Web App: adds a11y semantic attributes to slider (flutter/flutter#151985) 2024-07-25 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from eb8fac2b1703 to 74737820a8ee (8 revisions) (flutter/flutter#152305) 2024-07-25 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from eb8fac2b1703 to a57655cccb55 (6 revisions) (#152293)" (flutter/flutter#152304) 2024-07-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from eb8fac2b1703 to a57655cccb55 (6 revisions) (flutter/flutter#152293) 2024-07-25 biggs0125@gmail.com Modify stepping integration test to accommodate new DDC async semantics. (flutter/flutter#152204) 2024-07-25 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from eb8fac2b1703 to e1259b86ba02 (2 revisions) (#152285)" (flutter/flutter#152289) 2024-07-25 biggs0125@gmail.com Update fake_codec.dart to use Future.value instead of SynchronousFuture (flutter/flutter#152182) 2024-07-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from eb8fac2b1703 to e1259b86ba02 (2 revisions) (flutter/flutter#152285) 2024-07-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4b952093cb99 to eb8fac2b1703 (3 revisions) (flutter/flutter#152278) 2024-07-24 dkwingsmt@users.noreply.github.com [CupertinoAlertDialog] Rewrite (flutter/flutter#150410) 2024-07-24 victorsanniay@gmail.com Revert "Make `CupertinoRadio`'s `mouseCursor` a `WidgetStateProperty`" (flutter/flutter#152254) 2024-07-24 rmolivares@renzo-olivares.dev Fix: A selectable's selection under the active selection should not be cleared on right-click (flutter/flutter#151851) 2024-07-24 fluttergithubbot@gmail.com Marks Mac channels_integration_test to be flaky (flutter/flutter#151882) 2024-07-24 engine-flutter-autoroll@skia.org Roll Flutter Engine from c2f489d783d6 to 4b952093cb99 (3 revisions) (flutter/flutter#152264) 2024-07-24 42016383+DBowen33@users.noreply.github.com added Semantics label to TextField with InputDecoration to let user k� (flutter/flutter#151996) 2024-07-24 jasonkang14@gmail.com feat: Add alignmentOffset to DropdownMenu (flutter/flutter#151731) 2024-07-24 engine-flutter-autoroll@skia.org Roll Flutter Engine from 490576daf810 to c2f489d783d6 (6 revisions) (flutter/flutter#152260) 2024-07-24 matanlurey@users.noreply.github.com Scaffolding for `NativeDriver` and `AndroidNativeDriver` for taking screenshots using `adb`. (flutter/flutter#152194) 2024-07-24 31859944+LongCatIsLooong@users.noreply.github.com `widgets` docImport (flutter/flutter#152146) 2024-07-24 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3078f6a90e71 to 490576daf810 (1 revision) (flutter/flutter#152239) 2024-07-24 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Use more CORS headers for `flutter run` server (#152048)" (flutter/flutter#152248) 2024-07-24 ryjohn@google.com Use more CORS headers for `flutter run` server (flutter/flutter#152048) 2024-07-24 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Enable Swift Package Manager by default on master channel (#152049)" (flutter/flutter#152243) ...
…lutter#151268) ## Description This adds a call to the `PlatformDispatcher` whenever the focus changes, so that the engine can decide what to do about view focus. This lets widgets use autofocus, and when they are focused their view will also receive focus. ## Related Issues - Fixes flutter#151251 ## Tests - Added a test and some methods to the `TestPlatformDispatcher` to allow introspection of the values sent.
…lutter#151268) ## Description This adds a call to the `PlatformDispatcher` whenever the focus changes, so that the engine can decide what to do about view focus. This lets widgets use autofocus, and when they are focused their view will also receive focus. ## Related Issues - Fixes flutter#151251 ## Tests - Added a test and some methods to the `TestPlatformDispatcher` to allow introspection of the values sent.
Description
This adds a call to the
PlatformDispatcherwhenever the focus changes, so that the engine can decide what to do about view focus. This lets widgets use autofocus, and when they are focused their view will also receive focus.Related Issues
Tests
TestPlatformDispatcherto allow introspection of the values sent.