Treat hidden `IndexedStack` children as offstage for test finder by simolus3 · Pull Request #123129 · flutter/flutter

simolus3 · 2023-03-21T16:34:19Z

This PR is a duplicate of #111479, I've copied the description here:

An IndexedStack only ever shows one of its children. Other children aren't rendered or hit-tested. In this sense, I think these other children could be considered off-stage.
This PR changes the behavior of IndexedStack to hide inactive children from an element visitor only interested in on-stage children. The main effect is that inactive children are no longer found through a finder in widget tests by default.

This closes #111478.

I was asked to re-open this PR so that the impact of this breaking change can be evaluated (cc @goderbauer).

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

fzyzcjy · 2023-03-22T00:24:47Z

For future readers: Context #122393 (comment)

goderbauer · 2023-03-22T22:54:27Z

I've kicked off an internal test run, let's see what happens...

goderbauer

Please also add tests that specifically test the new behavior of the offstage behavior of IndexStack.

goderbauer · 2023-03-23T22:54:04Z

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

This will consider all children onstage, right? I though if the index is null, the IndexedStack displays nothing, so all of them should be offstage?

goderbauer · 2023-03-23T22:59:21Z

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

couldn't this be simplified to

Suggested change

final Iterator<Element> onlyOnstageChild = children.skip(index).iterator;

if (onlyOnstageChild.moveNext()) {

visitor(onlyOnstageChild.current);

}

visitor(children.elementAt(index));

I had to special-case the scenario where index is out of bounds because there are no children (the default when constructing an IndexedStack()), but elementAt is much more readable. Thanks for the suggestion!

goderbauer · 2023-03-23T23:05:54Z

From initial testing, it doesn't look too bad. Once the comment about the index == null case is addressed, I can run some additional tests to get the full picture.

goderbauer

LGTM, but we have to wait and see what Google Testing does

goderbauer · 2023-03-28T23:53:16Z

@simolus3 something got messed up when I tried to rebase this PR and a lot of checks are failing now. If you have a chance, could you take a look?

goderbauer · 2023-03-29T16:40:00Z

The customer_testing checks are failing because of #123669.

goderbauer · 2023-03-29T19:55:32Z

The handful of google tests that were failing on this have been migrated, so all checks are green on this PR now. We just need to get a secondary review on this to land it.

Piinks

…der (flutter/flutter#123129)

…der (#123129)" This reverts commit c71f1dd.

CaseyHillers · 2023-04-03T17:08:04Z

As FYI, this is breaking Google Testing, and I've put up a revert in #124035. I'm checking with @goderbauer and @Piinks to see if we can fix forward without the need to revert this change.

goderbauer · 2023-04-03T17:10:43Z

A g3fix for that breakage is scheduled. No further action should be necessary.

…tter#123129) Treat hidden `IndexedStack` children as offstage for test finder

…der (flutter/flutter#123129)

simolus3 mentioned this pull request Mar 21, 2023

Fix: IndexedStack says its children are offstage when dumping render tree, but they are indeed onstage and users get confused #122393

Closed

8 tasks

goderbauer self-requested a review March 21, 2023 22:11

goderbauer force-pushed the indexed-stack-hide-from-finder-v2 branch from dfc5539 to df434c4 Compare March 23, 2023 22:09

goderbauer reviewed Mar 23, 2023

View reviewed changes

simolus3 force-pushed the indexed-stack-hide-from-finder-v2 branch from df434c4 to 623980f Compare March 24, 2023 20:51

goderbauer approved these changes Mar 24, 2023

View reviewed changes

gnprice mentioned this pull request Mar 25, 2023

Google testing fails to report status to GitHub #123289

Closed

goderbauer force-pushed the indexed-stack-hide-from-finder-v2 branch from 623980f to 1e2364a Compare March 28, 2023 22:45

simolus3 added 3 commits March 29, 2023 11:02

IndexedStack treats inactive children as offstage

aa4ea14

Add test for offstage IndexedStack behavior

dfc3ec4

Migrate element to _RawIndexedStack

78c70a8

goderbauer force-pushed the indexed-stack-hide-from-finder-v2 branch from 84fe65c to 78c70a8 Compare March 29, 2023 18:03

Piinks approved these changes Mar 29, 2023

View reviewed changes

Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 29, 2023

auto-submit bot merged commit c71f1dd into flutter:master Mar 29, 2023

simolus3 deleted the indexed-stack-hide-from-finder-v2 branch March 29, 2023 21:27

engine-flutter-autoroll mentioned this pull request Mar 30, 2023

Roll Flutter from 12b36de8f7ac to 7b54a30e317c (43 revisions) flutter/packages#3591

Closed

engine-flutter-autoroll mentioned this pull request Mar 30, 2023

Roll Flutter from 12b36de8f7ac to 8afd600e79b4 (44 revisions) flutter/packages#3592

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 30, 2023

c71f1dd Treat hidden IndexedStack children as offstage for test fin…

e866b8b

…der (flutter/flutter#123129)

engine-flutter-autoroll mentioned this pull request Mar 30, 2023

Roll Flutter from 12b36de8f7ac to 302c087738a4 (46 revisions) flutter/packages#3593

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 30, 2023

c71f1dd Treat hidden IndexedStack children as offstage for test fin…

e0ad5b9

…der (flutter/flutter#123129)

engine-flutter-autoroll mentioned this pull request Mar 30, 2023

Roll Flutter from 12b36de8f7ac to 8c5d223bab70 (49 revisions) flutter/packages#3594

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 30, 2023

c71f1dd Treat hidden IndexedStack children as offstage for test fin…

09699fe

…der (flutter/flutter#123129)

engine-flutter-autoroll mentioned this pull request Mar 30, 2023

Roll Flutter from 12b36de8f7ac to 5f7117948072 (50 revisions) flutter/packages#3595

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 30, 2023

c71f1dd Treat hidden IndexedStack children as offstage for test fin…

d18c6c3

…der (flutter/flutter#123129)

engine-flutter-autoroll mentioned this pull request Mar 30, 2023

Roll Flutter from 12b36de8f7ac to a28aae918eae (51 revisions) flutter/packages#3597

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 30, 2023

c71f1dd Treat hidden IndexedStack children as offstage for test fin…

402d73c

…der (flutter/flutter#123129)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023

c71f1dd Treat hidden IndexedStack children as offstage for test fin…

7fb9a84

…der (flutter/flutter#123129)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023

c71f1dd Treat hidden IndexedStack children as offstage for test fin…

974f06c

…der (flutter/flutter#123129)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023

c71f1dd Treat hidden IndexedStack children as offstage for test fin…

074e4be

…der (flutter/flutter#123129)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023

c71f1dd Treat hidden IndexedStack children as offstage for test fin…

2648425

…der (flutter/flutter#123129)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023

c71f1dd Treat hidden IndexedStack children as offstage for test fin…

bf9d700

…der (flutter/flutter#123129)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023

c71f1dd Treat hidden IndexedStack children as offstage for test fin…

e657a5c

…der (flutter/flutter#123129)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023

c71f1dd Treat hidden IndexedStack children as offstage for test fin…

014fbed

…der (flutter/flutter#123129)

reidbaker mentioned this pull request Apr 1, 2023

Add Java-Gradle-AGP validation to flutter analyze #123916

Merged

8 tasks

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2023

c71f1dd Treat hidden IndexedStack children as offstage for test fin…

c23266b

…der (flutter/flutter#123129)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2023

c71f1dd Treat hidden IndexedStack children as offstage for test fin…

4c8fa25

…der (flutter/flutter#123129)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2023

c71f1dd Treat hidden IndexedStack children as offstage for test fin…

1d24fc5

…der (flutter/flutter#123129)

CaseyHillers added a commit that referenced this pull request Apr 3, 2023

Revert "Treat hidden IndexedStack children as offstage for test fin…

8014722

…der (#123129)" This reverts commit c71f1dd.

CaseyHillers mentioned this pull request Apr 3, 2023

Revert "Treat hidden IndexedStack children as offstage for test finder" #124035

Closed

reidbaker mentioned this pull request Apr 5, 2023

Update getGradleVersion to ignore commented out lines #124260

Merged

8 tasks

exaby73 pushed a commit to NevercodeHQ/flutter that referenced this pull request Apr 17, 2023

Treat hidden IndexedStack children as offstage for test finder (flu…

ce31d53

…tter#123129) Treat hidden `IndexedStack` children as offstage for test finder

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

c71f1dd Treat hidden IndexedStack children as offstage for test fin…

0d6e09c

…der (flutter/flutter#123129)

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

c71f1dd Treat hidden IndexedStack children as offstage for test fin…

4110c83

…der (flutter/flutter#123129)

marcossevilla mentioned this pull request Jul 17, 2023

chore: migrate to flutter 3.10.0 marcossevilla/lazy_indexed_stack#20

Merged

7 tasks

-      final Iterator<Element> onlyOnstageChild = children.skip(index).iterator;
-      if (onlyOnstageChild.moveNext()) {
-        visitor(onlyOnstageChild.current);
-      }
+      visitor(children.elementAt(index));

Conversation

simolus3 commented Mar 21, 2023

Pre-launch Checklist

Uh oh!

fzyzcjy commented Mar 22, 2023

Uh oh!

goderbauer commented Mar 22, 2023

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

goderbauer Mar 23, 2023

Choose a reason for hiding this comment

Uh oh!

goderbauer Mar 23, 2023

Choose a reason for hiding this comment

Uh oh!

simolus3 Mar 24, 2023

Choose a reason for hiding this comment

Uh oh!

goderbauer commented Mar 23, 2023

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

goderbauer commented Mar 28, 2023

Uh oh!

goderbauer commented Mar 29, 2023

Uh oh!

goderbauer commented Mar 29, 2023

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

CaseyHillers commented Apr 3, 2023

Uh oh!

goderbauer commented Apr 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants