feat(themes): remove deprecated color steps and update bg and text steps#31035
feat(themes): remove deprecated color steps and update bg and text steps#31035thetaPC wants to merge 3 commits intoionic-modularfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
bot
added
package: core
thetaPC
changed the title
thetaPC
changed the title
thetaPC
requested review from
BenOsodrac and
brandyscarney
and removed request for
BenOsodrac
| --ion-background-color-step-850: #d9d9d9; | ||
| --ion-background-color-step-900: #e6e6e6; | ||
| --ion-background-color-step-950: #f2f2f2; | ||
|
|
There was a problem hiding this comment.
Don't we need to add the --ion-text-color-step vars here too?
| --ion-background-color-step-850: #d9d9d9; | ||
| --ion-background-color-step-900: #e6e6e6; | ||
| --ion-background-color-step-950: #f2f2f2; | ||
|
|
There was a problem hiding this comment.
Don't we need to add the --ion-text-color-step vars here too?
| --ion-background-color-step-900: #e8eaeb; | ||
| --ion-background-color-step-950: #f4f4f5; | ||
| --ion-background-color-step-1000: #fff; | ||
| } |
There was a problem hiding this comment.
Don't we need to add the --ion-text-color-step vars here too?
| --ion-background-color-step-850: #dbdbdb; | ||
| --ion-background-color-step-900: #e7e7e7; | ||
| --ion-background-color-step-950: #f3f3f3; | ||
|
|
There was a problem hiding this comment.
Don't we need to add the --ion-text-color-step vars here too?
| --ion-background-color-step-850: #d9d9d9; | ||
| --ion-background-color-step-900: #e6e6e6; | ||
| --ion-background-color-step-950: #f2f2f2; | ||
|
|
There was a problem hiding this comment.
Don't we need to add the --ion-text-color-step vars here too?
| --ion-color-step-50, | ||
| var(--ion-text-color-step-950, mix($background-color-value, $text-color-value, 95%)) | ||
| ); | ||
| $background-color-step-50: var(--ion-background-color-step-50, mix($text-color-value, $background-color-value, 5%)); |
There was a problem hiding this comment.
We should consider moving these to the default tokens file instead and just having the Sass files use the tokens directly instead of Sass variables. Doesn't have to be done on this PR though.
| backgroundColor: '#ffffff', | ||
| textColor: '#000000', | ||
|
|
||
| backgroundColorStep: { |
There was a problem hiding this comment.
Why was this removed entirely?
| backgroundColorStep: generateColorSteps('#ffffff', '#121212'), | ||
| textColorStep: generateColorSteps('#888888', '#ffffff'), |
There was a problem hiding this comment.
These values are not right. The values are flipped for the dark palette.
| export const highContrastTheme: HighContrastTheme = { | ||
| ...baseHighContrastTheme, | ||
|
|
||
| backgroundColor: '#ffffff', |
There was a problem hiding this comment.
Why was this removed entirely?
There was a problem hiding this comment.
Like my earlier comment - we should consider making these always available for every theme and palette.
2 participants
Issue number: internal
What is the current behavior?
Code still references the deprecated color steps.
What is the new behavior?
Does this introduce a breaking change?
--ion-color-step-[number], have been removed and should be replaced with either--ion-background-color-step-[number]or--ion-text-color-step-[number]depending on the specific use case, as outlined in the migration guide.Other information
Preview