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

SurfaceTintColor parameter added to control background color more clean #156

Merged
merged 2 commits into from
Mar 3, 2024

Conversation

yusufnadar
Copy link
Contributor

@yusufnadar yusufnadar commented Mar 2, 2024

Description

The surfaceTintColor parameter must be changed to see the same tone of color that is set for the background. Because the barrierColor value for the BottomSheet is defaulted to Colors.black54.
SurfaceTintColor can also be modified in WoltModalSheetThemeData for further customization.
Check how the white color looks on the background in the preview image.

Related Issues

Fix #139

Preview

Before After
Before After

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.
  • My PR includes tests for all changed/updated/fixed behaviors.
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (melos run analyze) does not report any problems on my PR.
  • The package compiles with the minimum Flutter version stated in the pubspec.yaml

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change.
  • No, this is not a breaking change.

@yusufnadar yusufnadar closed this Mar 2, 2024
@yusufnadar yusufnadar deleted the fix/surface_tint_color branch March 2, 2024 21:43
@yusufnadar yusufnadar restored the fix/surface_tint_color branch March 2, 2024 21:43
@yusufnadar yusufnadar deleted the fix/surface_tint_color branch March 2, 2024 21:44
@yusufnadar yusufnadar restored the fix/surface_tint_color branch March 2, 2024 21:57
@yusufnadar yusufnadar reopened this Mar 2, 2024
@ulusoyca
Copy link
Collaborator

ulusoyca commented Mar 3, 2024

Thanks for the PR. The surfaceTintColor is also used in WoltStickyActionBarWrapper. Can you also do the changes there?

/// If null, [WoltModalSheet] will not display an overlay color.
///
/// See [Material.surfaceTintColor] for more details.
final Color? surfaceTintColor;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like the documentation I provided in the existing source code is wrong. Let's fix this here, and in the old place. Here is my suggestion:

  /// The color of the surface tint overlay applied to the material color
  /// to indicate elevation for the modal sheet page. The [surfaceTintColor] is applied to the 
  /// modal sheet background color, top bar color, and the sticky action bar wrapper colors.
  ///
  /// {@template flutter.material.material.surfaceTintColor}
  /// Material Design 3 introduced a new way for some components to indicate
  /// their elevation by using a surface tint color overlay on top of the
  /// base material [color]. This overlay is painted with an opacity that is
  /// related to the [elevation] of the material.
  ///
  /// If [ThemeData.useMaterial3] is false, then this property is not used.
  ///
  /// If [ThemeData.useMaterial3] is true and [surfaceTintColor] is not null and
  /// not [Colors.transparent], then it will be used to overlay the base [backgroundColor]
  /// with an opacity based on the [modalElevation].
  ///
  /// If [ThemeData.useMaterial3] is true and [surfaceTintColor] is null, then the default 
  /// [surfaceTintColor] value is taken from the [ColorScheme].
  ///
  /// See also:
  ///
  ///   * [ThemeData.useMaterial3], which turns this feature on.
  ///   * [ElevationOverlay.applySurfaceTint], which is used to implement the
  ///     tint.
  ///   * https://m3.material.io/styles/color/the-color-system/color-roles
  ///     which specifies how the overlay is applied.

@yusufnadar yusufnadar force-pushed the fix/surface_tint_color branch from 6cbb21f to 983597c Compare March 3, 2024 08:09
Copy link
Collaborator

@ulusoyca ulusoyca left a comment

Choose a reason for hiding this comment

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

lgtm_6

@ulusoyca ulusoyca enabled auto-merge March 3, 2024 08:11
auto-merge was automatically disabled March 3, 2024 08:15

Head branch was pushed to by a user without write access

@ulusoyca ulusoyca enabled auto-merge March 3, 2024 08:18
@ulusoyca ulusoyca merged commit 35d0b52 into woltapp:main Mar 3, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

I found the background can not set as pure COlors.white?
2 participants