Skip to content

Conversation

@thetaPC
Copy link
Contributor

@thetaPC thetaPC commented Dec 15, 2025

Issue number: resolves internal


What is the current behavior?

Component styles for ion-chip are fragmented across multiple files (ios, md, ionic). Developers were restricted to those themes and how those themes structured the logic and styles.

What is the new behavior?

  • Unified Style Architecture: Combined theme-specific styling into a single chip.base.scss file that consumes CSS variables, ensuring a single source of truth for component logic.
  • Property-First Token Structure: Refactored the IonChip type and tokens to prioritize a Property-First approach. Instead of flat or element-centric tokens, they are nested by element and then by property (e.g., icon.firstChild.margin.end). This ensures:
    • Predictability: Developers can determine token names based on the CSS property they wish to override.
    • Consistency: The structure matches other system keys like padding and border.
    • Granularity: Allows for specific overrides of individual directions without cluttering the top-level interface.
  • Defined TypeScript Interfaces: Added chip.interfaces.ts with reusable utility types like HueRef to standardize the "Bold vs. Subtle" logic used in variants and interaction states.
  • Standardized Sizing: Introduced a "small" to md and ios.
  • Standardized Hues: Introduced a "subtle" to md and ios.
  • Defined Defaults: Updated defaults to be consistent regardless of theme.

Does this introduce a breaking change?

  • Yes
  • No

This PR introduces a breaking change to how IonChip is styled. Existing manual CSS overrides targeting internal chip structures or old token names will no longer work due to the shift to a Property-First token hierarchy and a unified base SCSS file.

Migration Path:

  1. Token Updates: Update any custom theme configurations to match the new nested structure.

  2. CSS Overrides: Ensure selectors align with the new slotted element logic and variable names (e.g., --ion-chip-hue-bold-bg).

--background should no longer be used. Setting the value with IonChip.hue.bold.bg and IonChip.hue.subtle.bg within the tokens file should be used to change the background based on the hue.

  1. Properties: Update any references to match old defaults based on the theme.

If md or ios with no shape, then update the component to default to soft.

Other information

This PR significantly improves the developer experience for theming. By moving logic into default.tokens.ts and away from hardcoded SCSS functions, designers and developers can now override complex states (like an activated outline chip) purely through token configuration.

Preview:

@vercel
Copy link

vercel bot commented Dec 15, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
ionic-framework Ready Ready Preview, Comment Jan 20, 2026 6:00pm

Request Review

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need Sass variables for this? I feel like it just adds another layer when the CSS variables would do

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created it so we don't have such long variables cluttering the base file. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up removing it: 4dd7e23

};

type Components = {
IonChip?: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we should keep these interfaces in the component's folder to keep it better organized? Or is this something we might move later and combine between components?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 95 to 96
paddingVertical: '6px',
paddingHorizontal: '12px',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
paddingVertical: '6px',
paddingHorizontal: '12px',
padding: {
vertical: '6px',
horizontal: '12px',
}

Would it not be cleaner to do it as objects?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. We might want to move to top, end, start and bottom for all components for consistency but we can tackle that when we get there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I lean towards tackling it when we get there. Mainly because I would like to keep the tokens to a bare minimum. But I'm open to do it now if you prefer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of these values are hardcoded - why aren't we using the design tokens like this:

--ion-scaling-xxs
--ion-scaling-xs
--ion-scaling-sm
etc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was in early stages of just testing. Here's the refined version: 401ef1f

},

avatar: {
size: `${24 / fontSizes.chipBase}em`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These numbers seem made up - maybe we need to define these calculations better?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The avatar size is based on the existing values from the original vars file.

}

const fontSizes = {
chipBase: 14,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the base 14 for Chip? Shouldn't all components have the same base size?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original value is 14 based on the vars file. I don't remember why 14 was chosen.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comments on the md file apply here as well

thetaPC and others added 3 commits December 15, 2025 16:50
Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
@include mixins.border-radius(var(--border-radius));
@include mixins.margin(vars.$chip-margin);
@include mixins.padding(vars.$chip-padding-vertical, vars.$chip-padding-horizontal);
@include mixins.typography(vars.$chip-typography);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The font family for chip on ionic-modular and this branch do not match when it comes to the ionic theme. The ionic theme uses ion-body-sm-medium to set the typography for chip. This variable includes font family with the value of -apple-system, system-ui, "Segoe UI", Roboto, Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol". This is exactly what is being rendered on this branch. However, the ionic-modular renders it with a value of var(--ion-font-family, inherit). Based on my investigation, ionic-modular and next are not loading the correct font family. This branch fixes it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes for ios are happening because

  • the shape default is now round instead of soft.
  • the hue default is now subtle instead of bold.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes for md are happening because

  • the shape default is now round instead of soft.
  • the hue default is now subtle instead of bold.

@thetaPC thetaPC marked this pull request as ready for review January 19, 2026 17:56
@thetaPC thetaPC requested a review from a team as a code owner January 19, 2026 17:56
Comment on lines +170 to +175
const customConfig = customTheme?.config;
if (customConfig) {
Object.entries(customConfig).forEach(([key, value]) => {
config.set(key as keyof IonicConfig, value);
});
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed for configs like rippleEffect to be set if the user passed them through the customTheme obj.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: angular @ionic/angular package package: core @ionic/core package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants