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

added light background for better hover visibility on dark theme #8043

Merged
merged 3 commits into from
Oct 29, 2024

Conversation

abdullah5361k
Copy link
Contributor

fixes: #7969

What does this PR do?

Apply a lighter background color to enhance the hover effect in dark mode.

twenty-hover

@abdullah5361k
Copy link
Contributor Author

@Bonapara Could you please review this pull request? I'm unsure why the last one didn't work as expected.

@Bonapara
Copy link
Member

Bonapara commented Oct 25, 2024

Thanks @abdullah5361k, can you also add back the hover on the view switcher dropdown?

CleanShot.2024-10-25.at.09.37.30.mp4

@abdullah5361k
Copy link
Contributor Author

abdullah5361k commented Oct 25, 2024

Thanks @abdullah5361k, can you also add back the hover on the view switcher dropdown?

@Bonapara Sure, I’ll. Apologies for the delayed response—I'm currently doing an internship with a daily 70-kilometer commute each way, which sometimes delays my replies.

@abdullah5361k
Copy link
Contributor Author

@Bonapara, could you please take a look at my pull request and provide feedback?

@Bonapara Bonapara enabled auto-merge (squash) October 28, 2024 10:13
@Bonapara
Copy link
Member

Hi @abdullah5361k, thanks, looks good to me!

@Bonapara
Copy link
Member

Thank you for contributing, and good luck with your internship! What are you doing?

@Devessier Devessier self-assigned this Oct 28, 2024
@abdullah5361k
Copy link
Contributor Author

Thank you for contributing, and good luck with your internship! What are you doing?

Thank you! I’m currently involved in projects focused on web development.

@Devessier
Copy link
Contributor

Devessier commented Oct 29, 2024

Hi @abdullah5361k, I reviewed your PR. Your fix worked well! The components you modified weren't well structured; we shouldn't rely on a css filter to apply different colors to the background and the text on hover. It's much more predictable to set explicit colors. That's the refactor I made. Thank you a lot for your time!

@Bonapara Bonapara merged commit 77a4aa2 into twentyhq:main Oct 29, 2024
18 of 20 checks passed
Copy link

oss-gg bot commented Oct 29, 2024

Awarding abdullah5361k: 150 points 🕹️ Well done! Check out your new contribution on oss.gg/abdullah5361k

@abdullah5361k
Copy link
Contributor Author

Hi @abdullah5361k, I reviewed your PR. Your fix worked well! The components you modified weren't well structured; we should rely on a css filter to apply different colors to the background and the text on hover. It's much more predictable to set explicit colors. That's the refactor I made. Thank you a lot for your time!

Thank you, @Devessier , for the feedback.

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.

Light icon button hover is not visible in Dark mode
3 participants