diff --git a/.changeset/mosaic-xstyle-props.md b/.changeset/mosaic-xstyle-props.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/mosaic-xstyle-props.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/.claude/skills/mosaic/SKILL.md b/.claude/skills/mosaic/SKILL.md index 6a9f74da1ed..c876dbb6c20 100644 --- a/.claude/skills/mosaic/SKILL.md +++ b/.claude/skills/mosaic/SKILL.md @@ -19,7 +19,8 @@ Two things live under Mosaic, and this skill covers the how-to for both: - **Styled components** are authored with **StyleX** — `stylex.create` declares the styles, `themeProps` emits the part's public identity (the `.cl-` class plus `data-` attrs), and `mergeStyleProps` fuses the two with the - consumer's `className`/`style`. + props the part was called with. A part takes `xstyle` (StyleX atoms for its + root), never `className`/`style`. - **Flows** follow a **model → controller → view** split — _where the data comes from_ → _what the user is doing to it_ → _what that looks like_. What crosses each boundary is plain data: no Clerk resource reaches the controller, no diff --git a/.claude/skills/mosaic/references/stylex.md b/.claude/skills/mosaic/references/stylex.md index 5c6675e468a..47c03e5e9fb 100644 --- a/.claude/skills/mosaic/references/stylex.md +++ b/.claude/skills/mosaic/references/stylex.md @@ -25,7 +25,7 @@ conventions to follow by hand, not guarantees the toolchain makes for you. | `utils/` | everything shared across components — styles and non-style helpers alike | | `/.markers.stylex.ts` | `stylex.defineMarker()` results for scoped ancestor states | | `/.tsx` | component; spreads `stylex.props(...)` via `mergeStyleProps` | -| `props.ts` | `themeProps` (`.cl-` + `data-`) + `mergeStyleProps` | +| `props.ts` | `themeProps` (`.cl-` + `data-`), `mergeStyleProps`, the `Mosaic*Props` types | | `styles/index.ts` | isolated-build barrel; derives `*VarName` types | The `@stylexjs` eslint rules run on `src/mosaic/**`. The `enforce-extension` @@ -577,28 +577,70 @@ The element carries three things, and nothing else is a contract: plus a kebab-cased `data-` reflection of the visual props, so consumers target stable data-attribute selectors, not collision-prone class names. -`mergeStyleProps` fuses everything in precedence order — **theme props → StyleX atoms → -consumer `className`/`style`** — so the consumer always wins. It concatenates -className left-to-right and merges `style` with the consumer object spread last: +`mergeStyleProps` fuses its bags left to right — **theme props → StyleX atoms → +the props the part was called with** — concatenating `className`, shallow-merging +`style` with the later bag winning, and letting the later bag overwrite anything else: ```tsx -); @@ -60,7 +65,7 @@ describe('Mosaic Button', () => { expect(screen.getByRole('button')).not.toHaveAttribute('touchtarget'); }); - it('wires variant props and consumer className/style through to the element', () => { + it('wires variant props and xstyle atoms through to the element', () => { render( , @@ -80,8 +84,7 @@ describe('Mosaic Button', () => { expect(button).toHaveAttribute('data-size', 'sm'); expect(button).toHaveAttribute('data-shape', 'circle'); expect(button).toHaveAttribute('data-full-width', ''); - expect(button).toHaveClass('cl-button', 'my-button'); - expect(button).toHaveStyle({ marginTop: '8px' }); + expect(button).toHaveClass('cl-button', stylex.props(atoms.spaced).className ?? ''); }); it.each(['primary', 'neutral', 'negative'] as const)('reflects the %s color', color => { diff --git a/packages/ui/src/mosaic/components/button/button.tsx b/packages/ui/src/mosaic/components/button/button.tsx index 5ed6e0f63e9..ae516872c93 100644 --- a/packages/ui/src/mosaic/components/button/button.tsx +++ b/packages/ui/src/mosaic/components/button/button.tsx @@ -104,8 +104,7 @@ export const Button = React.forwardRef(function touchTarget = true, disabled: disabledProp, focusableWhenDisabled = false, - className, - style, + xstyle, children, ...rest }, @@ -144,11 +143,10 @@ export const Button = React.forwardRef(function fullWidth && styles.fullWidth, disabled && styles.disabled, defaults.styles, + xstyle, ), - className, - style, + rest, )} - {...rest} > {withTruncatableLabel(children)} diff --git a/packages/ui/src/mosaic/components/button/submit-button.test.tsx b/packages/ui/src/mosaic/components/button/submit-button.test.tsx index c9ce86b091f..ccbb7f99a01 100644 --- a/packages/ui/src/mosaic/components/button/submit-button.test.tsx +++ b/packages/ui/src/mosaic/components/button/submit-button.test.tsx @@ -1,3 +1,4 @@ +import * as stylex from '@stylexjs/stylex'; import { act, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; @@ -5,6 +6,10 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { SubmitButton } from './submit-button'; +const consumerAtoms = stylex.create({ + spaced: { marginTop: '8px' }, +}); + /** The spinner is decorative, so it has no role or name to query — only its slot class. */ const spinner = () => document.querySelector('.cl-spinner'); const content = () => screen.getByRole('button').firstElementChild; @@ -193,15 +198,14 @@ describe('Mosaic SubmitButton', () => { expect(spinner()).toHaveAttribute('data-size', expected); }); - it('wires the button variant props and consumer className/style through', () => { + it('wires the button variant props and xstyle atoms through', () => { render( Save , @@ -211,8 +215,7 @@ describe('Mosaic SubmitButton', () => { expect(button).toHaveAttribute('data-variant', 'outline'); expect(button).toHaveAttribute('data-size', 'sm'); expect(button).toHaveAttribute('data-full-width', ''); - expect(button).toHaveClass('cl-button', 'my-button'); - expect(button).toHaveStyle({ marginTop: '8px' }); + expect(button).toHaveClass('cl-button', stylex.props(consumerAtoms.spaced).className ?? ''); }); it('keeps the isPending prop off the element', () => { diff --git a/packages/ui/src/mosaic/components/button/submit-button.tsx b/packages/ui/src/mosaic/components/button/submit-button.tsx index 29f0b6996ad..3d0f3fa141a 100644 --- a/packages/ui/src/mosaic/components/button/submit-button.tsx +++ b/packages/ui/src/mosaic/components/button/submit-button.tsx @@ -58,7 +58,7 @@ const DEFAULT_SPIN_DELAY = 300; * Delete */ export const SubmitButton = React.forwardRef(function MosaicSubmitButton( - { isPending = false, pendingLabel = 'pending', size = 'md', spinDelay, className, children, onClick, ...rest }, + { isPending = false, pendingLabel = 'pending', size = 'md', spinDelay, xstyle, children, onClick, ...rest }, ref, ) { const { delay = DEFAULT_SPIN_DELAY, minDuration } = spinDelay ?? {}; @@ -93,7 +93,7 @@ export const SubmitButton = React.forwardRef ) : null} diff --git a/packages/ui/src/mosaic/components/card/card.test.tsx b/packages/ui/src/mosaic/components/card/card.test.tsx index 813f13f5b2a..bf6cb94aca5 100644 --- a/packages/ui/src/mosaic/components/card/card.test.tsx +++ b/packages/ui/src/mosaic/components/card/card.test.tsx @@ -9,6 +9,13 @@ import { Card } from './card'; const compactCard = '@container card (max-width: 20rem)' as const; +const callerStyles = stylex.create({ + root: { width: '20rem' }, + header: { textAlign: 'right' }, + content: { paddingInline: 0 }, + footer: { paddingBlockEnd: 0 }, +}); + const responsiveLayout = stylex.create({ root: { containerName: 'card', @@ -80,39 +87,52 @@ describe('Mosaic Card', () => { expect(screen.getByTestId('footer')).toHaveAttribute('data-elevation', 'overlay'); }); - it('lets consumer className and style win on every slot', () => { + it('composes caller xstyle onto every slot', () => { render( , ); - expect(screen.getByTestId('root')).toHaveClass('cl-card-root', 'my-card'); - expect(screen.getByTestId('root')).toHaveStyle({ width: '20rem' }); - expect(screen.getByTestId('header')).toHaveClass('cl-card-header', 'my-header'); - expect(screen.getByTestId('header')).toHaveStyle({ textAlign: 'right' }); - expect(screen.getByTestId('content')).toHaveClass('cl-card-content', 'my-content'); - expect(screen.getByTestId('content')).toHaveStyle({ paddingInline: 0 }); - expect(screen.getByTestId('footer')).toHaveClass('cl-card-footer', 'my-footer'); - expect(screen.getByTestId('footer')).toHaveStyle({ paddingBlockEnd: 0 }); + expect(screen.getByTestId('root')).toHaveClass('cl-card-root', stylex.props(callerStyles.root).className ?? ''); + expect(screen.getByTestId('header')).toHaveClass( + 'cl-card-header', + stylex.props(callerStyles.header).className ?? '', + ); + expect(screen.getByTestId('content')).toHaveClass( + 'cl-card-content', + stylex.props(callerStyles.content).className ?? '', + ); + expect(screen.getByTestId('footer')).toHaveClass( + 'cl-card-footer', + stylex.props(callerStyles.footer).className ?? '', + ); + }); + + it('merges the className and style a render source hands a slot', () => { + render( + + } /> + , + ); + + // `Card.Header` clones its merged class onto the `Card.Content` it renders; the content + // keeps its own slot class rather than being overwritten by the incoming one. + expect(screen.getByTestId('header')).toHaveClass('cl-card-header', 'cl-card-content'); }); it('forwards refs and arbitrary props from compound slots', () => { diff --git a/packages/ui/src/mosaic/components/card/card.tsx b/packages/ui/src/mosaic/components/card/card.tsx index 727239cb8ac..8b4c6a0f1ac 100644 --- a/packages/ui/src/mosaic/components/card/card.tsx +++ b/packages/ui/src/mosaic/components/card/card.tsx @@ -39,7 +39,7 @@ export interface CardProps extends MosaicComponentProps<'div'> { } const Root = React.forwardRef(function CardRoot( - { elevation = DEFAULT_ELEVATION, renderBranding = true, render, className, style, children, ...rest }, + { elevation = DEFAULT_ELEVATION, renderBranding = true, render, xstyle, children, ...rest }, ref, ) { const element = useRender({ @@ -49,11 +49,9 @@ const Root = React.forwardRef(function CardRoot( props: { ...mergeStyleProps( themeProps('card-root', { elevation }), - stylex.props(reset.base, slots.root.base, slots.root[elevation]), - className, - style, + stylex.props(reset.base, slots.root.base, slots.root[elevation], xstyle), + rest, ), - ...rest, children: ( <> {children} @@ -90,7 +88,7 @@ function HeaderCloseButton() { } const Header = React.forwardRef>(function CardHeader( - { render, className, style, children, ...rest }, + { render, xstyle, children, ...rest }, ref, ) { const dialog = React.useContext(DialogContext); @@ -99,8 +97,7 @@ const Header = React.forwardRef>(fun render, ref, props: { - ...mergeStyleProps(themeProps('card-header'), stylex.props(reset.base, slots.header.base), className, style), - ...rest, + ...mergeStyleProps(themeProps('card-header'), stylex.props(reset.base, slots.header.base, xstyle), rest), children: ( <> {/* First in the DOM, so it is the first tabbable element and takes the dialog's opening @@ -121,7 +118,7 @@ const Header = React.forwardRef>(fun * `aria-labelledby` at, so the card names the dialog without knowing it is in one. */ const Title = React.forwardRef>(function CardTitle( - { render, className, style, ...rest }, + { render, xstyle, ...rest }, ref, ) { const dialog = React.useContext(DialogContext); @@ -130,8 +127,7 @@ const Title = React.forwardRef>(f render, ref, props: { - ...mergeStyleProps(themeProps('card-title'), stylex.props(reset.base, slots.header.title), className, style), - ...rest, + ...mergeStyleProps(themeProps('card-title'), stylex.props(reset.base, slots.header.title, xstyle), rest), // The popup points `aria-labelledby` at this id, so the surface outranks the caller: an id // that displaced it would leave the dialog with no accessible name. ...(dialog && { id: dialog.labelId }), @@ -141,7 +137,7 @@ const Title = React.forwardRef>(f /** Describes the card. The `aria-describedby` counterpart to {@link Title}. */ const Description = React.forwardRef>(function CardDescription( - { render, className, style, ...rest }, + { render, xstyle, ...rest }, ref, ) { const dialog = React.useContext(DialogContext); @@ -152,18 +148,16 @@ const Description = React.forwardRef>(function CardContent( - { render, className, style, ...rest }, + { render, xstyle, ...rest }, ref, ) { return useRender({ @@ -173,17 +167,15 @@ const Content = React.forwardRef>(fu props: { ...mergeStyleProps( themeProps('card-content'), - stylex.props(reset.base, slots.content.base, cardContentMarker), - className, - style, + stylex.props(reset.base, slots.content.base, cardContentMarker, xstyle), + rest, ), - ...rest, }, }); }); const Footer = React.forwardRef>(function CardFooter( - { render, className, style, ...rest }, + { render, xstyle, ...rest }, ref, ) { const elevation = React.useContext(CardElevationContext); @@ -194,11 +186,9 @@ const Footer = React.forwardRef>(fun props: { ...mergeStyleProps( themeProps('card-footer', { elevation }), - stylex.props(reset.base, slots.footer.base), - className, - style, + stylex.props(reset.base, slots.footer.base, xstyle), + rest, ), - ...rest, }, }); }); diff --git a/packages/ui/src/mosaic/components/combobox/combobox.tsx b/packages/ui/src/mosaic/components/combobox/combobox.tsx index fc275a1ac7e..931740225ca 100644 --- a/packages/ui/src/mosaic/components/combobox/combobox.tsx +++ b/packages/ui/src/mosaic/components/combobox/combobox.tsx @@ -39,14 +39,13 @@ export function ComboboxRoot({ sideOffset = 8, ...props }: ComboboxRootProps) { } export const ComboboxTrigger = React.forwardRef(function MosaicComboboxTrigger( - { className, style, ...props }, + { xstyle, ...rest }, ref, ) { return ( ); }); @@ -57,7 +56,7 @@ export interface ComboboxInputProps extends Omit, } export const ComboboxInput = React.forwardRef(function MosaicComboboxInput( - { size: sizeProp, variant: variantProp, render, className, style, ...rest }, + { size: sizeProp, variant: variantProp, render, xstyle, ...rest }, ref, ) { const inputGroup = useOptionalInputGroupContext(); @@ -81,8 +80,7 @@ export const ComboboxInput = React.forwardRef ) } - {...mergeStyleProps(themeProps('combobox-input', { size, variant }), className, style)} - {...rest} + {...mergeStyleProps(themeProps('combobox-input', { size, variant }), stylex.props(xstyle), rest)} /> ); }); @@ -96,7 +94,7 @@ export interface ComboboxPopupProps extends MosaicComponentProps<'div'> { /** Floating listbox surface. Portal and positioning are handled internally. */ export const ComboboxPopup = React.forwardRef(function MosaicComboboxPopup( - { anchor, portalRoot, className, style, children, ...rest }, + { anchor, portalRoot, xstyle, children, ...rest }, ref, ) { const context = React.useContext(ComboboxAnchorContext); @@ -110,11 +108,9 @@ export const ComboboxPopup = React.forwardRef
; /** Scrollable listbox used when the combobox is embedded in another surface. */ export const ComboboxList = React.forwardRef(function MosaicComboboxList( - { className, style, ...rest }, + { xstyle, ...rest }, ref, ) { return ( @@ -142,11 +138,9 @@ export const ComboboxList = React.forwardRef( ref={ref} {...mergeStyleProps( themeProps('combobox-list'), - stylex.props(reset.base, scrollAreaRoot, ...scrollAreaViewport(), styles.list), - className, - style, + stylex.props(reset.base, scrollAreaRoot, ...scrollAreaViewport(), styles.list, xstyle), + rest, )} - {...rest} /> ); }); @@ -158,14 +152,13 @@ export interface ComboboxOptionProps extends MosaicComponentProps<'div'> { } export const ComboboxOption = React.forwardRef(function MosaicComboboxOption( - { className, style, ...rest }, + { xstyle, ...rest }, ref, ) { return ( ); }); @@ -175,12 +168,11 @@ export type ComboboxEmptyProps = MosaicComponentProps<'p'>; export type ComboboxOptionIndicatorProps = MosaicComponentProps<'span'>; export const ComboboxOptionIndicator = React.forwardRef( - function MosaicComboboxOptionIndicator({ className, style, children, ...props }, ref) { + function MosaicComboboxOptionIndicator({ xstyle, children, ...rest }, ref) { return ( {children ?? ( (function MosaicComboboxEmpty( - { render, className, style, ...rest }, + { render, xstyle, ...rest }, ref, ) { return useRender({ @@ -202,8 +194,7 @@ export const ComboboxEmpty = React.forwardRef { }); describe('Dialog.Actions', () => { - it('merges consumer className and style', () => { + it('composes consumer xstyle onto the row', () => { + const caller = stylex.create({ actions: { marginBlockStart: '2rem' } }); render( { Discard changes? This address has not been saved. Keep editing @@ -320,10 +321,32 @@ describe('Dialog.Actions', () => { , ); - const actions = screen.getByTestId('actions'); - expect(actions).toHaveClass('cl-dialog-actions'); - expect(actions).toHaveClass('custom'); - expect(actions).toHaveStyle({ marginBlockStart: '2rem' }); + expect(screen.getByTestId('actions')).toHaveClass( + 'cl-dialog-actions', + stylex.props(caller.actions).className ?? '', + ); + }); + + it('merges the className a render source hands the row', () => { + render( + + + Discard changes? + This address has not been saved. + } + data-testid='actions' + > + Keep editing + + + , + ); + + expect(screen.getByTestId('actions')).toHaveClass('cl-dialog-actions', 'from-render'); }); it('renders as another element through render', () => { diff --git a/packages/ui/src/mosaic/components/dialog/dialog.test.tsx b/packages/ui/src/mosaic/components/dialog/dialog.test.tsx index f64848c53b0..5b92bdf8d7b 100644 --- a/packages/ui/src/mosaic/components/dialog/dialog.test.tsx +++ b/packages/ui/src/mosaic/components/dialog/dialog.test.tsx @@ -81,21 +81,28 @@ describe('Mosaic Dialog', () => { expect(document.querySelector('.cl-dialog-viewport')).toHaveAttribute('data-size', 'profile'); }); - it('merges consumer className and style onto the popup', () => { + it('composes consumer xstyle onto the popup', () => { + const caller = stylex.create({ popup: { marginTop: '8px' } }); render( - - Body - + Body + , + ); + + expect(document.querySelector('.cl-dialog-popup')).toHaveClass( + 'cl-dialog-popup', + stylex.props(caller.popup).className ?? '', + ); + }); + + it('merges the className a render source hands the popup', () => { + render( + + }>Body , ); - const popup = document.querySelector('.cl-dialog-popup'); - expect(popup).toHaveClass('cl-dialog-popup', 'my-popup'); - expect(popup).toHaveStyle({ marginTop: '8px' }); + expect(document.querySelector('.cl-dialog-popup')).toHaveClass('cl-dialog-popup', 'from-render'); }); it('closes on Dialog.Close, reporting it through onOpenChange', async () => { diff --git a/packages/ui/src/mosaic/components/dialog/dialog.tsx b/packages/ui/src/mosaic/components/dialog/dialog.tsx index b7d17b9831f..b0099a512d0 100644 --- a/packages/ui/src/mosaic/components/dialog/dialog.tsx +++ b/packages/ui/src/mosaic/components/dialog/dialog.tsx @@ -185,11 +185,14 @@ function Root({ } /** Opens the dialog. Renders a `