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

Alpha colors incompatable. #293

Closed
tomByrer opened this issue Oct 17, 2019 · 18 comments · Fixed by #300
Assignees
Labels
bug

Comments

@tomByrer
Copy link

@tomByrer tomByrer commented Oct 17, 2019

Versions (please complete the following information):

  • Win10
  • VS Code 1.38.1
  • Peacock 3.1.5

Describe the bug
When using CSS alpha colors, the auto-contrast for menu & icons does not work.

To Reproduce

"peacock.favoriteColors": [
        {
            "name": "Nord: Auroa Red",
            "value": "#bf616a4d"
        },
        {
            "name": "Nord: Auroa Orange",
            "value": "#d087704d"
        },
        {
            "name": "Nord: Auroa Yellow",
            "value": "#ebcb8b4d"
        },
        {
            "name": "Nord: Auroa Green",
            "value": "#a3be8c4d"
        },
        {
            "name": "Nord: Auroa Purple",
            "value": "#b48ead4d"
        },
etc

Expected behavior
The menu & left-icons to be legible

Screenshots
You can see the 'base' color in the window behind the dark-orange peacock-themed with alpha.
vscode-peacock-bug-alpha

Additional context
Kinda low priority. Quickest way is just post instructions how to set the menu foreground colors.
A good comprimise is to add optional fields to each theme color:

"peacock.favoriteColors": [
        {
            "name": "Nord: Auroa Red",
            "value": "#bf616a4d",
            "menu": "white",
            "icon": "lemon"
        },

Thanks for your great work.

@johnpapa

This comment has been minimized.

Copy link
Owner

@johnpapa johnpapa commented Oct 29, 2019

Peacock should be handling this.

@musicfuel can you look at this?

@musicfuel

This comment has been minimized.

Copy link
Contributor

@musicfuel musicfuel commented Oct 29, 2019

I can definitely take a look at this. I remember doing the contrast and readability work awhile back and I know my own mental model didn't account for opacity. I know I was leaning on the library at the time for many of the calculations, but it is very likely only taking the color channels into account.

@musicfuel

This comment has been minimized.

Copy link
Contributor

@musicfuel musicfuel commented Oct 29, 2019

Interesting. The latest issue on TinyColor relates directly to this bgrins/TinyColor#209

@johnpapa

This comment has been minimized.

Copy link
Owner

@johnpapa johnpapa commented Oct 29, 2019

Ouch. Ok. Well I guess we have our answer. Though I haven’t built and deployed peacock in weeks so I wonder how long this has existed.

@musicfuel

This comment has been minimized.

Copy link
Contributor

@musicfuel musicfuel commented Oct 29, 2019

This has probably existed since the earliest introduction of TinyColor and likely even in your original code without it.

I suspect the actual use case here is not especially common. Since the VSCode window itself cannot be opaque, any use of opacity in the background color would be a blend of the chosen color and the default window chrome. An opaque foreground color would blend into the background color, which if were opaque would then be a blend of the default window chrome, the background color, and the foreground. As a user, the resulting color is a bit difficult to predict in these cases.

I'm thinking we can probably handle this without too much trouble by looking for selections that contain less than full alpha and calculating the mixing before running readability tests. This should be doable as long as the default window chrome colors are accessible.

@johnpapa

This comment has been minimized.

Copy link
Owner

@johnpapa johnpapa commented Oct 30, 2019

Thanks @musicfuel - let me know when yo have a branch to review. Looks like tests will be easy enough for opacity too.

@tomByrer

This comment has been minimized.

Copy link
Author

@tomByrer tomByrer commented Oct 31, 2019

actual use case here is not especially common

I figured I was the one of the few weirdos who would try this. Looking at many VSCode theems, almost no one would use opacity except for highlights & selection indicators. I used it more liberally to reduce brightness on dark themes. I guessed correctly that peacock was an overlay color as well.

Thanks for looking into this!

One short-term workaround would be add more fields in settings.json. Though it seems I could manually hack the .vscode settings in the project directory?

@musicfuel

This comment has been minimized.

Copy link
Contributor

@musicfuel musicfuel commented Nov 1, 2019

@johnpapa TLDR version as proposals:

  1. Create function to effectively guess if the current theme is light or dark and then use hardcoded values for the workbench background. Hopefully replace these one day with actual VSCode API calls.
  2. Attempt to extend the VSCode API with a PR to add basic support for certain theme calls (they seem resistant to this, but it is being asked for)
  3. Consider removing alpha support for background colors from Peacock and only support color entry without alpha values.

So I've looked into this a bit and there is a problem (similar to other issues in the past) with the lack of theme support in the VSCode API. In this case we really don't need to know the current theme colors since the Peacock customizations replace those, but we really need to one of 2 things:

  1. Is the current theme a light or dark theme?
  2. What is the default workbench background color (which is based on light/dark)?

The reason for this is that the backgroundColor for any of our areas in Peacock will overlay the default workbench color (think of it like a <div> with a background color over another <div> with a different background color). If the backgroundColor in Peacock has an alpha component then we need to figure out the resulting color of that overlaying the workbench color. Once we have that, then we can adjust foreground readability issues accordingly.

The problem is that the VSCode API currently does not provide either of the above. We could basically make a guess of light or dark by comparing different current theme color background/foregrounds, though that isn't bulletproof. Even with that though, we don't know what the actual workbench color is programmatically. We could effectively hardcode it or assume black or white, but that isn't ideal.

I do know what the colors are and we could hardcode them. The can be found in the Theme class in VSCode and you can see them as returned values from WORKBENCH_BACKGROUND.

@johnpapa

This comment has been minimized.

Copy link
Owner

@johnpapa johnpapa commented Nov 1, 2019

Wondering if we should just remove alpha support fro peacock OR leave it as is and add some FAQ text referencing this issue and explaining it is a use at your own risk

@musicfuel

This comment has been minimized.

Copy link
Contributor

@musicfuel musicfuel commented Nov 1, 2019

That's what I am leaning toward since I don't believe this would be used very much to begin with.

@johnpapa

This comment has been minimized.

Copy link
Owner

@johnpapa johnpapa commented Nov 1, 2019

you feel comfortable creating that FAQ entry?

@musicfuel

This comment has been minimized.

Copy link
Contributor

@musicfuel musicfuel commented Nov 1, 2019

Sure, I can take a stab at it. I probably wouldn't be too difficult to also provide an information warning entering a color with alpha as part of a command, just to reinforce it.

@tomByrer

This comment has been minimized.

Copy link
Author

@tomByrer tomByrer commented Nov 2, 2019

The main solution I was hoping for would be manual foreground choice, which could benefit even non-alhpa users. Or is that already possible in settings.json?

@johnpapa

This comment has been minimized.

Copy link
Owner

@johnpapa johnpapa commented Nov 2, 2019

Have you tried dark and light foreground settings? Docs have info on these and other settings.

@johnpapa johnpapa closed this in #300 Nov 2, 2019
@musicfuel

This comment has been minimized.

Copy link
Contributor

@musicfuel musicfuel commented Nov 2, 2019

The light and dark foreground settings can be used to explicitly override the readability choices, though the detection of dark/light background could be off based on the transparency of the color as well.

@tomByrer Peacock works by writing to the workbench.colorCustomizations key in the local .vscode/settings.json file to modify the colors of different elements. After using Peacock to establish the colors, you can easily manually adjust any of them in that file to your liking. In your case, you are probably most concerned with any of the foreground keys and/or activityBarBadge.background.

@tomByrer

This comment has been minimized.

Copy link
Author

@tomByrer tomByrer commented Nov 13, 2019

you can easily manually adjust any of them in that file to your liking

Doesn't work, Peacock overwrites my settings.

Reopen please? (even if just to warn others)

@johnpapa

This comment has been minimized.

Copy link
Owner

@johnpapa johnpapa commented Nov 13, 2019

The most recent comments show that given constraints we decided to disable alpha usage. We had no good way to verify the accessibility and contrast given an alpha. I am very open to any new ideas and pull requests that could enable this.

@musicfuel

This comment has been minimized.

Copy link
Contributor

@musicfuel musicfuel commented Nov 14, 2019

I think the overwrite mentioned above is in reference to #306 and not the issue here. I agree with @johnpapa that this should remain closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.