Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| > | ||
| {items.map((item) => ( | ||
| <ion-picker-column-option | ||
| color={this.color} |
There was a problem hiding this comment.
The month/year picker was displaying selected values in the datetime's color (for example, red for color="danger"), but the time picker was ignoring this and showing selected values in primary color instead, creating an inconsistent experience on md. This was wrong because the datetime's color prop should control all picker text colors uniformly.
| }); | ||
| }); | ||
| }); | ||
|
|
brandyscarney
left a comment
There was a problem hiding this comment.
Looks great! Left some minor feedback.
There was a problem hiding this comment.
Are there any breaking changes associated with this, like CSS that may start overriding where it previously wasn't?
There was a problem hiding this comment.
I can't think of any. Devs shouldn't be able to tell the difference with these changes even if they are customizing the primary variables themselves.
| background: ion-color(primary, base, 0.2); | ||
|
|
||
| box-shadow: 0px 0px 0px 4px ion-color(primary, base, 0.2); | ||
| } | ||
|
|
||
| :host(.ion-color) .calendar-day:not(.calendar-day-adjacent-day):focus { | ||
| background: #{current-color(base, 0.2)}; | ||
|
|
||
| box-shadow: 0px 0px 0px 4px current-color(base, 0.2); | ||
| box-shadow: 0px 0px 0px 4px #{current-color(base, 0.2)}; |
There was a problem hiding this comment.
| background: ion-color(primary, base, 0.2); | |
| box-shadow: 0px 0px 0px 4px ion-color(primary, base, 0.2); | |
| } | |
| :host(.ion-color) .calendar-day:not(.calendar-day-adjacent-day):focus { | |
| background: #{current-color(base, 0.2)}; | |
| box-shadow: 0px 0px 0px 4px current-color(base, 0.2); | |
| box-shadow: 0px 0px 0px 4px #{current-color(base, 0.2)}; | |
| background: ion-color(primary, base, 0.2); | |
| box-shadow: 0px 0px 0px 4px ion-color(primary, base, 0.2); | |
| } | |
| :host(.ion-color) .calendar-day:not(.calendar-day-adjacent-day):focus { | |
| background: current-color(base, 0.2); | |
| box-shadow: 0px 0px 0px 4px current-color(base, 0.2); |
Nit: you don't need to escape unless it's in a CSS variable
There was a problem hiding this comment.
I updated the PR description to reflect it better. Let me know what you think. I can also fix it another PR if you prefer.
Co-authored-by: Brandy Carney <6577830+brandyscarney@users.noreply.github.com>
ShaneK
left a comment
There was a problem hiding this comment.
Looks good to me, awesome work! Had a nit that's just a follow up on Brandy's earlier comment, but just a nit
| } | ||
|
|
||
| :host(.ion-color) .calendar-day:not(.calendar-day-adjacent-day):focus { | ||
| background: #{current-color(base, 0.2)}; |
There was a problem hiding this comment.
| background: #{current-color(base, 0.2)}; | |
| background: current-color(base, 0.2); |
Nit: looks like this one got missed when you removed the escapes. The box-shadow right below it and the default rule above both drop the #{}, so this can too. Up to you!


Issue number: resolves internal
What is the current behavior?
ion-datetimeand related components (ion-datetime-button,ion-picker-column,ion-picker-column-option) have a defaultcolor="primary"property. This forces users who want to customize datetime styling to override the entire primary color system or set explicit colors on every instance, rather than being able to style the component normally.What is the new behavior?
color="primary"property assignments from all four componentsion-color(primary, base)) instead of relying on a property default:host(.ion-color)overrides in SCSS to apply explicit colors viacurrent-color()when a color prop is setmainFW-6996Does this introduce a breaking change?
Backwards compatible: Visual appearance is unchanged. Components still appear with primary color by default. Existing code that explicitly sets
colorprops continues to work identically.Other information
Dev build:
8.8.14-dev.11783706459.1d5a7aaePreviews: