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

Add ThemeData.shadowColor #33257

Closed
wants to merge 1 commit into from
Closed

Conversation

lucasreiners
Copy link

@lucasreiners lucasreiners commented May 23, 2019

Description

This pull request adds shadowColor to the ThemeData to be able to override the default color of the elevation shadows. The property is used by the Material widget as a fallback, when no shadowColor is supplied to the Material widget. When the user did not override the shadowColor in the Theme, it defaults to black. This way we preserve the backward compatibility all the way to the Material widget.

The priority is the following, going to the next, when null:

  1. shadowColor property of Material Widget
  2. shadowColor property of ThemeData
  3. black (for backward compatibility)

Related Issues

#27461

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?

  • No, this is not a breaking change.

…Default stays black to preserve backward compatibility
@lucasreiners lucasreiners marked this pull request as ready for review May 23, 2019 14:35
@goderbauer goderbauer added the f: material design flutter/packages/flutter/material repository. label May 23, 2019
@jamesblasco
Copy link
Contributor

Well done!!! It would be very useful

@josh-burton
Copy link
Contributor

Would be great to see this merged. Any change of fixing those tests?

@gspencergoog
Copy link
Contributor

@LucasR93, are you still interested in pursuing this PR? If so, you would need to resolve the test failures, and the conflicting files.

Also, you will need to add a test for your change that fails without your change, and succeeds with it.

@Hixie
Copy link
Contributor

Hixie commented Jan 9, 2020

I suspect that @HansMuller may want the shadow color to go into the ColorScheme rather than the ThemeData?

@droplet-js
Copy link

Any news update?

@HansMuller
Copy link
Contributor

The material color scheme doesn't dictate the appearance of shadows. The approach taken here, adding a property to ThemeData, seems reasonable.

@LucasR93 - are you interested in returning to this PR?

@HansMuller HansMuller added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Jan 21, 2020
@frmatthew
Copy link

@LucasR93 Do you have any intention to revise this PR and get it merged?

@reminjp reminjp mentioned this pull request Jun 26, 2020
13 tasks
@goderbauer
Copy link
Member

I am going to close this PR since there hasn't been any activity since the beginning of the year. Feel free to re-open if you find the time to work on this in the future.

@goderbauer goderbauer closed this Jun 26, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
f: material design flutter/packages/flutter/material repository. waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants