Skip to content

Support Material 3 in bottom sheet#112466

Merged
auto-submit[bot] merged 10 commits intoflutter:masterfrom
hannah-hyj:M3-BottomSheets
Oct 11, 2022
Merged

Support Material 3 in bottom sheet#112466
auto-submit[bot] merged 10 commits intoflutter:masterfrom
hannah-hyj:M3-BottomSheets

Conversation

@hannah-hyj
Copy link
Member

@hannah-hyj hannah-hyj commented Sep 27, 2022 •
edited by guidezpl
Loading

issue: #111448, #111688

fixes #111448

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.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos documentation f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels Sep 27, 2022
@hannah-hyj hannah-hyj closed this Sep 27, 2022
@hannah-hyj hannah-hyj reopened this Sep 27, 2022
darrenaustin
darrenaustin suggested changes Sep 30, 2022
View reviewed changes
Copy link
Contributor

@darrenaustin darrenaustin left a comment

Choose a reason for hiding this comment

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

Nice work @Hangyujin. Just have a few suggestions and a question about the M2 defaults. Thx.

dev/tools/gen_defaults/lib/template.dart Outdated
Copy link
Contributor

@darrenaustin darrenaustin Sep 30, 2022

Choose a reason for hiding this comment

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

Nice optimization.

examples/api/lib/material/bottom_sheet/show_modal_bottom_sheet.1.dart Outdated
Copy link
Contributor

@darrenaustin darrenaustin Sep 30, 2022

Choose a reason for hiding this comment

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

I would just inline this and make it something like: 'Bottom Sheet Sample'.

Copy link
Member Author

@hannah-hyj hannah-hyj Oct 8, 2022

Choose a reason for hiding this comment

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

Done

packages/flutter/lib/src/material/bottom_sheet.dart Outdated
Copy link
Contributor

@darrenaustin darrenaustin Sep 30, 2022

Choose a reason for hiding this comment

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

Are the M2 defaults really just an empty BottomSheetThemeData? I am surprised we didn't have some static constants for some of these.

Copy link
Member Author

@hannah-hyj hannah-hyj Oct 8, 2022

Choose a reason for hiding this comment

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

Yeah I didn't find any default value for M2

packages/flutter/lib/src/material/bottom_sheet.dart Outdated
Copy link
Contributor

@darrenaustin darrenaustin Sep 30, 2022

Choose a reason for hiding this comment

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

The generated extra comma here would be nice to get rid of. Kind of a pain to do with the template code, but it would look better here.

Copy link
Member Author

@hannah-hyj hannah-hyj Oct 8, 2022

Choose a reason for hiding this comment

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

Done

darrenaustin
darrenaustin approved these changes Oct 11, 2022
View reviewed changes
Copy link
Contributor

@darrenaustin darrenaustin left a comment

Choose a reason for hiding this comment

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

LGTM.

@hannah-hyj hannah-hyj added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 11, 2022
@auto-submit auto-submit bot merged commit 4fed6aa into flutter:master Oct 11, 2022
This was referenced Oct 11, 2022
Merged
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 11, 2022
@engine-flutter-autoroll
4fed6aa Support Material 3 in bottom sheet (flutter/flutter#112466)
e1cf603
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Oct 11, 2022
@engine-flutter-autoroll
4fed6aa Support Material 3 in bottom sheet (flutter/flutter#112466)
e53755c
@TahaTesser TahaTesser mentioned this pull request Nov 28, 2022
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@QuncCccccc QuncCccccc Awaiting requested review from QuncCccccc

1 more reviewer

@darrenaustin darrenaustin darrenaustin approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App c: contributor-productivity Team-specific productivity, code health, technical debt. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Update BottomSheet to support Material 3

2 participants

@hannah-hyj @darrenaustin