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.

Sign up for GitHub

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

Jump to bottom

Update Material 3 bottom sheet #122445

Merged
merged 20 commits into from Mar 19, 2023
Merged

Update Material 3 bottom sheet #122445

merged 20 commits into from Mar 19, 2023

Conversation

hangyujin
Copy link
Contributor

issue:#111448

  1. Added a default max width as 640
  2. Added a drag handle, the user can also tap it to dismiss bottom sheet in a11y mode. Added drag handle color and size to bottom sheet theme.

571677645837_ pic

581677646439_ pic

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 f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. team Infra upgrades, team productivity, code health, technical debt. See also team: labels. labels Mar 11, 2023
@hangyujin hangyujin mentioned this pull request Mar 11, 2023
Closed
8 tasks
guidezpl
guidezpl requested changes Mar 13, 2023
View reviewed changes
Copy link
Member

@guidezpl guidezpl left a comment

Choose a reason for hiding this comment

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

Nice! Quick comments about docs and relation with enableDrag

packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved
@gnprice
Copy link
Member

gnprice commented Mar 13, 2023 •
edited

Everything LGTM now except what's covered in existing open threads:
#120855 (comment)
#122445 (comment)

I'll leave @HansMuller and @guidezpl to wrap those up and approve the PR when they think it's ready. Thanks for all your work on this!

@hangyujin hangyujin requested a review from guidezpl March 15, 2023 03:23
HansMuller
HansMuller reviewed Mar 15, 2023
View reviewed changes
packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved
dev/tools/gen_defaults/lib/bottom_sheet_template.dart Show resolved Hide resolved
packages/flutter/lib/src/material/bottom_sheet.dart Show resolved Hide resolved
packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved
guidezpl
guidezpl approved these changes Mar 16, 2023
View reviewed changes
Copy link
Member

@guidezpl guidezpl 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! Just minor nits about formatting and docs

  _    ___ _____ __  __ 
 | |  / __|_   _|  \/  |
 | |_| (_ | | | | |\/| |
 |____\___| |_| |_|  |_|

packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/material/bottom_sheet.dart Outdated Show resolved Hide resolved
@hangyujin hangyujin added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 16, 2023
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 17, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented Mar 17, 2023

auto label is removed for flutter/flutter, pr: 122445, due to - The status or check suite Windows web_tool_tests_1_2 has failed. Please fix the issues identified (or deflake) before re-applying this label.

hangyujin and others added 7 commits March 16, 2023 20:21
@hangyujin @guidezpl
Update packages/flutter/lib/src/material/bottom_sheet.dart
7eaf6a4 
Co-authored-by: Pierre-Louis <6655696+guidezpl@users.noreply.github.com>
@hangyujin @guidezpl
Update packages/flutter/lib/src/material/bottom_sheet.dart
8a5662f 
Co-authored-by: Pierre-Louis <6655696+guidezpl@users.noreply.github.com>
@hangyujin @guidezpl
Update packages/flutter/lib/src/material/bottom_sheet.dart
eb2bc0f 
Co-authored-by: Pierre-Louis <6655696+guidezpl@users.noreply.github.com>
@hangyujin @guidezpl
Update packages/flutter/lib/src/material/bottom_sheet.dart
df6e9c1 
Co-authored-by: Pierre-Louis <6655696+guidezpl@users.noreply.github.com>
@hangyujin @guidezpl
Update packages/flutter/lib/src/material/bottom_sheet.dart
2318b95 
Co-authored-by: Pierre-Louis <6655696+guidezpl@users.noreply.github.com>
@hangyujin @guidezpl
Update packages/flutter/lib/src/material/bottom_sheet.dart
a2c2c44 
Co-authored-by: Pierre-Louis <6655696+guidezpl@users.noreply.github.com>
@hangyujin
Update bottom_sheet.dart
2b10969 
Update bottom_sheet_test.dart

Update bottom_sheet.dart
@hangyujin hangyujin merged commit 9fc1fd1 into flutter:master Mar 19, 2023
128 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 19, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 19, 2023
@engine-flutter-autoroll
9fc1fd15f Update Material 3 bottom sheet (flutter/flutter#122445)
5228bd9
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 19, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 19, 2023
@engine-flutter-autoroll
9fc1fd15f Update Material 3 bottom sheet (flutter/flutter#122445)
eb26316
@gnprice gnprice mentioned this pull request Mar 20, 2023
Closed
8 tasks
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
@engine-flutter-autoroll
9fc1fd15f Update Material 3 bottom sheet (flutter/flutter#122445)
68799d4
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
@engine-flutter-autoroll
9fc1fd15f Update Material 3 bottom sheet (flutter/flutter#122445)
7d26a14
@TahaTesser TahaTesser mentioned this pull request May 17, 2023
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guidezpl guidezpl guidezpl approved these changes

@gnprice gnprice Awaiting requested review from gnprice

@HansMuller HansMuller Awaiting requested review from HansMuller

Assignees
No one assigned
Labels
f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. team Infra upgrades, team productivity, code health, technical debt. See also team: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@hangyujin @gnprice @HansMuller @guidezpl