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

Colors with border? #17

Open
FedericoPonzi opened this issue Apr 27, 2016 · 9 comments
Open

Colors with border? #17

FedericoPonzi opened this issue Apr 27, 2016 · 9 comments

Comments

@FedericoPonzi
Copy link

Hi,
First of all thanks for the great library.
I've just added spectrum for an app I'm doing on my spare time (you can find the source on my github), and works fine but some colors appear with borders:
photo_2016-04-27_23-28-55
Also, is there a way to keep a state about the last selected color inside my dialog?
Thanks a lot!

@yussefbens
Copy link

yussefbens commented Apr 27, 2016

I have the same problem as you.

@nwalters512
Copy link
Member

An outline will appear if you use SpectrumPalette#setOutlineWidth() or use spectrum_outlineWidth in your XML declaration. If you didn't do that and the border still appears, then it looks like we have a bug!

Spectrum won't store any state for you; you'll have to use OnColorSelectedListener to listen for when the user selects the color, and then you can save the color from there.

@FedericoPonzi
Copy link
Author

FedericoPonzi commented Apr 28, 2016

That was a fast reply! Thanks!
I do have an outlineWidth of 2dp, but since it's not rendered on every color isn'it a bug? I'll try to remove this property and check the result as soon as possibile.

About the other question, I knwo how to save the color select by the user so i Can save it myself. But is there a Way to pre-select the last color the user selected?
Something like a "default" option maybe?
Thanks!
Edit: I guess setSelectedColor() should work to me. I'll let you know about the outline, thanks again!

@yussefbens
Copy link

yussefbens commented Apr 28, 2016

Thank you @nwalters512, I have solved the problem by deleting spectrum_outlineWidth from my XML file.

@nwalters512
Copy link
Member

@FedericoPonzi the outlining feature is sort of half-baked at this point. The other colors actually do have an outline; however, it's white, so it blends in with the background and makes it appear like there isn't one. See https://github.com/the-blue-alliance/spectrum/blob/master/spectrum/src/main/java/com/thebluealliance/spectrum/internal/ColorItem.java#L166

If you'd like to try to improve that behavior, you're welcome to submit a PR!

@FedericoPonzi
Copy link
Author

Removing the outline resolved the issue, thanks!
Can this be a more suitable way to display it?
screenshot_20160428-220250
BTW I think the best way should be to use a gradient from white to black background, mOutlineWidth bigger thank the ForegroundDrawable, but I haven't figured how to do it yet.
What do you think?

@nwalters512
Copy link
Member

Can this be a more suitable way to display it?

This makes the border visible on all items, even if it was previously "hidden" for many items if the background was white.

I had discussed some potential improvements to outlining here. I only really want to display the outline when it's absolutely necessary to differentiate the color from the background; I think it looks much cleaner this way. I also want to design the next iteration of this feature so that it's compatible with a dark background, in case I ever introduce theming for the various Spectrum dialogs.

BTW I think the best way should be to use a gradient from white to black background, mOutlineWidth bigger thank the ForegroundDrawable, but I haven't figured how to do it yet.

I'm not quite sure what you mean by that - could you make an illustration to demonstrate what you mean?

@FedericoPonzi
Copy link
Author

Sorry for my bad English and my graphic skills (Paint FTW):
immagine
immagine
immagine

A gradient from black to white as a background of the ColorItem. In that way we would avoid both the need to set a different outline color for different color variants and the dotted alternative that I've just proposed.

About the improvements discussed in the PR you linked, I think that the library needs an outline in most cases (maybe the user has light backgrounds and is going to display dark colors), but I really don't like the fact that some colors will end up having a border and some won't.

P.s. I got the effect of dot on the last screenshot using this as stroke:

mask.setStroke(mOutlineWidth, Color.GRAY, 50, 1.05f);

@nwalters512
Copy link
Member

I don't think the solution should be to apply a very obvious outline to every single color just because a (generally small) subset of colors will require them. I think in most cases, it definitely looks the most clean to just leave colors borderless (even though most colors technically have a white border now, they're invisible against a white background and don't exist for all intents and purposes).

That being said, there could be a real need for customization here. We should be shooting for sensible defaults while still letting developers customize as much as possible.

One possible solution would be adding the ability to supply a OutlinePaintProvider to the various Spectrum components. I envision it looking like this:

public interface OutlinePaintProvider {
    void setupPaint(Paint paint, @ColorInt int color);
}

In this case, you would pass an implementation of OutlinePaintProvider to the palette/dialog/preference/etc., which would then call setupPaint(...) for each color item. That method would be responsible for configuring the Paint object to draw an outline for the specified color.

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

No branches or pull requests

3 participants