Getting rid of containers #147432

nate-thegrate · 2024-04-26T19:36:02Z

This pull request aims to make the Flutter framework more efficient, based on issue #147431.

Going into it, I didn't expect to spend 4 hours updating tests, but apparently find.byType(Container) is very convenient and was used a lot.

packages/flutter/lib/src/cupertino/context_menu.dart

goderbauer

LGTM

auto-submit · 2024-05-07T22:39:23Z

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

jmagman · 2024-05-08T22:50:31Z

The Google testing failures look related to this PR:

Expected: exactly 2 matching candidates
  Actual: _TypeWidgetFinder:<Found 1 widget with type
"Container": [
            Container(bg: BoxDecoration(color: Color(0xffe8f0fe),
border: Border.all(BorderSide(color: Color(0x00000000))),
borderRadius: BorderRadius.circular(8.0))),
          ]>
   Which: is not enough

and another is failing at

final BuildContext context = tester.element(find.byType(Container));

The following StateError was thrown running a test:
Bad state: No element

@goderbauer could you help with the g3fix, or whatever would be needed to get those tests passing?

goderbauer · 2024-05-09T19:29:21Z

I am gonna rebase this to kick the checks again.

goderbauer · 2024-05-09T19:30:11Z

Actually, the github UI isn't allowing me to do a rebase. @nate-thegrate Can you rebase this with the latest main branch?

nate-thegrate · 2024-05-09T19:30:28Z

Done!

(Edit: that was a merge commit, not a rebase… I'm pretty sure autosubmit does a "squash and merge", so at least the main branch won't be as messy as this PR 🙂)

nate-thegrate · 2024-05-09T22:31:39Z

Merging per the recent Discord message:

christopherfujino: I am actively investigating autosubmit not working (in #148092 and a thread in this chat)—in the meanwhile, if:

all presubmits are green (including tree status)

PR has two approving Flutter GitHub org members (including the author if applicable)

Then Flutter GitHub org members should feel empowered to manually merge PRs, until #148092 is fixed.

flutter/flutter@2bfb1b0...2aa05c1 2024-05-11 [email protected] Roll Flutter Engine from fad88cb16d03 to 558a81dd8b08 (3 revisions) (flutter/flutter#148163) 2024-05-11 [email protected] Roll Flutter Engine from ba8e0d3e2f23 to fad88cb16d03 (9 revisions) (flutter/flutter#148156) 2024-05-11 [email protected] Add test for scaffold.1.dart (flutter/flutter#147966) 2024-05-10 [email protected] Fix `MaterialStateBorderSide` lerp in the `Checkbox` and chips (flutter/flutter#148124) 2024-05-10 [email protected] Docs on TextField disposed by a scrollable (flutter/flutter#148149) 2024-05-10 [email protected] Roll Flutter Engine from d4f705ccb695 to ba8e0d3e2f23 (8 revisions) (flutter/flutter#148147) 2024-05-10 [email protected] Roll pub packages (flutter/flutter#148148) 2024-05-10 [email protected] Add `clipBehavior` to `DialogTheme` (flutter/flutter#147635) 2024-05-10 [email protected] bump cupertino_icons to 1.08 (flutter/flutter#146806) 2024-05-10 [email protected] Add test for animated_size.0.dart API example. (flutter/flutter#147828) 2024-05-10 [email protected] Fix `DropdownMenu` keyboard navigation (flutter/flutter#147294) 2024-05-10 [email protected] Add test for draggable.0.dart API example. (flutter/flutter#147941) 2024-05-10 [email protected] Update TESTOWNERS (flutter/flutter#148108) 2024-05-10 [email protected] Add tests for stream_builder.0.dart API example. (flutter/flutter#147832) 2024-05-10 [email protected] Roll Flutter Engine from 1ccd0c308b3a to d4f705ccb695 (2 revisions) (flutter/flutter#148142) 2024-05-10 [email protected] Roll Packages from 8de142d to 6c4482a (8 revisions) (flutter/flutter#148079) 2024-05-10 [email protected] Roll Flutter Engine from c0917b14fc36 to 1ccd0c308b3a (10 revisions) (flutter/flutter#148137) 2024-05-10 [email protected] `if` chains â�� `switch` expressions (flutter/flutter#147793) 2024-05-10 49699333+dependabot[bot]@users.noreply.github.com Bump ossf/scorecard-action from 2.3.1 to 2.3.3 (flutter/flutter#148091) 2024-05-10 [email protected] Reland "Implement computeDryBaseline for `RenderWrap` (#146260)" (flutter/flutter#148086) 2024-05-10 [email protected] Update dependabot reviewers (flutter/flutter#148101) 2024-05-10 [email protected] Roll Flutter Engine from 6e722ae213bd to c0917b14fc36 (1 revision) (flutter/flutter#148084) 2024-05-09 [email protected] Don't pin package:macros (flutter/flutter#148087) 2024-05-09 [email protected] Remove hidden dependencies on the default LocalPlatform (flutter/flutter#147342) 2024-05-09 [email protected] Getting rid of containers (flutter/flutter#147432) 2024-05-09 [email protected] Roll Flutter Engine from c0fd3386d018 to 6e722ae213bd (2 revisions) (flutter/flutter#148070) 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

This pull request follows up on [a PR from 4 months ago](#147432) that aimed to reduce the number of `Container` objects in the framework. I feel like now's a good time to wrap it up! (especially since I've gained a grasp of how "rebase" vs. "merge commit" can [affect test results](https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#using-git) ð��) <br> resolves #147431

nate-thegrate added 6 commits March 28, 2024 23:05

get rid of containers!

2df9506

fix analysis problems

88ef5c7

Merge github.com:flutter/flutter into no-more-containers

5b3fe23

Merge github.com:flutter/flutter into no-more-containers

2eb9b4d

more container refactoring!

f5868fb

Merge branch 'master' into no-more-containers

49c3c2d

goderbauer reviewed Apr 26, 2024

View reviewed changes

packages/flutter/lib/src/cupertino/context_menu.dart Outdated Show resolved Hide resolved

nate-thegrate added 19 commits April 26, 2024 16:52

revert DecoratedBox changes

5130eb8

update CupertinoListTile & relevant test

8c2e67e

fix CupertinoContextMenu test

8a0baa9

revert data_table.dart for now

532a819

update card_theme_test.dart

9fd914f

fix stepper_test.dart

50fcc9b

improve drawer.dart and relevant test

4087e62

fix card_test.dart

1943456

update dropdown_menu.dart and relevant test

a97d690

fix popup_menu_test

feeeaa9

remove redundant import

90614d8

more semantics!

571da17

pushed to the wrong branch 😑

b52f2ca

fix context_menu_test.dart

7bed677

list_section tweak

79a76d5

reverting CupertinoListTile & CupertinoListSection for now

2986d4d

Merge branch 'master' into no-more-containers

8130456

fix drawer_theme_test.dart

99c2c1b

fix navigation_drawer.dart and the relevant test

5c8c311

nate-thegrate added 3 commits May 6, 2024 19:25

Merge github.com:flutter/flutter into no-more-containers

722f7dd

keep cupertino & material a-d, revert others

0dfb13d

improve date_picker.dart

5a76fd4

nate-thegrate marked this pull request as ready for review May 7, 2024 17:11

Merge github.com:flutter/flutter into no-more-containers

503d9f6

nate-thegrate requested a review from goderbauer May 7, 2024 18:39

goderbauer approved these changes May 7, 2024

View reviewed changes

nate-thegrate added the autosubmit Merge PR when tree becomes green via auto submit App label May 7, 2024

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

Merge branch 'master' into no-more-containers

5ac446b

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

nate-thegrate merged commit 94a7151 into flutter:master May 9, 2024

nate-thegrate deleted the no-more-containers branch May 9, 2024 23:56

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

Roll Flutter from 2bfb1b0e2f61 to 839ba6b3e395 (25 revisions) flutter/packages#6712

Closed

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

94a7151 Getting rid of containers (flutter/flutter#147432)

20666e9

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

Roll Flutter from 2bfb1b0e2f61 to 2aa05c16509b (26 revisions) flutter/packages#6713

Merged

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

94a7151 Getting rid of containers (flutter/flutter#147432)

243d9de

nate-thegrate added the refactor Improving readability/efficiency without behavioral changes label Jul 12, 2024

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

94a7151 Getting rid of containers (flutter/flutter#147432)

2a4bf38

nate-thegrate mentioned this pull request Aug 17, 2024

Factor out Container objects #153619

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Getting rid of containers #147432

Getting rid of containers #147432

Uh oh!

nate-thegrate commented Apr 26, 2024 •

edited

Loading

Uh oh!

Uh oh!

goderbauer left a comment

Uh oh!

auto-submit bot commented May 7, 2024

Uh oh!

jmagman commented May 8, 2024

Uh oh!

goderbauer commented May 9, 2024

Uh oh!

goderbauer commented May 9, 2024

Uh oh!

nate-thegrate commented May 9, 2024 •

edited

Loading

Uh oh!

nate-thegrate commented May 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Getting rid of containers #147432

Getting rid of containers #147432

Uh oh!

Conversation

nate-thegrate commented Apr 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented May 7, 2024

Uh oh!

jmagman commented May 8, 2024

Uh oh!

goderbauer commented May 9, 2024

Uh oh!

goderbauer commented May 9, 2024

Uh oh!

nate-thegrate commented May 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nate-thegrate commented May 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nate-thegrate commented Apr 26, 2024 •

edited

Loading

nate-thegrate commented May 9, 2024 •

edited

Loading