Factor out `RawView`, make `View` listen to engine generated view focus events #143259

gspencergoog · 2024-02-10T00:08:27Z

Description

This factors out a separate RawView that doesn't add a MediaQuery or a FocusScope. This PR also adds a new method WidgetsBindingObserver.didChangeViewFocus which allows the observer to know when the FlutterView that has focus has changed.

It also makes the View widget a stateful widget that contains a FocusScope and FocusTraversalGroup so that it can respond to changes in the focus of the view.

I've also added a new function to FocusScopeNode that will allow the scope node itself to be focused, without looking for descendants that could take the focus. This lets the focus be "parked" at the FocusManager.instance.rootScope so that nothing else appears to have focus.

Tests

Added tests for the new functionality.

ditman · 2024-03-11T19:29:44Z

I've deployed the latest version of this PR with a small multi-view test web app I have, and it works beautifully!

https://dit-multiview-tests.web.app (source)

goderbauer · 2024-03-25T22:02:54Z

Looks like this is failing many checks...

packages/flutter/lib/src/services/binding.dart

packages/flutter/lib/src/widgets/view.dart

packages/flutter/test/widgets/binding_test.dart

packages/flutter/lib/src/widgets/focus_manager.dart

gspencergoog · 2024-03-25T22:44:06Z

Wait... Something is very off about this PR. There's a lot of code in there that I didn't write for this PR. Let me try re-merging and push again.

ditman

Will look again later!

packages/flutter/lib/src/widgets/binding.dart

packages/flutter/lib/src/widgets/view.dart

ditman · 2024-03-25T22:51:04Z

(Also calling @tugorez since he worked on the engine side of things)

goderbauer

This pretty much looks good to me. I would like to see RawView get separated out of this and it looks like this is still failing tests.

tugorez · 2024-03-26T19:38:18Z

I've deployed the latest version of this PR with a small multi-view test web app I have, and it works beautifully!

https://dit-multiview-tests.web.app (source)

@ditman do you mind redeploying the app with the latest changes?

goderbauer

LGTM

packages/flutter/lib/src/widgets/binding.dart

packages/flutter/lib/src/widgets/view.dart

packages/flutter/lib/src/widgets/binding.dart

auto-submit · 2024-03-26T20:20:58Z

auto label is removed for flutter/flutter/143259, due to - The status or check suite Mac customer_testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

ditman · 2024-03-27T01:23:00Z

I've deployed the latest version of this PR with a small multi-view test web app I have, and it works beautifully!

https://dit-multiview-tests.web.app (source)

@ditman do you mind redeploying the app with the latest changes?

Redeployed. Apologies for the delay @tugorez!

ditman · 2024-03-27T01:38:05Z

I took a quick look at the failing web tests, and most of them (all?) seem to be related to the changes in the widget tree, and tests having "golden" expectations of full debugWidgetTrees or whatever we call this:

Expected: normalized value matches RenderPadding#00000 relayoutBoundary=up1
           │ creator: Padding ← Container ← Align ← MediaQuery ←
           │   _MediaQueryFromView ← _PipelineOwnerScope ← _ViewScope ←
           │   _RawView-[_DeprecatedRawViewKey TestFlutterView#00000] ← View ←
           │   [root]
           │ parentData: offset=Offset(0.0, 0.0) (can use size)
           │ constraints: BoxConstraints(0.0<=w<=800.0, 0.0<=h<=600.0)
           │ size: Size(63.0, 88.0)
           │ padding: EdgeInsets.all(5.0)
           │
           └─child: RenderConstrainedBox#00000 relayoutBoundary=up2
             │ creator: ConstrainedBox ← Padding ← Container ← Align ←
             │   MediaQuery ← _MediaQueryFromView ← _PipelineOwnerScope ←
             │   _ViewScope ← _RawView-[_DeprecatedRawViewKey
             │   TestFlutterView#00000] ← View ← [root]
             │ parentData: offset=Offset(5.0, 5.0) (can use size)
             │ constraints: BoxConstraints(0.0<=w<=790.0, 0.0<=h<=590.0)
             │ size: Size(53.0, 78.0)
             │ additionalConstraints: BoxConstraints(w=53.0, h=78.0)
             │
             └─child: RenderDecoratedBox#00000
               │ creator: DecoratedBox ← ConstrainedBox ← Padding ← Container ←
               │   Align ← MediaQuery ← _MediaQueryFromView ← _PipelineOwnerScope
               │   ← _ViewScope ← _RawView-[_DeprecatedRawViewKey
               │   TestFlutterView#00000] ← View ← [root]
               │ parentData: <none> (can use size)
               │ constraints: BoxConstraints(w=53.0, h=78.0)
               │ size: Size(53.0, 78.0)
               │ decoration: BoxDecoration:
   .... (and much more)

I can help updating the "goldens" if needed, do ping me if we want to fix like that, or there's another way I don't see :)

tugorez · 2024-03-27T15:01:12Z

@gspencergoog one thing I notice while traversing multiple counters in https://dit-multiview-tests.web.app/ is that it requires two tabs to go from the button in a given view A to the button in the adjacent view B (either before or after A).

…52527) Put the view focus functionality behind a (disabled by default) flag Should unblock flutter/flutter#143259 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

tugorez · 2024-05-08T18:29:41Z

@gspencergoog I think this PR is ready to be landed.

auto-submit · 2024-05-17T22:01:06Z

auto label is removed for flutter/flutter/143259, due to - The status or check suite Linux analyze has failed. Please fix the issues identified (or deflake) before re-applying this label.

auto-submit · 2024-05-17T22:22:35Z

auto label is removed for flutter/flutter/143259, due to - The status or check suite Linux customer_testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

…view focus events (flutter/flutter#143259)

flutter/flutter@02a6c91...d02292d 2024-05-21 [email protected] Make FileSystem dependency explicit througout (more). (flutter/flutter#148095) 2024-05-20 [email protected] Remove add-to-app bitcode warning (flutter/flutter#148587) 2024-05-20 [email protected] SelectionArea's selection should not be cleared on loss of window focus (flutter/flutter#148067) 2024-05-20 [email protected] [wiki migration] Engine team pages (flutter/flutter#148696) 2024-05-20 [email protected] Manual roll camera dependency (flutter/flutter#148426) 2024-05-20 [email protected] [wiki migration] Framework team pages (flutter/flutter#148721) 2024-05-20 [email protected] Roll Flutter Engine from a8fb9daae8d0 to c2ef01f6f1ab (3 revisions) (flutter/flutter#148722) 2024-05-20 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.25.5 to 3.25.6 (flutter/flutter#148715) 2024-05-20 49699333+dependabot[bot]@users.noreply.github.com Bump codecov/codecov-action from 4.4.0 to 4.4.1 (flutter/flutter#148714) 2024-05-20 [email protected] Fixes incorrect read/write permissions on Flutter.framework and FlutterMacOS.framework (flutter/flutter#148580) 2024-05-20 [email protected] Roll Flutter Engine from c6fecf65fbf3 to a8fb9daae8d0 (3 revisions) (flutter/flutter#148700) 2024-05-20 [email protected] Remove the no-shuffle tag on the flutter_tools create_test suite (flutter/flutter#148688) 2024-05-20 [email protected] log incoming vm service messages in `FlutterVMService::runInView` (flutter/flutter#148596) 2024-05-20 [email protected] Add tests for shared_app_data.#.dart API examples. (flutter/flutter#147830) 2024-05-20 [email protected] Add tests for logical_key_set.0.dart API example. (flutter/flutter#147735) 2024-05-20 [email protected] [wiki migration] Ecosystem team pages (flutter/flutter#148589) 2024-05-20 [email protected] Fix painting API examples tests directories structure. (flutter/flutter#148177) 2024-05-20 [email protected] fixes `CupertinoModalPopupRoute` (flutter/flutter#147823) 2024-05-20 [email protected] Implement new `AnimationStatus` getters (flutter/flutter#148570) 2024-05-20 [email protected] Reland "`if` chains â�� `switch` expressions" (flutter/flutter#148634) 2024-05-20 [email protected] Factor out `RawView`, make `View` listen to engine generated view focus events (flutter/flutter#143259) 2024-05-20 [email protected] Remove all tests from a02s. Replace with mokey in bringup (flutter/flutter#148563) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…view focus events (flutter/flutter#143259)

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: focus Focus traversal, gaining or losing focus labels Feb 10, 2024

gspencergoog force-pushed the view_focus branch from 1bf9b21 to 000a5cf Compare March 8, 2024 22:00

This was referenced Mar 18, 2024

Make sure the framework correctly handles the view focus events. #144235

Closed

[Flutter Web] Create proof of concept wrappers for focus management #144853

Open

gspencergoog force-pushed the view_focus branch 2 times, most recently from 4d29b0b to f7b0216 Compare March 25, 2024 20:50

gspencergoog marked this pull request as ready for review March 25, 2024 20:50

gspencergoog requested review from ditman and goderbauer March 25, 2024 20:50

goderbauer reviewed Mar 25, 2024

View reviewed changes

ditman reviewed Mar 25, 2024

View reviewed changes

packages/flutter/lib/src/widgets/binding.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/widgets/view.dart Outdated Show resolved Hide resolved

ditman requested a review from tugorez March 25, 2024 22:50

gspencergoog force-pushed the view_focus branch from f7b0216 to 9a2e816 Compare March 25, 2024 22:56

goderbauer reviewed Mar 26, 2024

View reviewed changes

gspencergoog force-pushed the view_focus branch from b4c8307 to f811a65 Compare March 26, 2024 19:13

github-actions bot added the a: tests "flutter test", flutter_test, or one of our tests label Mar 26, 2024

gspencergoog changed the title ~~Make View listen to engine generated view focus events~~ Factor out RawView, make View listen to engine generated view focus events Mar 26, 2024

goderbauer approved these changes Mar 26, 2024

View reviewed changes

gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 26, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 26, 2024

gspencergoog force-pushed the view_focus branch from e97f696 to 0a1145a Compare May 8, 2024 19:21

github-actions bot added the f: material design flutter/packages/flutter/material repository. label May 8, 2024

gspencergoog force-pushed the view_focus branch from 338bb27 to b9caea7 Compare May 10, 2024 01:25

gspencergoog mentioned this pull request May 10, 2024

Remove fragile test flutter/devtools#7740

Merged

gspencergoog force-pushed the view_focus branch from b9caea7 to 6a68c4c Compare May 17, 2024 21:52

gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label May 17, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 17, 2024

gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label May 17, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 17, 2024

gspencergoog added 4 commits May 20, 2024 08:50

Make View listen to engine generated view focus events

ce6b032

Fix Tests

60bc235

Fix more tests.

21ff7ce

More tests fixed

3efe046

gspencergoog force-pushed the view_focus branch from 6a68c4c to 3efe046 Compare May 20, 2024 16:34

gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label May 20, 2024

auto-submit bot merged commit 333c076 into flutter:master May 20, 2024

engine-flutter-autoroll mentioned this pull request May 21, 2024

Roll Flutter from 02a6c91e4d37 to d02292dbc409 (22 revisions) flutter/packages#6778

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 21, 2024

333c076 Factor out RawView, make View listen to engine generated …

26e826e

…view focus events (flutter/flutter#143259)

polina-c mentioned this pull request May 21, 2024

Fix leaky test. #148788

Merged

tugorez mentioned this pull request May 22, 2024

[Flutter] Make the framework aware of which view has focus and publish changes through the platform dispatcher. #148890

Open

Piinks mentioned this pull request May 31, 2024

Flutter Framework Team: Notable Commits #121415

Closed

elliette mentioned this pull request Jun 11, 2024

Add back golden test to inspector_service_test flutter/devtools#7913

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 6, 2024

333c076 Factor out RawView, make View listen to engine generated …

be7f221

…view focus events (flutter/flutter#143259)

Factor out RawView, make View listen to engine generated view focus events #143259

Factor out RawView, make View listen to engine generated view focus events #143259

Uh oh!

Conversation

gspencergoog commented Feb 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Tests

Uh oh!

ditman commented Mar 11, 2024

Uh oh!

goderbauer commented Mar 25, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gspencergoog commented Mar 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ditman left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ditman commented Mar 25, 2024

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

tugorez commented Mar 26, 2024

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

auto-submit bot commented Mar 26, 2024

Uh oh!

ditman commented Mar 27, 2024

Uh oh!

ditman commented Mar 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tugorez commented Mar 27, 2024

Uh oh!

tugorez commented May 8, 2024

Uh oh!

auto-submit bot commented May 17, 2024

Uh oh!

auto-submit bot commented May 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Factor out `RawView`, make `View` listen to engine generated view focus events #143259

Factor out `RawView`, make `View` listen to engine generated view focus events #143259

gspencergoog commented Feb 10, 2024 •

edited

Loading

gspencergoog commented Mar 25, 2024 •

edited

Loading

ditman left a comment •

edited

Loading

ditman commented Mar 27, 2024 •

edited

Loading