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

[WDTK] Popup black version #1923

Merged
merged 3 commits into from
Oct 8, 2024
Merged

[WDTK] Popup black version #1923

merged 3 commits into from
Oct 8, 2024

Conversation

lucascumsille
Copy link
Contributor

Relevant issue(s)

Helen discovered

What does this do?

Improved contrast for the popup element so it passes contrast text

Why was this needed?

Implementation notes

Screenshots

Black version
Screenshot 2024-09-23 at 07 44 07

Green version
I get why originally we are using the mySociety green. but I think the problem with that it rarely looks nice inmediately right next to the other colours, they tend to clash with each other. Assuming using green is important I added a gradient that makes it readable, but probably my least favourite option. I tried a darker version so we can continue using white font text, but it didn't look very nice next to the navbar.
Screenshot 2024-09-23 at 07 59 32

WDTK Variation
I created a variation of WDTK primary colour, using the same HUE, unlike the green version I think this one looks more WDTK style.
Screenshot 2024-09-23 at 08 14 20

Let me know if you have any preferences.

Notes to reviewer

@HelenWDTK
Copy link
Contributor

HelenWDTK commented Sep 23, 2024

I love the black and WDTK variations - I don't think there is any particular reason why it needs to be green.

@lucascumsille
Copy link
Contributor Author

Thanks you @HelenWDTK happy to go with that. The first one(Black version) is a bit subtle, while the WDTK versions pop up more.

@gbp gbp merged commit 026fcde into master Oct 8, 2024
2 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.

3 participants