Skip to content
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

Getting rid of containers #147432

Merged
merged 44 commits into from May 9, 2024
Merged

Conversation

nate-thegrate
Copy link
Member

@nate-thegrate nate-thegrate commented Apr 26, 2024

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.

@github-actions github-actions bot added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. f: cupertino flutter/packages/flutter/cupertino repository labels Apr 26, 2024
@nate-thegrate
Copy link
Member Author

I'm going to try splitting this in half to make it easier to review. We'll see how it goes :)

@nate-thegrate nate-thegrate marked this pull request as draft May 6, 2024 21:35
@github-actions github-actions bot added the f: cupertino flutter/packages/flutter/cupertino repository label May 7, 2024
@nate-thegrate nate-thegrate marked this pull request as ready for review May 7, 2024 17:11
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nate-thegrate nate-thegrate added the autosubmit Merge PR when tree becomes green via auto submit App label May 7, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 7, 2024
Copy link
Contributor

auto-submit bot commented May 7, 2024

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
Copy link
Member

jmagman commented May 8, 2024

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
Copy link
Member

I am gonna rebase this to kick the checks again.

@goderbauer
Copy link
Member

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
Copy link
Member Author

nate-thegrate commented May 9, 2024

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 🙂)

@goderbauer goderbauer added the autosubmit Merge PR when tree becomes green via auto submit App label May 9, 2024
@nate-thegrate
Copy link
Member Author

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:

  1. all presubmits are green (including tree status)
  2. 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.

@nate-thegrate nate-thegrate merged commit 94a7151 into flutter:master May 9, 2024
73 checks passed
@nate-thegrate nate-thegrate deleted the no-more-containers branch May 9, 2024 23:56
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 11, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request May 11, 2024
flutter/flutter@2bfb1b0...2aa05c1

2024-05-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from fad88cb16d03 to 558a81dd8b08 (3 revisions) (flutter/flutter#148163)
2024-05-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from ba8e0d3e2f23 to fad88cb16d03 (9 revisions) (flutter/flutter#148156)
2024-05-11 32538273+ValentinVignal@users.noreply.github.com Add test for scaffold.1.dart (flutter/flutter#147966)
2024-05-10 tessertaha@gmail.com Fix `MaterialStateBorderSide` lerp in the `Checkbox` and chips (flutter/flutter#148124)
2024-05-10 jmccandless@google.com Docs on TextField disposed by a scrollable (flutter/flutter#148149)
2024-05-10 engine-flutter-autoroll@skia.org Roll Flutter Engine from d4f705ccb695 to ba8e0d3e2f23 (8 revisions) (flutter/flutter#148147)
2024-05-10 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#148148)
2024-05-10 32538273+ValentinVignal@users.noreply.github.com Add `clipBehavior` to `DialogTheme` (flutter/flutter#147635)
2024-05-10 31859944+LongCatIsLooong@users.noreply.github.com bump cupertino_icons to 1.08 (flutter/flutter#146806)
2024-05-10 sokolovskyi.konstantin@gmail.com Add test for animated_size.0.dart API example. (flutter/flutter#147828)
2024-05-10 120297255+PurplePolyhedron@users.noreply.github.com Fix `DropdownMenu` keyboard navigation (flutter/flutter#147294)
2024-05-10 sokolovskyi.konstantin@gmail.com Add test for draggable.0.dart API example. (flutter/flutter#147941)
2024-05-10 magder@google.com Update TESTOWNERS (flutter/flutter#148108)
2024-05-10 sokolovskyi.konstantin@gmail.com Add tests for stream_builder.0.dart API example. (flutter/flutter#147832)
2024-05-10 engine-flutter-autoroll@skia.org Roll Flutter Engine from 1ccd0c308b3a to d4f705ccb695 (2 revisions) (flutter/flutter#148142)
2024-05-10 engine-flutter-autoroll@skia.org Roll Packages from 8de142d to 6c4482a (8 revisions) (flutter/flutter#148079)
2024-05-10 engine-flutter-autoroll@skia.org Roll Flutter Engine from c0917b14fc36 to 1ccd0c308b3a (10 revisions) (flutter/flutter#148137)
2024-05-10 nate.w5687@gmail.com `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 31859944+LongCatIsLooong@users.noreply.github.com Reland "Implement computeDryBaseline for `RenderWrap` (#146260)" (flutter/flutter#148086)
2024-05-10 magder@google.com Update dependabot reviewers (flutter/flutter#148101)
2024-05-10 engine-flutter-autoroll@skia.org Roll Flutter Engine from 6e722ae213bd to c0917b14fc36 (1 revision) (flutter/flutter#148084)
2024-05-09 christopherfujino@gmail.com Don't pin package:macros (flutter/flutter#148087)
2024-05-09 ian@hixie.ch Remove hidden dependencies on the default LocalPlatform (flutter/flutter#147342)
2024-05-09 nate.w5687@gmail.com Getting rid of containers (flutter/flutter#147432)
2024-05-09 engine-flutter-autoroll@skia.org 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 bmparr@google.com,rmistry@google.com,stuartmorgan@google.com 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: text input Entering text in a text field or keyboard related problems autosubmit Merge PR when tree becomes green via auto submit App f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants