feat(badge): add recipe and tokens#31043
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.
|
|
||
| /** | ||
| * @virtualProp {"ios" | "md"} mode - The mode determines the platform behaviors of the component. | ||
| * @virtualProp {"ios" | "md" | "ionic"} theme - The theme determines the visual appearance of the component. |
There was a problem hiding this comment.
Forgot to remove this in the first iteration of review.
brandyscarney
left a comment
brandyscarney
left a comment
There was a problem hiding this comment.
Most of my feedback is related to styles and tests. 👍
There was a problem hiding this comment.
Shouldn't the badge always be a circle at minimum (when it contains only a 1)?
There was a problem hiding this comment.
I was trying to keep it as close to the original styling in main but that lead to the inconsistent styles that you see now. I've updated it based on the MD3 specs since they seem similar to MD2. f3b59c2
I left the icon sizes alone so they're still using the sizes from main. Let me now if you prefer them to also be completely round. The reason that I left them alone is because native MD does not support icon badges.
There was a problem hiding this comment.
Ah okay. Yeah I think the icon ones would look better completely round also. This does look better!
There was a problem hiding this comment.
The badge currently has two internal layout modes : content (has text or an icon) and dot (empty). A third case exists in practice, icon only, but is not currently distinguished.
The padding is asymmetric (0 vertical, Xpx horizontal) because that is what the MD3 spec provides. This works correctly for single-digit text badges in order for it to be rendered completely round. Equal padding would cause text badges to become a tall oval because the line height contribution adds height that isn't present horizontally.
The downside is that icon only badges can't be perfectly circular with this padding, resulting in a slightly long oval.
Two options to resolve this:
- Add an icon badge type: a third variant alongside dot and content. Icon only badges would get equal padding on all sides, guaranteeing a perfect circle. The trade-off is a larger API surface and an additional type to maintain and test.
- Leave it as-is: icon-only badges will be slightly non-circular. Acceptable if icon badges are rare or the visual difference is minor at small sizes.
For now, leaving it as-is to avoid scope creep. If icon only becomes a first-class use case, adding the icon type is the clean path. If you still wish for it to be a circle now, let me know so I can just add the third variant.
There was a problem hiding this comment.
Here is how it looks in a native Swift app in iOS 18:
and iOS 26:
It does look like the small size matches better but the font weight is off for both and the padding is off when it contains 999+:
I think the lack of padding on 999+ (in this image 123) is what makes it look off.
There was a problem hiding this comment.
I don't think we need a lot of these, in particular:
The icon-only examples - we should just have 1 example for this (combine top, bottom, start, end, label-hide and do not include icon-hide)
The label-only examples - we should just have 1 example for this (combine top, bottom, start, end, icon-hide and do not include label-hide)
Unless you think the positioning really needs the visual tests here, these all render the same and we don’t really support showing just a badge so we don’t need to verify it. This feedback applies to both badge positions.
There was a problem hiding this comment.
Why is the padding on this gone?
There was a problem hiding this comment.
After digging around, I finally found the right line height! a8bd996
The screenshots are when the font size is very large.
| MD | FW-6837 |
|---|---|
 |
 |
There was a problem hiding this comment.
This padding still looks small, is this right?
There was a problem hiding this comment.
Yes, the badge padding has been verified directly against the native iOS implementation using Xcode's Debug View Hierarchy and Size Inspector.
_UIBarBadgeView is the badge container and UILabel is the text container inside it. Padding was calculated by subtracting the UILabel size from the _UIBarBadgeView size to get the total padding, then dividing by 2 to get the padding on each side, since the label is centered within the container meaning the padding is equally distributed on both sides.
Measurements at default font size:
- Single digit:
_UIBarBadgeView18.33×18.33,UILabel10.33×14.33 -> horizontal (18.33 - 10.33) / 2 = 4px, vertical (18.33 - 14.33) / 2 = 2px - Long text:
_UIBarBadgeView39.33×18.33,UILabel31.33×14.33 -> horizontal (39.33 - 31.33) / 2 = 4px, vertical (18.33 - 14.33) / 2 = 2px
Measurements at maximum accessibility font size:
- Single digit:
_UIBarBadgeView23.33×23.33,UILabel15.33×19.33 -> horizontal (23.33 - 15.33) / 2 = 4px, vertical (23.33 - 19.33) / 2 = 2px - Long text:
_UIBarBadgeView48.67×23.33,UILabel40.67×19.33 -> horizontal (48.67 - 40.67) / 2 = 4px, vertical (23.33 - 19.33) / 2 = 2px
Padding is consistent across both font sizes and badge lengths at 4px horizontal and 2px vertical on all sides.
If you have different values please let me know what they should be and how you can came to get them so I can make sure to use the same source of truth.
I did notice that the line height wasn't accurate so I updated it: 9db6dfc
There was a problem hiding this comment.
We may need to work on the positioning logic or the tab bar height here. The badge is cut off when rendered at the bottom:
You may have already added this to the tab bar ticket.
There was a problem hiding this comment.
Same issue here with badges being cut off. Apologies if you already added this to the tab bar and told me. 😅
There was a problem hiding this comment.
I think it's fine to leave it for now. We can always look into this more when we migrate button.
There was a problem hiding this comment.
Ah okay. Yeah I think the icon ones would look better completely round also. This does look better!
|
|
||
| :host { | ||
| /** | ||
| * @prop --ion-badge-font-family - the font family of the badge text |
There was a problem hiding this comment.
| * @prop --ion-badge-font-family - the font family of the badge text | |
| * @prop --ion-badge-font-family: the font family of the badge text |
The @prop --foo - description syntax is breaking the docs generator. You can see it directly in the api.txt diff, descriptions get concatenated into the next prop's name field and the inline section labels (Bold, Subtle, Shapes, Sizes: Content (...)) bleed into adjacent entries. e.g. --ion-badge-font-family - the font family of the badge text Bold shows up as one entry. Every other component uses : (see chip.scss). All 51 props need the swap, plus the section labels should move onto their own comment lines so they don't get slurped into the previous prop's description.
| @@ -0,0 +1,91 @@ | |||
| import { expect } from '@playwright/test'; | |||
There was a problem hiding this comment.
Should this be renamed to button.e2e.ts? Every other test under button/test/ uses the host component name. Same pattern Brandy flagged on the avatar version in this thread.
| @@ -0,0 +1,137 @@ | |||
| import { expect } from '@playwright/test'; | |||
There was a problem hiding this comment.
Same naming question, should this be tab-button.e2e.ts? See tab-button/test/layout/tab-button.e2e.ts for the existing convention.
| */ | ||
| @Prop() disabled = false; | ||
|
|
||
| componentDidLoad() { |
There was a problem hiding this comment.
componentDidLoad only fires once per Stencil instance, but disconnectedCallback tears down the observer. If the host is moved or re-attached (Vue keep-alive, Angular structural directives, virtual lists), connectedCallback re-fires but componentDidLoad doesn't, so badgeManager.init() never runs again and the badge stops repositioning for the rest of the instance's life.
Same gap exists in button.tsx and tab-button.tsx. One pattern that covers both first-load and reconnect:
private hasLoaded = false;
connectedCallback() {
if (this.hasLoaded) this.badgeManager.init();
}
componentDidLoad() {
this.hasLoaded = true;
this.badgeManager.init();
}| @@ -32,6 +33,43 @@ export class TabButton implements ComponentInterface, AnchorInterface { | |||
|
|
|||
| @Element() el!: HTMLElement; | |||
|
|
|||
| private badgeManager = createBadgeManager(this.el, () => { | |||
There was a problem hiding this comment.
The getConfig closure here reads this.layout to pick between the icon and label as the badge target, but getConfig only re-runs from init() (once) and onSlotChanged() (slot mutations only). If layout is changed programmatically after mount (it's @Prop({ mutable: true }) on line 102), the ResizeObserver keeps watching the old element and the badge ends up at the wrong corner. A @Watch('layout') that calls this.badgeManager.init() should handle it since init() already disconnects the existing observer first.
| positionBadge(config); | ||
| }); | ||
|
|
||
| observer.observe(config.target); |
There was a problem hiding this comment.
Should the observer also watch the badge itself? The arc-based branch uses badgeRect.height / 2 for both top and bottom calculations, so if badge content changes after layout (notification count goes from 9 to 99+, icon swap, font-size scaling) the badge resizes but the position never recomputes, leaving it offset by half the height delta. Notification badges are exactly the use case being shipped, so this seems likely to bite. Adding observer.observe(badge) next to the existing target/relativeTo watches would catch it.
| * | ||
| * If omitted, `target` is used for direction detection. | ||
| */ | ||
| host?: HTMLElement; |
There was a problem hiding this comment.
The "If omitted, target is used for direction detection" framing reads a bit misleading. When target is in shadow DOM (which both button.tsx and tab-button.tsx do), reading dir from the shadow descendant gives a different answer than the light-DOM host, so passing host is effectively required there. Worth either tightening the type, updating the doc to say "Required when target is inside shadow DOM", or auto-falling-back via (target.getRootNode() as ShadowRoot).host?
Co-authored-by: Shane <561207+ShaneK@users.noreply.github.com>
…nect Co-authored-by: Shane <561207+ShaneK@users.noreply.github.com>
Co-authored-by: Shane <561207+ShaneK@users.noreply.github.com>
Co-authored-by: Shane <561207+ShaneK@users.noreply.github.com>
4 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.