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

style(cosmo): harmonise value box colour (opacity) #10029

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mcanouil
Copy link
Collaborator

@mcanouil mcanouil commented Jun 15, 2024

This pull request harmonises the value box opacity in the Cosmo style. The valuebox-bg-info and valuebox-bg-danger colours have been adjusted to have consistent opacity, i.e., no opacity as the other value box background.

Few typos fixed in the changelog.

@mcanouil mcanouil self-assigned this Jun 15, 2024
@cderv
Copy link
Collaborator

cderv commented Jun 17, 2024

@dragonstyle if you happen pass around ? Any reason to have set transparency ?

My feeling tells me it may be on purpose for default styling choice.

@cscheid
Copy link
Collaborator

cscheid commented Jun 17, 2024

  1. This changes more than just cosmo, right?
  2. CSS changes are pretty hard to test for regression, and so I don't want to take them as late as we are into 1.5.

@cscheid cscheid added this to the v1.6 milestone Jun 17, 2024
@mcanouil
Copy link
Collaborator Author

This PR was not necessarily for 1.5.

The changes should only affect value box for the cosmo theme.

If not then that would mean there is something very wrong with the structure and the naming inside this SCSS file.

@cscheid
Copy link
Collaborator

cscheid commented Jun 17, 2024

Sorry: I saw this and missed the fact that it was scoped for cosmo

image

So this looks good. Let's just wait for 1.6 first.

@mcanouil
Copy link
Collaborator Author

mcanouil commented Jun 17, 2024

A bit of context: apparently the purpose of the 70% opacity was to reduce brightness for info/danger.

An alternative, could be to use darken instead.
For instance, using 3% darkening on all colours:

image

(Setting as draft for now)

@mcanouil mcanouil marked this pull request as draft June 17, 2024 20:34
@mcanouil mcanouil added needs-discussion Issues that require a team-wide discussion before proceeding further themes Related to HTML theming or any other style related issue (like highlight-style) html Issues with HTML and related web technology (html/css/scss) labels Jun 17, 2024
@jjallaire
Copy link
Collaborator

Most instructive will be to look at these variations in an actual dashboard. What Charles and I found was that more saturated colors tended to take up too much "weight" visually visa-vi the rest of the dashboard, making them feel visually top-heavy. I don't doubt we could come up with improvements, but you need to look at the variations in context to really know.

@mcanouil mcanouil changed the title style(cosmo): harmonise value box opacity style(cosmo): harmonise value box colour (opacity) Jun 17, 2024
@mcanouil
Copy link
Collaborator Author

mcanouil commented Jun 18, 2024

A real life example of a dashboard with/without opacity:

Screen.Recording.2024-06-18.at.18.11.09.mov

(I don't have yet "darken" examples)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
html Issues with HTML and related web technology (html/css/scss) needs-discussion Issues that require a team-wide discussion before proceeding further themes Related to HTML theming or any other style related issue (like highlight-style)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants