Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| :host ::slotted(ion-badge[vertical]:not(:empty)) { | ||
| @include globals.padding(2px); | ||
| } |
There was a problem hiding this comment.
Removed this completely because based on MD3, the padding should always be consistent regardless of the host component.
| }, | ||
|
|
||
| // Sizes | ||
| size: { |
There was a problem hiding this comment.
| }, | ||
|
|
||
| // Sizes | ||
| size: { |
There was a problem hiding this comment.
Avatar sizes have yet to be implemented.
There was a problem hiding this comment.
Did you mean to leave this comment here? The ionic theme does have sizes: https://ionic-framework-git-fw-6837-ionic1.vercel.app/src/components/avatar/test/badge?ionic:theme=ionic
There was a problem hiding this comment.
The default is now small so the snapshot was updated.
There was a problem hiding this comment.
What led to this decision?
There was a problem hiding this comment.
I renamed the folder from hint to vertical.
There was a problem hiding this comment.
Moved this test to badge-size-rectangular since there was no need to split them into smaller tests. The same reason goes for the other shape snapshots that were "deleted".
There was a problem hiding this comment.
Moved this test to badge-size-large since there was no need to split them into smaller tests. The same reason goes for the other size snapshots that were "deleted".
| /** | ||
| * "-apple-system-body" on iOS never goes smaller than | ||
| * 14px according to https://developer.apple.com/design/human-interface-guidelines/typography#Specifications. | ||
| * However, we still keep the max() usage here for consistency | ||
| * with other components and in case "-apple-system-body" does | ||
| * go smaller than 14px in the future. | ||
| */ |
There was a problem hiding this comment.
This comment lived in the ios theme file, I moved it over to keep record of it.
There was a problem hiding this comment.
Not sure why this keeps being persistent. I tried reverting it so it wouldn't bog down the PR but it's still here. I was able to revert the other browsers but not Firefox.
thetaPC
requested review from
brandyscarney and
gnbm
and removed request for
gnbm
There was a problem hiding this comment.
Did you mean to leave this comment here? The ionic theme does have sizes: https://ionic-framework-git-fw-6837-ionic1.vercel.app/src/components/avatar/test/badge?ionic:theme=ionic
There was a problem hiding this comment.
The avatar itself is squished when badge position is none for the ionic theme. This happens for all sizes on this test: Avatar - Badge. Is this expected behavior for now or should it be fixed?
There was a problem hiding this comment.
What led to this decision?
There was a problem hiding this comment.
A few things on this test:
- Should the badges always be on top of the buttons? In some examples it is behind the next button:
-
There are so many examples here that it's hard to see what is going on. Can we split this up somehow? By size, position or type of button (text vs icon)?
-
Do we need to capture the different fill types?
There was a problem hiding this comment.
This one is a lot easier to parse compared to the ionic and md themes, maybe we should force it to be a grid of 2 or 4 columns. There is still a lot going on in this test though.
| badge.style.right = `${inlineStart}px`; | ||
| badge.style.left = ''; |
There was a problem hiding this comment.
Could we use inset-inline-start and inset-inline-end instead of checking for rtl? It looks like our current browser support works with it: https://ionicframework.com/docs/reference/browser-support#browsers
| interface CurrentColorOptions { | ||
| alpha?: number | string | null; | ||
| subtle?: boolean; | ||
| } |
There was a problem hiding this comment.
Do we need this to be different from IonColorOptions? I went looking and I don't see anywhere we call rgb by itself. All the usage I could find shows it just passing alpha in.
| @@ -26,6 +26,12 @@ export const defaultTheme: DefaultTheme = { | |||
| rippleEffect: true, | |||
|
|
|||
| components: { | |||
| IonBadge: { | |||
| hue: 'bold', | |||
| shape: 'soft', | |||
There was a problem hiding this comment.
Should it be round to match how badges on tabs look in md2?
| @@ -23,6 +23,12 @@ export const defaultTheme: DefaultTheme = { | |||
|
|
|||
| config: { | |||
| components: { | |||
| IonBadge: { | |||
| hue: 'bold', | |||
| shape: 'soft', | |||
There was a problem hiding this comment.
Why is this soft when Swift examples show round badges:
| * Defaults to `"soft"` if both the shape property and theme config are unset. | ||
| */ | ||
| @Prop() shape?: 'soft' | 'round | rectangular'; | ||
| @Prop() shape?: IonBadgeShape; |
There was a problem hiding this comment.
I'm not sure if I love the name crisp but I don't know of anything that would be a better fit. Maybe we can ask the team for ideas?
2 participants
Issue number: internal
What is the current behavior?
Component styles for
ion-badgeare fragmented across multiple files (ios, md). Developers were restricted to those themes and how those themes structured the logic and styles.What is the new behavior?
badge.scssfile that consumes CSS variables, ensuring a single source of truth for component logic.badge.interfaces.tsDoes this introduce a breaking change?
This PR introduces a breaking change to how
IonBadgeis styled. Existing manual CSS overrides targeting internal badge structures or old token names will no longer work due to the shift to thew new token hierarchy and a unified base SCSS file.Migration Path:
Token Updates: Update any custom theme configurations to match the new nested structure.
CSS Overrides: Ensure selectors align with the new slotted element logic and variable names (e.g.,
--ion-badge-hue-bold-default-background).--backgroundshould no longer be used. Setting the value,Badge.hue.bold.background, within the tokens file should be used to change the background for the bold hue. Setting the value,Badge.hue.subtle.background, within the tokens file should be used to change the background for the subtle hue.badge.mdOther information
This PR significantly improves the developer experience for theming. By moving logic into
default.tokens.tsand away from hardcoded SCSS functions, designers and developers can now override styles (like the size on a slotted avatar) purely through token configuration.