added scrimColor property in Scaffold widget#31025
added scrimColor property in Scaffold widget#31025HansMuller merged 14 commits intoflutter:masterfrom
scrimColor property in Scaffold widget#31025Conversation
ghost
added
f: material design
rami-a
left a comment
rami-a
left a comment
There was a problem hiding this comment.
Thanks for this PR, this looks great! Please take a look at and address my comments when you've got a chance.
- doc comments updated.
|
Looks like the analyzer job is still failing: White space in those 2 files on those lines. I think you just want to make sure there's no whitespace after the |
|
updated now! done |
rami-a
left a comment
rami-a
left a comment
There was a problem hiding this comment.
Awesome, looks good to me! Passing it off to @HansMuller for a final look
|
Hey @diegoveloper it looks like some tests are failing |
|
@rami-a I see the test failing is the I fixed the test, I found a bug in the change I made when there is no Color to |
| @override | ||
| void initState() { | ||
| super.initState(); | ||
| _color = ColorTween(begin: Colors.transparent, end: widget.scrimColor); |
There was a problem hiding this comment.
I believe you still want to keep a default color for widget.scrimColor, since the tween expects both arguments to not be null.
There was a problem hiding this comment.
you are right, done!
HansMuller
left a comment
HansMuller
left a comment
There was a problem hiding this comment.
This looks good. We're not asserting that scrimColor (et al) are non-null everywhere, which is OK. It would be a little simpler to let the scrimColor properties default to null and then in the one place where the color is needed, provide a default:
_color = ColorTween(begin: Colors.transparent, end: widget.scrimColor ?? Colors.black54);
Other than that, I think we should be about ready to land this change.
| }); | ||
| } | ||
|
|
||
| Container _getContainerScaffold(WidgetTester tester) { |
There was a problem hiding this comment.
There's no need to factor this out. It could be a local function within the one test that needs it, or it could be just be inlined there.
4 participants
Description
Based on this issue reported in StackOverflow: https://stackoverflow.com/questions/55669516/flutter-change-color-of-drawer-scrim, it is necessary to have the option to change the background color when we use the Drawer.
In Android we have the option to do it using this method https://developer.android.com/reference/android/support/v4/widget/DrawerLayout.html#setScrimColor(int)
Reviewing the source code of Scaffold and Drawer, I see that the color black54 was used by default with no option to change it.
With this change the color and also kept the black54 by default in case no value is entered.
Checklist
///).flutter analyze --flutter-repo) does not report any problems on my PR.