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

fix: update plugin card color on dark theme #1600

Merged

Conversation

nouralmulhem
Copy link
Contributor

@nouralmulhem nouralmulhem commented Feb 29, 2024

related issue: #1577

Before:

image

After:

image

@zbynek
Copy link
Contributor

zbynek commented Feb 29, 2024

Thanks! Definitely looks better in dark mode.

This also changes the color in light mode, causing the "icons" (.Plugin--IconContainer i) to have too low contrast now. Since the background for those is the same in light/dark mode, they should also use the same foreground in both.

@nouralmulhem
Copy link
Contributor Author

nouralmulhem commented Mar 1, 2024

thank you for noticing this!! , I think this has nothing to do with the fact that the text color has to change, wanted also to refer to which was presented previously

image

as you said i recommend making icon text foreground color unified (not the whole text), and this should be the solution, as the background color doesn't change

i will update my branch with the proposed solution and maybe you can share your thoughts again with me

Copy link
Contributor

@zbynek zbynek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

Copy link
Member

@NotMyFault NotMyFault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@NotMyFault NotMyFault merged commit 3d3eb2c into jenkins-infra:master Mar 1, 2024
7 checks passed
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

Successfully merging this pull request may close these issues.

None yet

3 participants