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

Update tailwind.config.js #378

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Update tailwind.config.js #378

wants to merge 4 commits into from

Conversation

annedino4
Copy link
Contributor

Added new colours and changed some old colours with the new colourful tree design in mind. See below file for reference:
https://www.figma.com/file/w8RpeMkOo63As4k50y4ezt/Primer-Design-Library-22?node-id=0%3A1

@@ -12,6 +12,7 @@ module.exports = {
blue: {
primary: "#34375d",
secondary: "#4b5097",
quaternary: "64B0C8",
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be called tertiary.

@dhess
Copy link
Member

dhess commented Jun 6, 2022

@annedino4 This PR appears to be based on an old PR of George's, so I'm going to rebase it.

@dhess
Copy link
Member

dhess commented Jun 6, 2022

We're unable to use Chromatic right now due to breakage, which is unfortunate as this PR is exactly the sort of thing it's good at, but oh well. I've made screenshots instead, which I'll share here.

I think secondary-hover is too close to primary now:

Screen Shot 2022-06-06 at 11 04 36 AM

@dhess
Copy link
Member

dhess commented Jun 6, 2022

On a related note, the secondary red color is now more red than "salmon." I think we chose the more pink-ish red for warnings & holes because it looks less like an error, so what's the thinking in moving towards red?

@annedino4
Copy link
Contributor Author

On a related note, the secondary red color is now more red than "salmon." I think we chose the more pink-ish red for warnings & holes because it looks less like an error, so what's the thinking in moving towards red?

Screenshot 2022-06-06 at 11 33 02 am

We use that colour for tiny text (undo) which is not suitable contract-wise. It is safer and more versatile using a darker colour for warning. How about below?

Screenshot 2022-06-06 at 11 43 02 am

@dhess
Copy link
Member

dhess commented Jun 6, 2022

It's a good point about the contrast of the "undo" text, but personally I really dislike those brown-ish colors for warnings, especially as they're going to be fairly commonly used.

What if, instead, we kept the current warning color since it looks nice as a button/hole color, and instead make the Undo and Redo buttons on the toolbar solid colors like our other buttons? Then we can use white text against the background and everything is fine. It's also more consistent with our other buttons, including the tree/text button that's also on the toolbar.

@dhess
Copy link
Member

dhess commented Jun 6, 2022

Alternatively, there's no reason we can't use red-primary for the Undo button (including the text) so that it has better contrast.

@annedino4
Copy link
Contributor Author

Alternatively, there's no reason we can't use red-primary for the Undo button (including the text) so that it has better contrast.

Yes I agree, I'll update the document.

I have also explored using dark text on the salmon colour, as small white text on the salmon background failed the contrast check as well.
Screenshot 2022-06-06 at 12 09 41 pm

We could make sure going forward, we (1) Don't use the salmon colour for text and (2) Always use dark text against the salmon coloured background?

@dhess
Copy link
Member

dhess commented Jun 6, 2022

Hmm, that is unfortunate that the salmon color with white text doesn't pass the check, because I think it would be out of place if it were the only button color without white text. Can we explore some more orange-ish options that pass the test with white text?

@annedino4
Copy link
Contributor Author

annedino4 commented Jun 6, 2022

Hmm, that is unfortunate that the salmon color with white text doesn't pass the check, because I think it would be out of place if it were the only button color without white text. Can we explore some more orange-ish options that pass the test with white text?

Sorry, the background colour needs to be "dark enough" to display small white text to pass the check, see below for avalible colours:
Screenshot 2022-06-06 at 2 34 29 pm

Alternatively, we could follow the style of a standard action button, rendering the bloder the warning colour
Screenshot 2022-06-06 at 2 29 26 pm

@dhess
Copy link
Member

dhess commented Jun 6, 2022

I think I prefer the 2nd row (#CD3864 and #7E213B). You?

@annedino4
Copy link
Contributor Author

I think I prefer the 2nd row (#CD3864 and #7E213B). You?

Yes happy to go with that! I will update the colours accordingly.
Now since the warning colour create no issues displaying small text, components effected will remain using the warning colour :
Screenshot 2022-06-06 at 3 17 33 pm

Added new colours and changed some old colours with the new colourful tree design in mind. See below file for reference:
https://www.figma.com/file/w8RpeMkOo63As4k50y4ezt/Primer-Design-Library-22?node-id=0%3A1
Updated blue tertiary & quaternary.
Updated blue tertiary, quaternary and red secondary-hover,
As discussed, changed the red colour pallet.
@dhess
Copy link
Member

dhess commented Jun 27, 2022

I will pick this up and take it over the line. In light of our decision to create a separate high-contrast theme, some of the changes here may not be necessary.

@dhess dhess self-assigned this Jun 27, 2022
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.

2 participants