Skip to content

Add BottomSheetTheme to enable theming color, elevation, shape of BottomSheet - #31318

Merged
rami-a merged 8 commits into
flutter:masterfrom
rami-a:feature-30444-bottomsheet-shape
May 1, 2019
Merged

rami-a merged 8 commits into
flutter:masterfrom
rami-a:feature-30444-bottomsheet-shape

Conversation

@rami-a

@rami-a rami-a commented Apr 19, 2019 •

Copy link
Copy Markdown
Contributor

Description

This change introduces a BottomSheetTheme that allows you to theme color, elevation, and shape of BottomSheet. This can be done at the theme level and at call time of showBottomSheet and showModalBottomSheet. See the following image for an example (with some questionable design choices):

Related Issues

Closes #26854
Closes #30444

Tests

I added the following tests:

  • Tests for the theme and modifying the color/elevation/shape of the BottomSheet itself
  • Tests for ensuring the showBottomSheet and showModalBottomSheet pass through color/elevation/shape to the BottomSheet

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (Please read Handling breaking changes). Replace this with a link to the e-mail where you asked for input on this proposed change.
  • No, this is not a breaking change.

@rami-a rami-a changed the title Add BottomSheetTheme to enable theming color, elevation, shape of BottomSheet WIP Add BottomSheetTheme to enable theming color, elevation, shape of BottomSheet Apr 19, 2019
@goderbauer goderbauer added p: material_ui material_ui package in flutter/packages framework flutter/packages/flutter repository. See also f: labels. labels Apr 22, 2019
@rami-a
rami-a force-pushed the feature-30444-bottomsheet-shape branch from 825c6e5 to da9ee6b Compare April 22, 2019 16:57
@rami-a rami-a mentioned this pull request Apr 22, 2019
6 tasks done
@rami-a rami-a changed the title WIP Add BottomSheetTheme to enable theming color, elevation, shape of BottomSheet Add BottomSheetTheme to enable theming color, elevation, shape of BottomSheet Apr 23, 2019
@rami-a
rami-a marked this pull request as ready for review April 23, 2019 13:26

@johnsonmh johnsonmh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

Comment thread packages/flutter/lib/src/material/bottom_sheet.dart Outdated
Comment thread packages/flutter/lib/src/material/bottom_sheet.dart Outdated
Comment thread packages/flutter/lib/src/material/bottom_sheet.dart Outdated
Comment thread packages/flutter/lib/src/material/bottom_sheet_theme.dart Outdated
Comment thread packages/flutter/lib/src/material/bottom_sheet_theme.dart Outdated
Comment thread packages/flutter/lib/src/material/scaffold.dart
Comment thread packages/flutter/lib/src/material/scaffold.dart
@rami-a
rami-a requested a review from HansMuller April 23, 2019 18:36

@HansMuller HansMuller left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a nice simple addition. Mostly just some comments about testing.

Comment thread packages/flutter/lib/src/material/bottom_sheet_theme.dart
Comment thread packages/flutter/lib/src/material/bottom_sheet_theme.dart Outdated
Comment thread packages/flutter/lib/src/material/bottom_sheet_theme.dart Outdated
Comment thread packages/flutter/lib/src/material/bottom_sheet_theme.dart
Comment thread packages/flutter/test/material/modal_bottom_sheet_test.dart
@rami-a
rami-a requested a review from HansMuller April 30, 2019 21:34

@HansMuller HansMuller left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks pretty good; I think one final round or simplification is possible.

Comment thread packages/flutter/test/material/bottom_sheet_theme_test.dart Outdated
Comment thread packages/flutter/test/material/bottom_sheet_theme_test.dart Outdated
Comment thread packages/flutter/test/material/bottom_sheet_theme_test.dart Outdated
Comment thread packages/flutter/test/material/bottom_sheet_theme_test.dart
@rami-a
rami-a requested a review from HansMuller May 1, 2019 13:32

@HansMuller HansMuller left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@rami-a
rami-a merged commit 6a1468d into flutter:master May 1, 2019
@rami-a
rami-a deleted the feature-30444-bottomsheet-shape branch May 1, 2019 17:10
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

framework flutter/packages/flutter repository. See also f: labels. p: material_ui material_ui package in flutter/packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement shape support for BottomSheet Adding shape for bottom sheet

5 participants