diff --git a/.changeset/mosaic-button-variants.md b/.changeset/mosaic-button-variants.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/mosaic-button-variants.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/.claude/skills/mosaic/references/stylex.md b/.claude/skills/mosaic/references/stylex.md index 0f68b16fec5..8ab5bac8569 100644 --- a/.claude/skills/mosaic/references/stylex.md +++ b/.claude/skills/mosaic/references/stylex.md @@ -78,11 +78,29 @@ export const colorVars = stylex.defineVars(colorDefaults); - **DO** name internal, non-contract vars with a `--_cl-*` prefix (e.g. a value a parent writes for a child to read). They still emit verbatim but the `_` marks them "not a contract, don't override." +- **DO** name a radius by **what the surface is**, not by size, so the steps nest: + `--cl-radius-inner` for a mark inside a control, `--cl-radius-control` for the + control itself (button, avatar square), `--cl-radius-container` for anything + wrapping controls. A role name survives a value change; `--cl-radius-md` doesn't. - **DO** compute tints at the call site with `color-mix()`, not as their own tokens. `color-mix(in oklab, ${primary}, ${fg} 12%)` beats minting `--cl-color-primary-hover-12`. - **DON'T** mint a per-step derivative token for something a `calc()`/`color-mix()` can express from an existing token. +- **DO** build a fill that sits _on top of_ an unknown backdrop — a hover or pressed + wash on a transparent `outline`/`ghost` control — as a **scrim**: an opacity of + black-on-light / white-on-dark over `transparent`, not a percentage of a gray token. + + ```ts + const step = `color-mix(in oklab, light-dark(oklch(0 0 0), oklch(1 0 0)) 12%, transparent)`; + ``` + + A gray token like `--cl-color-neutral` is a 900, not black, so the same percentage of + it lands lighter than the percentage of black — and by an amount that shifts with + whatever the control sits on, so the step numbers stop describing what they render. + The scrim composites, so one ramp reads consistently on every surface. This applies + to overlay fills only: a fill with a color of its own (`filled-primary`) still blends + from that color toward its own on-fill. **Spacing is one exposed var plus a `defineConsts` scale.** Only `--cl-spacing` is a custom property; every step is inlined at build as `calc(var(--cl-spacing) * @@ -166,6 +184,64 @@ objects and compose them at the call site. - A helper that assembles a slot may **return an array** of atoms (`[base, direction[x], gap[n]]`) for the caller to spread into `stylex.props`. +### Multiplying axes: flatten, don't plumb vars + +When two visual axes multiply (`color` × `variant`), the tempting move is to have +one axis write custom properties and the other read them — 3 + 4 declarations +instead of 12, with each axis staying independent. **Don't.** + +- **DON'T** declare custom properties inside a component's `stylex.create` to + decouple that component's own axes. They emit into the stylesheet and appear on + every instance in devtools, which reads as public API nobody agreed to support. + A `--cl-*` name is worse still — that prefix is reserved for the overridable + contract (see "Tokens"). +- **DON'T** reach for `defineVars` + `createTheme` to dodge that. The names get + hashed instead of `--cl-*`, but they still land in the inspector on every element. +- **DO** flatten the cross product into one variant map keyed `-`, + indexed directly. A template-literal key makes an unhandled pair a compile error, + with no lookup map to maintain: + + ```ts + const variants = stylex.create({ + 'filled-primary': { + /*…*/ + }, + 'filled-negative': { + /*…*/ + }, + 'outline-primary': { + /*…*/ + }, + // …one entry per cell + }); + // .tsx — TS resolves the template against the literal keys + variants[`${variant}-${color}`]; + ``` + + Each entry is self-contained, so a cell can be read and tuned straight against a + design matrix — which is usually the form these specs arrive in. + +- **DO** hoist a value shared across cells to a **same-file** `const`. StyleX + inlines it at build, so the duplication leaves the source without emitting a var: + + ```ts + const primaryHover = `color-mix(in oklab, ${colorVars['--cl-color-primary']}, ${colorVars['--cl-color-primary-foreground']} 12%)`; + ``` + + Same-file is required — an imported one fails static evaluation ("Atoms" above). + +**Where a var is still right.** This rule is about a component plumbing values to +itself. A custom property remains the correct tool when a value must cross an +element boundary (a parent computes, a descendant reads) or collapse an unbounded +dynamic value into a single atom — the `--_cl-*` cases in "Tokens" and sub-pattern +A in "Dynamic styles". Those have no var-free equivalent; decoupling one element's +own axes does. + +Flattening does scale multiplicatively, so for a genuinely large product reach for +role atoms instead — each color exports the same role names (`solid`, `ink`, +`tint`) and the variant picks which to apply. Still var-free, at the cost of an +indirection the flattened map doesn't have. + ### Conditions & state Use StyleX's conditional-value objects (a `default` plus pseudo / at-rule keys). @@ -173,16 +249,54 @@ Use StyleX's conditional-value objects (a `default` plus pseudo / at-rule keys). ```ts backgroundColor: { default: colorVars['--cl-color-primary'], - ':active': `color-mix(in oklab, ${colorVars['--cl-color-primary']}, ${colorVars['--cl-color-primary-foreground']} 24%)`, + ':active': primaryActive, '@media (hover: hover)': { - // only the top-level value needs `default`; the nested block just adds `:hover` - ':hover': `color-mix(in oklab, ${colorVars['--cl-color-primary']}, ${colorVars['--cl-color-primary-foreground']} 12%)`, + // the media block contributes only the pseudo; the top-level `default` still + // paints the rest state. `:not(:active)` is load-bearing — see below. + default: null, + ':hover:not(:active)': primaryHover, }, }, ``` - **DO** guard `:hover` behind `@media (hover: hover)` so it never sticks on touch; leave `:active`, `:focus-visible`, `:disabled` unguarded. + +#### A media-wrapped `:hover` outranks a bare `:active` + +StyleX encodes precedence by **repeating the class name**, and an at-rule adds +priority. So a `@media (hover: hover)` `:hover` compiles to a doubled selector while +a bare `:active` stays single — and the hover wins while the button is pressed: + +```css +.x1ozrsgg:active { +} /* 0,1,1 — loses */ +@media (hover: hover) { + .x1f1bnkq.x1f1bnkq:hover { + } /* 0,3,0 — wins during the press */ +} +``` + +The symptom is a hovered button that never shows its pressed colour on a mouse +device, while touch devices look correct. + +- **DO** write the hover branch as `':hover:not(:active)'`. It stops the hover rule + **matching** during a press, so the fix rests on selector semantics rather than on + how StyleX happens to order equal-specificity rules. +- **DO** keep the bare `':active'` outside the media query. It is the only pressed + state a no-hover device ever sees, since such a device never matches + `(hover: hover)`. +- **DON'T** fix it by re-declaring `':active'` _inside_ the `@media` block. It does + work — both selectors end up doubled and StyleX emits `:active` after `:hover` — + but it depends on StyleX's emission order for the tiebreak and duplicates the + value in every cell. +- Applies to any state pair where one side is inside an at-rule and the other is + not. Confirm the output rather than trusting it: `pnpm build:mosaic --filter @clerk/ui`, + then grep `dist-mosaic/styles.css` for the two selectors and compare their + specificity. + +Worked example: `packages/ui/src/mosaic/components/button/button.styles.ts`. + - **DO** use `:focus-visible` for focus rings (never bare `:focus`). For a **container** that should ring when a child is focused, use `:has(:focus-visible)` — **not** `:focus-within`. `:focus-within` matches any @@ -208,15 +322,59 @@ backgroundColor: { outlineOffset: { default: null, ':focus-visible': space['0.5'] }, ``` -- **DO** gate every transition/animation on reduced motion, in the same object: +- **DO** take durations from `durationVars` (`--cl-duration-instant` / `-fast` / + `-base` / `-slow` / `-slower`) rather than a literal, and vary them by state + where the feedback should read as more or less direct: ```ts transitionDuration: { default: durationVars['--cl-duration-fast'], + ':active': durationVars['--cl-duration-instant'], + }, + ``` + +- **DO** pick the timing function by **what the property does**, not by how long + it runs. Properties that move — `transform`, `translate`, `scale`, `rotate`, + insets — take `easingVars['--cl-ease-default']`, whose slight overshoot is what + sells the motion. Color and opacity take plain `linear`: their interpolation is + already perceptually non-uniform, so an ease on top only makes the midpoint + drag, and an overshoot extrapolates past the target color for nothing. A + transform at `--cl-duration-fast` still wants the curve. + +- **DO** gate transitions/animations of **motion-bearing** properties on reduced + motion — `transform`, `translate`, `scale`, `rotate`, positional insets — in the + same object. `prefers-reduced-motion` is a vestibular-safety signal, so color + and opacity transitions do **not** need it: + + ```ts + transitionDuration: { + default: durationVars['--cl-duration-base'], '@media (prefers-reduced-motion: reduce)': '0.01ms', }, ``` +- **DO** floor an interactive control's hit area at `targetVars['--cl-target-coarse']` + under `@media (pointer: coarse)`. Use `minHeight`/`minWidth` so the size axis stays + in charge otherwise, and give it its own atom rather than writing it into every + size — the floor is one physical constant, and a part that isn't really a control + (a `link` variant, which reads as text) opts out by not receiving the atom: + + ```ts + touchTarget: { + minHeight: { default: null, '@media (pointer: coarse)': targetVars['--cl-target-coarse'] }, + }, + ``` + + Square controls need the floor on **both** axes, or the target grows tall and narrow. + `--cl-target-coarse` is deliberately off the `--cl-spacing` scale: a consumer + rescaling density must not shrink a touch target with it. + +- **DON'T** suppress interaction with `pointer-events: none` on a disabled control. + An element that isn't hit-tested supplies no cursor, so `cursor: not-allowed` never + renders, and a wrapping tooltip never gets the pointer to explain the disabled + state. Gate the interactive states on `:enabled` instead — `:hover`/`:active` still + match a disabled element, so every one of them needs the gate. + - **DO** reflect runtime conditions the component owns (disabled, selected, invalid) as `data-` attrs via `themeProps`, in addition to the atom, so the state stays overridable in plain consumer CSS. @@ -364,6 +522,7 @@ token colors aren't down-leveled into an invalid polyfill. - YES: `var()`, `calc()`, `color-mix()`, `light-dark()`, nested `@media`+pseudo, `:hover`/`:active`/`:focus-visible`/`:focus-within`/`:disabled`, `:has()`, + `:not()` and compound pseudo keys (`':hover:not(:active)'` compiles and lints), `::before`/`::after`/`::backdrop`, `@starting-style` (enter animations), `stylex.keyframes(...)`, `anchor-size(width|height)` (popover/menu matching its trigger), CSS counters, `@media (hover: hover)` / `(prefers-reduced-motion)` / diff --git a/packages/swingset/src/stories/button.mdx b/packages/swingset/src/stories/button.mdx index ebdbc6f16cb..5c447778fc8 100644 --- a/packages/swingset/src/stories/button.mdx +++ b/packages/swingset/src/stories/button.mdx @@ -35,6 +35,35 @@ Button is the primary action element in Mosaic, used for form submissions and di storyModule={ButtonStories} /> +`size` sets the height, and under a coarse pointer every size floors at 44px instead — a +fingertip is the same width whatever density the surrounding UI runs at, so the floor is a fixed +`--cl-target-coarse` rather than a step on the spacing scale a consumer can rescale. Icon buttons +take the floor on both axes, so a square button stays square rather than growing tall and narrow. +`link` opts out entirely: it reads as text, not a control. None of this is visible on a mouse — +it applies at `@media (pointer: coarse)`. + +### Variants + + + +### Colors + +`color` and `variant` are independent: `color` picks the palette, `variant` decides how much of +it to show. Every combination below is the same two props. + +Hover and focus are part of the matrix — hover a cell, or tab through it, to see them. `filled` +shifts its fill and `link` underlines. `outline` and `ghost` share the same neutral hover fill, +except `ghost` under `negative`, which is the one cell that tints by color. The focus ring is +the same for every cell. + + + ### Shapes +### Icons + +An icon is a child, not a prop. Give it a `placement` and `Icon` reflects that as `data-icon`, +which the button selects on to tighten the padding on that edge — pass icons on either side, or +both, with nothing to declare on the button itself. + + + +The tightened value isn't chosen by eye. An icon is centered in the button, so it already has +`(height − icon size) / 2` of space above and below it; matching the inline side to that puts it +in a square cell — 8px at every size, against 12px of text padding at `md` and `lg`, 10px at `sm`. +The text side keeps the larger inset, since a run of text ends in a stem where an icon trails off. +The middle button in each row below carries no icon, for comparison. + + + +### Truncation + +A button sizes to its own content and doesn't shrink, so most of the time there's nothing to +truncate. When its width _is_ constrained — `fullWidth` in a narrow container, or an explicit +width — the label can't wrap to cope, because `size` fixes the height and a second line would +grow out of the button. It stays on one line and ends in an ellipsis instead. + +Only text children truncate. Button wraps a text child in a span of its own to run the ellipsis +against, since a bare text node is laid out in an anonymous box no selector can reach. Element +children pass through as direct flex items, so an icon keeps its size and its padding while the +label gives way around it. + + + ### Disabled + +A disabled button keeps its resting fill and dims — it doesn't fall back to a gray of its own, so +which color and variant it is stays legible while it's unavailable. Hover and press are suppressed +by the styles, not by `pointer-events`: the button stays hit-testable, which is what makes +`cursor: not-allowed` render at all and what lets a wrapping tooltip explain _why_ it's disabled. +That tooltip is worth adding — a disabled control with no explanation is a dead end. diff --git a/packages/swingset/src/stories/button.stories.tsx b/packages/swingset/src/stories/button.stories.tsx index 8a347b791ec..03fc14af232 100644 --- a/packages/swingset/src/stories/button.stories.tsx +++ b/packages/swingset/src/stories/button.stories.tsx @@ -1,6 +1,8 @@ /** @jsxImportSource @emotion/react */ import type { ButtonProps } from '@clerk/ui/mosaic/components/button'; import { Button } from '@clerk/ui/mosaic/components/button'; +import { Icon } from '@clerk/ui/mosaic/components/icon'; +import React from 'react'; import type { StoryMeta } from '@/lib/types'; @@ -15,14 +17,14 @@ export const meta: StoryMeta = { styleEngine: 'stylex', styles: { _variants: { - intent: { primary: {}, destructive: {} }, - variant: { filled: {}, outline: {}, ghost: {} }, - size: { sm: {}, md: {} }, + color: { primary: {}, neutral: {}, negative: {} }, + variant: { filled: {}, outline: {}, ghost: {}, link: {} }, + size: { sm: {}, md: {}, lg: {} }, shape: { default: {}, square: {}, circle: {} }, fullWidth: { true: {}, false: {} }, }, _defaultVariants: { - intent: 'primary', + color: 'primary', variant: 'filled', size: 'md', shape: 'default', @@ -56,6 +58,78 @@ export function Sizes(props: Record) { > Medium + + + ); +} + +export function Variants(props: Record) { + return ( +
+ + + + +
+ ); +} + +// The full variant × color matrix, laid out like the design spec: variants down, +// colors across. Every cell is the same two props, so the axes stay legible side +// by side — and hover/focus are live, since those states are what the matrix is for. +export function Colors(props: Record) { + const label = { fontSize: 12, color: '#71717a', alignSelf: 'center' } as const; + return ( +
+ + {(['primary', 'neutral', 'negative'] as const).map(color => ( + + {color} + + ))} + {(['filled', 'outline', 'ghost', 'link'] as const).map(variant => ( + + {variant} + {(['primary', 'neutral', 'negative'] as const).map(color => ( + + ))} + + ))}
); } @@ -147,6 +221,116 @@ export function Shapes(props: Record) { ); } +// `check` and `chevron-down`, not the horizontal chevrons: those draw about half the ink in the +// same box, so they read as a padding bug until the glyph set is optically normalized. +export function Icons(props: Record) { + return ( +
+ + + +
+ ); +} + +// The same three shapes at every size, so the tightened edge can be read against the +// untightened one — each row's middle button is the size's plain text padding. +export function IconSizes(props: Record) { + return ( +
+ {(['sm', 'md', 'lg'] as const).map(size => ( +
+ + + +
+ ))} +
+ ); +} + +// `fullWidth` in a narrow container is what constrains the width — a button sizes to its own +// content otherwise (`flex-shrink: 0`), so there'd be nothing to truncate against. The icon +// keeps its size and only the label gives way. +export function Truncation(props: Record) { + return ( +
+ + + +
+ ); +} + export function Disabled(props: Record) { return ( diff --git a/packages/swingset/src/stories/dialog.component.stories.tsx b/packages/swingset/src/stories/dialog.component.stories.tsx index 42c773e62c4..afe4b77b335 100644 --- a/packages/swingset/src/stories/dialog.component.stories.tsx +++ b/packages/swingset/src/stories/dialog.component.stories.tsx @@ -15,7 +15,9 @@ export const meta: StoryMeta = { styles: dialogRecipe, }; -const dialogTrigger = (props: React.HTMLAttributes) => ; +const dialogTrigger = ({ color: _nativeColor, ...props }: React.HTMLAttributes) => ( + +); export function Default(args: Record) { const { size } = args as { size?: 'md' | 'lg' }; @@ -29,7 +31,7 @@ export function Default(args: Record) { Confirm action Are you sure you want to proceed? This action cannot be undone. ); const button = screen.getByRole('button'); expect(button).toHaveClass('cl-button'); - expect(button).toHaveAttribute('data-intent', 'primary'); + expect(button).toHaveAttribute('data-color', 'primary'); expect(button).toHaveAttribute('data-variant', 'filled'); expect(button).toHaveAttribute('data-size', 'md'); expect(button).toHaveAttribute('data-shape', 'default'); expect(button).toHaveAttribute('type', 'button'); }); + it('omits the boolean modifiers when they are off', () => { + render(); + const button = screen.getByRole('button'); + expect(button).not.toHaveAttribute('data-full-width'); + expect(button).not.toHaveAttribute('data-disabled'); + expect(button).toBeEnabled(); + }); + it('wires variant props and consumer className/style through to the element', () => { render( , ); const button = screen.getByRole('button'); - expect(button).toHaveAttribute('data-intent', 'destructive'); + expect(button).toHaveAttribute('data-color', 'negative'); expect(button).toHaveAttribute('data-variant', 'outline'); expect(button).toHaveAttribute('data-size', 'sm'); expect(button).toHaveAttribute('data-shape', 'circle'); @@ -45,6 +54,112 @@ describe('Mosaic Button', () => { expect(button).toHaveStyle({ marginTop: '8px' }); }); + it.each(['primary', 'neutral', 'negative'] as const)('reflects the %s color', color => { + render(); + expect(screen.getByRole('button')).toHaveAttribute('data-color', color); + }); + + it.each(['filled', 'outline', 'ghost', 'link'] as const)('reflects the %s variant', variant => { + render(); + expect(screen.getByRole('button')).toHaveAttribute('data-variant', variant); + }); + + it.each(['sm', 'md', 'lg'] as const)('reflects the %s size', size => { + render(); + expect(screen.getByRole('button')).toHaveAttribute('data-size', size); + }); + + it.each(['default', 'square', 'circle'] as const)('reflects the %s shape', shape => { + render(); + expect(screen.getByRole('button')).toHaveAttribute('data-shape', shape); + }); + + it('gives a text child its own box to truncate against', () => { + render(); + const label = screen.getByRole('button').firstElementChild; + expect(label?.tagName).toBe('SPAN'); + expect(label).toHaveTextContent('Hi'); + }); + + it('keeps a run of adjacent text in one box so the gap stays out of the sentence', () => { + render(); + const button = screen.getByRole('button'); + expect(button.children).toHaveLength(1); + expect(button.firstElementChild?.tagName).toBe('SPAN'); + expect(button).toHaveAccessibleName('Delete Alice'); + }); + + it('splits the label runs around an element child', () => { + render( + , + ); + const button = screen.getByRole('button'); + expect(button.children).toHaveLength(3); + expect(button.children[0]).toHaveTextContent('Delete'); + expect(button.children[1]).toBe(screen.getByTestId('icon')); + expect(button.children[2]).toHaveTextContent('Alice'); + }); + + it('leaves element children as direct children so the gap still applies', () => { + render( + , + ); + const button = screen.getByRole('button'); + expect(button.children).toHaveLength(2); + expect(button.firstElementChild).toBe(screen.getByTestId('icon')); + expect(button.lastElementChild?.tagName).toBe('SPAN'); + expect(button).toHaveAccessibleName('Hi'); + }); + + it('boxes a numeric child the same as text', () => { + render(); + const button = screen.getByRole('button'); + expect(button.children).toHaveLength(1); + expect(button.firstElementChild?.tagName).toBe('SPAN'); + expect(button).toHaveAccessibleName('3'); + }); + + it('opens no label box for children that render nothing', () => { + render( + , + ); + const button = screen.getByRole('button'); + expect(button.children).toHaveLength(1); + expect(button.firstElementChild).toBe(screen.getByTestId('icon')); + }); + + it('calls onClick when pressed', async () => { + const onClick = vi.fn(); + render(); + await userEvent.click(screen.getByRole('button')); + expect(onClick).toHaveBeenCalledTimes(1); + }); + + it('does not call onClick while disabled', async () => { + const onClick = vi.fn(); + render( + , + ); + await userEvent.click(screen.getByRole('button')); + expect(onClick).not.toHaveBeenCalled(); + }); + it('reflects disabled as both the native attribute and data-disabled', () => { render(); const button = screen.getByRole('button'); diff --git a/packages/ui/src/mosaic/components/button/button.tsx b/packages/ui/src/mosaic/components/button/button.tsx index aa38245d477..93e92d42e30 100644 --- a/packages/ui/src/mosaic/components/button/button.tsx +++ b/packages/ui/src/mosaic/components/button/button.tsx @@ -1,28 +1,31 @@ import * as stylex from '@stylexjs/stylex'; import React from 'react'; +import type { MosaicComponentProps } from '../../props'; import { mergeStyleProps, themeProps } from '../../props'; -import { styles } from './button.styles'; +import { truncationStyles } from '../typography.styles'; +import { iconSizes, sizes, styles, variants } from './button.styles'; -export interface ButtonProps extends React.ComponentPropsWithRef<'button'> { - intent?: 'primary' | 'destructive'; - variant?: 'filled' | 'outline' | 'ghost'; - size?: 'sm' | 'md'; +export interface ButtonProps extends Omit, 'render'> { + color?: 'primary' | 'neutral' | 'negative'; + variant?: 'filled' | 'outline' | 'ghost' | 'link'; + size?: 'sm' | 'md' | 'lg'; shape?: 'default' | 'square' | 'circle'; fullWidth?: boolean; } /** * A clickable action styled by the Mosaic recipe. Renders a `button` and forwards its - * ref; `intent`, `variant`, `size`, and `shape` compose to cover the full set of styles. + * ref; `color`, `variant`, and `size` are independent axes, with `shape` and `fullWidth` + * as orthogonal modifiers. * * @example * // Default (primary, filled, md) * * * @example - * // Destructive intent with a non-filled variant - * + * // Negative color with a non-filled variant + * * * @example * // Icon-only, circular, small @@ -32,9 +35,48 @@ export interface ButtonProps extends React.ComponentPropsWithRef<'button'> { * // Full-width ghost button * */ +// Wrap the text children so they have a box of their own to truncate against — a bare text +// child is laid out in an anonymous flex item that no selector can reach. A whole run of +// adjacent text shares one box, or `Delete {name}` would split into two flex items with the +// button's `gap` opening up mid-sentence. Element children (icons) pass through untouched, +// so they stay direct flex items and `gap` still applies. +function withTruncatableLabel(children: React.ReactNode): React.ReactNode { + const result: React.ReactNode[] = []; + let run: React.ReactNode[] = []; + + const flushRun = () => { + if (run.length === 0) { + return; + } + result.push( + + {run} + , + ); + run = []; + }; + + // `toArray` rather than `forEach` so the elements it passes through carry the keys it + // assigns, and the array this returns doesn't warn about missing ones. + for (const child of React.Children.toArray(children)) { + if (typeof child === 'string' || typeof child === 'number') { + run.push(child); + } else { + flushRun(); + result.push(child); + } + } + flushRun(); + + return result; +} + export const Button = React.forwardRef(function MosaicButton( { - intent = 'primary', + color = 'primary', variant = 'filled', size = 'md', shape = 'default', @@ -54,19 +96,16 @@ export const Button = React.forwardRef(function type='button' disabled={disabled} {...mergeStyleProps( - themeProps('button', { intent, variant, size, shape, fullWidth, disabled }), + themeProps('button', { color, variant, size, shape, fullWidth, disabled }), stylex.props( styles.base, - variant === 'filled' && intent === 'primary' && styles.filledPrimary, - variant === 'filled' && intent === 'destructive' && styles.filledDestructive, - variant === 'outline' && intent === 'primary' && styles.outlinePrimary, - variant === 'outline' && intent === 'destructive' && styles.outlineDestructive, - variant === 'ghost' && intent === 'primary' && styles.ghostPrimary, - variant === 'ghost' && intent === 'destructive' && styles.ghostDestructive, - size === 'sm' ? styles.sizeSm : styles.sizeMd, + sizes[size], + variants[`${variant}-${color}`], shape === 'square' && styles.shapeSquare, shape === 'circle' && styles.shapeCircle, - isIconShape && (size === 'sm' ? styles.iconSizeSm : styles.iconSizeMd), + isIconShape && iconSizes[size], + variant !== 'link' && styles.touchTarget, + variant !== 'link' && isIconShape && styles.touchTargetIcon, fullWidth && styles.fullWidth, disabled && styles.disabled, ), @@ -75,7 +114,7 @@ export const Button = React.forwardRef(function )} {...rest} > - {children} + {withTruncatableLabel(children)} ); }); diff --git a/packages/ui/src/mosaic/components/icon/icon.markers.stylex.ts b/packages/ui/src/mosaic/components/icon/icon.markers.stylex.ts new file mode 100644 index 00000000000..8f989ce3ed1 --- /dev/null +++ b/packages/ui/src/mosaic/components/icon/icon.markers.stylex.ts @@ -0,0 +1,10 @@ +import * as stylex from '@stylexjs/stylex'; + +/** + * Scopes a container's `data-icon` selectors to a Mosaic `Icon`, so a stray `data-icon` elsewhere + * in the subtree can't trigger them. `Icon` applies it; `Button`'s inline padding matches on it. + * + * Its own module because `@stylexjs/enforce-extension` requires the define-primitives to live in + * a `.stylex.ts` file. + */ +export const iconScope = stylex.defineMarker(); diff --git a/packages/ui/src/mosaic/components/icon/icon.tsx b/packages/ui/src/mosaic/components/icon/icon.tsx index d2c62aec434..4323053f184 100644 --- a/packages/ui/src/mosaic/components/icon/icon.tsx +++ b/packages/ui/src/mosaic/components/icon/icon.tsx @@ -5,6 +5,7 @@ import { useMosaicIcons } from '../../appearance'; import type { IconName } from '../../icons/registry'; import { iconRegistry } from '../../icons/registry'; import { mergeStyleProps, themeProps } from '../../props'; +import { iconScope } from './icon.markers.stylex'; import { sizes, styles } from './icon.styles'; export interface IconProps extends React.ComponentPropsWithRef<'svg'> { @@ -32,7 +33,7 @@ export const Icon = React.forwardRef(function MosaicIc // child by what it is: `:has([data-icon='inline-end'])` can't match some other placed descendant. const props = mergeStyleProps( themeProps('icon', { size, icon: placement }), - stylex.props(styles.base, sizes[size]), + stylex.props(styles.base, sizes[size], iconScope), className, style, ); diff --git a/packages/ui/src/mosaic/organization/organization-profile-delete-section.view.tsx b/packages/ui/src/mosaic/organization/organization-profile-delete-section.view.tsx index 2c8f75d3b9e..5f6b35ea6e3 100644 --- a/packages/ui/src/mosaic/organization/organization-profile-delete-section.view.tsx +++ b/packages/ui/src/mosaic/organization/organization-profile-delete-section.view.tsx @@ -59,8 +59,8 @@ export function OrganizationProfileDeleteSectionView({ (