-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Changes the Linear Gradient on the CupertinoPicker from fixed white t… #16331
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
Changes the Linear Gradient on the CupertinoPicker from fixed white t… #16331
Conversation
…o the background color property
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
|
|
I signed it! |
|
CLAs look good, thanks! |
xster
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution. This looks sensible.
You can also remove the check on line 144 and 193 now.
|
|
||
| /// Makes the fade to white edge gradients. | ||
| /// Makes the fade to edge gradients according to the background color. | ||
| Widget _buildGradientScreen() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would avoid calling into this if the backgroundColor is transparent.
|
There might be a better fix involving a clever use of BlendModes... I think |
|
Do we want to go with the blendmode approach, or do we prefer the approach here of drawing a coloured gradient over the top? |
|
I don't know... I've been really busy lately and couldn't touch the code again. |
|
To me, the blend mode approach seems like the right (more general) one. |
|
It sounds like we're going to go with the blend approach so I'm going to close this PR for now. Thanks again for your contribution. If you want to try doing the blend approach and have time to do so, don't hesitate to submit a PR for that. Otherwise we'll get to it eventually, see #16831 to track the progress on that. Thanks! |
|
@Hixie Do we still want to work on this or are we keeping the linear, white based gradient? |
|
@xster Looks like that upstream blocker has indeed been resolved. As far as fixing this issue goes then, do you have a specific idea of what the blend modes should look like? Do you mean something like additive/multiply or something different? I think it could make sense to let the user optionally choose a blend color that is set to white by default. |
|
Since it's still an iOS picker, the gradient color is never different from the background color so we shouldn't let the user pick 2 colors. For the blend mode, see Ian's comment at #16331 (comment). Either dstIn or dstOut should work. It could just be a matter of specifying the backgroundBlendMode on BoxDecoration above the LinearGradient. |
|
@xster Do you know of any examples using the dstOut blend mode with the boxDecoration? Things aren't working as expected when I try to use it. The tests for boxDecoration are also pretty sparse. |
…o the background color property
The Problem
While using the CupertinoPicker widget, I noticed it adds a white Linear Gradient to make the fade effect. The problem is that for any background color, the gradient is always white. It also causes perceptible visual changes, even when using light colors.
e.g.
This picture is from the iOS Human Interface Guidelines
This other picture is from the current implementation on Flutter
There isn't a white shade on the first picture, but it is very present on the second one.
The biggest problem happens when we set a darker background color
The Fix
I have changed the Linear Gradient to use the widget's background color instead of white. That way, the gradient is correct for any background color
e.g.
This is the same picker after the fix
It will work for darker colors as well
Drawbacks