Skip to content

Conversation

@brunocalou
Copy link

…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

screen shot 2018-04-07 at 00 07 44

This other picture is from the current implementation on Flutter

screen shot 2018-04-07 at 00 09 14

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

screen shot 2018-04-07 at 00 37 58

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

screen shot 2018-04-07 at 00 09 38

It will work for darker colors as well

screen shot 2018-04-07 at 00 10 07

Drawbacks

  • Before the change, one could pass null to the backgroundColor. Now, it is not possible, since the Linear Gradient effect is mandatory and there is no way to apply it without a color
  • A transparent background color will render a black gradient, since Colors.transparent is defined as const Color(0x00000000)

@googlebot
Copy link

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. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@brunocalou
Copy link
Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels Apr 7, 2018
Copy link
Member

@xster xster left a 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() {
Copy link
Member

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.

@Hixie
Copy link
Contributor

Hixie commented Apr 10, 2018

There might be a better fix involving a clever use of BlendModes... I think dstOut is what you want here, with an alpha transparency mask (using alpha instead of the background color).
https://master-docs-flutter-io.firebaseapp.com/flutter/dart-ui/BlendMode-class.html
Then you would be able to handle a background image.

@Hixie
Copy link
Contributor

Hixie commented Apr 26, 2018

Do we want to go with the blendmode approach, or do we prefer the approach here of drawing a coloured gradient over the top?

@brunocalou
Copy link
Author

I don't know... I've been really busy lately and couldn't touch the code again.
What do you guys think?

@gspencergoog
Copy link
Contributor

To me, the blend mode approach seems like the right (more general) one.

@xster
Copy link
Member

xster commented May 18, 2018

I tried going with the blend approach for a bit (left #16831 for tracking) though I'm blocked by #14224 in the process. Nevertheless, I think it's the right approach too.

@Hixie
Copy link
Contributor

Hixie commented May 29, 2018

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 Hixie closed this May 29, 2018
@jslavitz jslavitz reopened this Oct 23, 2018
@jslavitz
Copy link
Contributor

@Hixie Do we still want to work on this or are we keeping the linear, white based gradient?

@xster
Copy link
Member

xster commented Oct 23, 2018

@jslavitz we can use your help with this one. Though with a slightly different approach using different blend modes instead of drawing a white or colored screen.

#16831. I suspect the upstream blocker on that one is solved but would double check.

@jslavitz
Copy link
Contributor

jslavitz commented Oct 25, 2018

@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.

@xster
Copy link
Member

xster commented Oct 25, 2018

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.

@jslavitz jslavitz self-assigned this Oct 25, 2018
@jslavitz
Copy link
Contributor

jslavitz commented Oct 27, 2018

@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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants