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.

Sign up for GitHub

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

Jump to bottom

FW-6214 standardize colors #657

Merged
merged 28 commits into from
Oct 31, 2024
Merged

FW-6214 standardize colors #657

merged 28 commits into from
Oct 31, 2024

Conversation

gmcauliffe
Copy link
Contributor

@gmcauliffe gmcauliffe commented Oct 29, 2024 •
edited
Loading

Description of Changes

  • reduces the number of colours we are using to 5 (plus tints of each of those colours) + 3 additional colours
  • Drops use of the names primary, secondary, fv-charcoal and the tertiaries in favour of colour names: jade, scarlet, blumine, ochre, and charcoal
  • Drops use of -light -dark and default in favour of Tailwind style numerical values e.g. scarlet-400
  • removes default Tailwind red, gray, green, and yellow
  • adds a customColor module for reusing colours under different Tailwind classNames/aliases
  • Introduces some colour aliases that we can use when a colour is being used for a specific purpose, will allow for easier reassigning of colour

Checklist

  • README / documentation has been updated if needed
  • PR title / commit messages contain Jira ticket number if applicable
  • Tests have been updated / created if applicable

Reviewers Should Do The Following:

  • Review the changes here
  • Pull the branch and test locally

Additional Notes

N/A

@gmcauliffe
Add new kids color variables
e3b232a
@gmcauliffe
Remove tertiaryD
be01250
@gmcauliffe
Replace bgRed with scarlet-800
96d3143
@gmcauliffe
Replace bgGreen with jade-500
a44889a
@gmcauliffe
Move new colors into separate module for reuse
29ad32b
@gmcauliffe
Remove dynamically generated light|dark|{type}Text color classes
e25cb50
@gmcauliffe
Replace any uses of {type}dark or light
c942a1d
@gmcauliffe
Replace all uses of {type}Text
93e1112
@gmcauliffe
Replace TertiaryB with shades of charcoal
ec38c1d
@gmcauliffe
Replace fv-charcoal with tints of charcoal
fe14973
@gmcauliffe
Switch uses of old type colors to new color variables and colors
a00ba1c
@gmcauliffe
Update SectionTitle props and remove when match default
df36dab
@gmcauliffe
Update TabsPresentation and associated props
a059202
@gmcauliffe
Update RadioButtonGroup and associated props
26f8299
@gmcauliffe
Update Toggle and associated props
541e2b1
@gmcauliffe
Set Wordsy defaults and update safelist
18c137c
@gmcauliffe
Replace TertiaryC with ochre-500
e3b8b7b
@gmcauliffe
Remove TertiaryA and replace with various
ba448e5
@gmcauliffe
Merge branch 'dev' into FW-6214-standardize-colors
ed3f4bd
@gmcauliffe
Replace secondary with tints of scarlet
093e8fc
@gmcauliffe
Replace primary with tints of blumine
d903ebc
@gmcauliffe
Merge branch 'dev' into FW-6214-standardize-colors
784e501
@gmcauliffe gmcauliffe marked this pull request as ready for review October 29, 2024 16:51
sahil97
sahil97 reviewed Oct 30, 2024
View reviewed changes
src/components/Alphabet/AlphabetPlaceholder.js
<div className="col-span-7 md:col-span-4">
<div className="grid grid-cols-7 md:grid-cols-5 lg:grid-cols-7">
<span className="bg-gray-100 text-gray-100 col-span-1 font-medium inline-flex justify-center m-1 p-5 rounded text-2xl">
<span className="bg-charcoal-50 text-charcoal-50 col-span-1 font-medium inline-flex justify-center m-1 p-5 rounded text-2xl">
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: we can make this style reusable similar to btn-icon for this file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reduction in duplication is definitely warranted here, but I think a local variable would be the way to go. Global classes, like btn-icon, are more for class combinations that are reused across many files and for styling that we would potentially want to change at a global level.

sahil97
sahil97 approved these changes Oct 30, 2024
View reviewed changes
Copy link
Contributor

@sahil97 sahil97 left a comment

Choose a reason for hiding this comment

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

Thank you for the huge refactor. LGTM !

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
4.3% Duplication on New Code

See analysis details on SonarCloud

@gmcauliffe gmcauliffe merged commit 6247d51 into dev Oct 31, 2024
8 checks passed
@gmcauliffe gmcauliffe deleted the FW-6214-standardize-colors branch October 31, 2024 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sahil97 sahil97 sahil97 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gmcauliffe @sahil97