refactor(components): Add semantic colors to typography on mobile [97200] #2033
Conversation
Aiden-Brine
force-pushed
the
JOB-97200/typography-semantic-colours-mobile
branch
from
eb4fd95
to
92d378a
Compare
Deploying atlantis with
|
| Latest commit: |
18920a7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5182a717.atlantis.pages.dev |
| Branch Preview URL: | https://job-97200-typography-semanti-lp2y.atlantis.pages.dev |
Aiden-Brine
force-pushed
the
JOB-97200/typography-semantic-colours-mobile
branch
4 times, most recently
from
84d9132
to
93b7a3c
Compare
Aiden-Brine
force-pushed
the
JOB-97200/typography-semantic-colours-mobile
branch
from
93b7a3c
to
b043da6
Compare
| color: tokens["color-brand"], | ||
| }, | ||
|
|
||
| // Deprecated |
There was a problem hiding this comment.
I did check all of these deprecated colors and unfortunately all of them are being used. I wrote up this issue to remove them
| color: tokens["color-heading"], | ||
| }, | ||
|
|
||
| base: { |
There was a problem hiding this comment.
I could see the argument that we don't want to deprecate base and baseReverse even though they are duplicates. I think we should strive to have no duplicates to cut down on confusion and make things clearer when comparing code but if folks think removing base is a step too far I can remove it from this section
| color: tokens["color-critical"], | ||
| }, | ||
|
|
||
| base: { | ||
| color: tokens["color-text"], | ||
| criticalSurace: { |
There was a problem hiding this comment.
| criticalSurace: { | |
| criticalSurface: { |
| color: tokens["color-informative--onSurface"], | ||
| }, | ||
|
|
||
| critical: { | ||
| // To be uncommented once success in Deprecated is removed | ||
| // success: { |
There was a problem hiding this comment.
I think we should just roll this particular change out and get rid of the one in the deprecated section. It's a fairly low-risk change and probably not worth the complexity of maintaining a present and future version side-by-side? (I know we'll need to with some of the other colors, but making the change to success now means one less duplicate at least)
There was a problem hiding this comment.
The reason why I left this out is that currently success is "color-success--onSurface" so in order to set success to "color-success" as it should be I will need to go through the mobile code and set all uses of success to successOnSurface so there is no color change and then remove the old success.
There was a problem hiding this comment.
Since successOnSurface isn't available yet this will need to be done in 3 steps
Motivations
All of the semantic colors should be available for Typography on mobile
Changes
Adds missing semantic colors to Typography.style.ts. Also reorganizes the colors to make it easier to find the color you need in the future and makes deprecated colors clear.
Added
Missing semantic colors for mobile typography
Changed
Re-organizes the colors
Deprecated
Groups various colors to be deprecated in the near future
Removed
No colors were removed
Fixed
N/A
Security
N/A
Testing
Open up storybook via the pre-release branch and locally tp play around with new colors/existing colors to ensure they all work
Changes can be
tested via Pre-release
In Atlantis we use Github's built in pull request reviews.