-
Notifications
You must be signed in to change notification settings - Fork 23
feat: add ControlPresentation primitive #1037
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
7a7c2b4
00bfaef
8d65534
413e0a7
4b77d56
f1fa78f
d1be6d2
fa7ed1a
adcb1e2
4a7281c
3ebe67a
0c96939
de7ca21
0e3714e
915a823
3020859
cfad4ac
14f7734
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import * as React from 'react' | ||
|
|
||
| import { render, screen } from '@testing-library/react' | ||
|
|
||
| import { ControlActionButton } from './control-action-button' | ||
|
|
||
| import styles from './control-presentation.module.css' | ||
|
|
||
| describe('ControlActionButton', () => { | ||
| it('renders the text-label Button branch with compact field styling', () => { | ||
| render(<ControlActionButton>Clear</ControlActionButton>) | ||
|
|
||
| const button = screen.getByRole('button', { name: 'Clear' }) | ||
| expect(button).toHaveClass(styles.controlActionButton!) | ||
| }) | ||
|
|
||
| it('renders the icon-only IconButton branch with compact field styling', () => { | ||
| render(<ControlActionButton icon="x" aria-label="Clear" />) | ||
|
|
||
| const button = screen.getByRole('button', { name: 'Clear' }) | ||
| expect(button).toHaveClass(styles.controlActionButton!) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import * as React from 'react' | ||
|
|
||
| import classNames from 'classnames' | ||
|
|
||
| import { Button, IconButton } from '../button' | ||
|
|
||
| import styles from './control-presentation.module.css' | ||
|
|
||
| import type { ButtonProps, IconButtonProps } from '../button' | ||
|
|
||
| export type ControlActionButtonProps = | ||
| | ({ | ||
| children: NonNullable<ButtonProps['children']> | ||
| icon?: never | ||
| } & Omit<ButtonProps, 'children' | 'variant' | 'size'>) | ||
| | ({ | ||
| icon: IconButtonProps['icon'] | ||
| children?: never | ||
| } & Omit<IconButtonProps, 'children' | 'icon' | 'variant' | 'size'>) | ||
|
|
||
| /** | ||
| * A compact action button intended for `ControlPresentation`'s `endSlot`. Wraps | ||
| * Reactist's `Button` / `IconButton` with a 24×24, 3px-radius variant sized to fit | ||
| * the field chrome alongside a 16px icon glyph. | ||
| */ | ||
| export const ControlActionButton = React.forwardRef<HTMLButtonElement, ControlActionButtonProps>( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] This new exported primitive ships without any dedicated test coverage. Please add tests for both branches of the union (
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| function ControlActionButton({ exceptionallySetClassName, ...props }, ref) { | ||
| const sharedProps = { | ||
| ref, | ||
| variant: 'quaternary' as const, | ||
| exceptionallySetClassName: classNames([ | ||
| styles.controlActionButton, | ||
| exceptionallySetClassName, | ||
| ]), | ||
| } | ||
|
|
||
| return 'children' in props ? ( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P3] The shared props between const sharedProps = {
ref,
variant: 'quaternary' as const,
exceptionallySetClassName: classNames([
styles.controlActionButton,
exceptionallySetClassName,
]),
}
return 'children' in props ? (
<Button {...props} {...sharedProps} />
) : (
<IconButton {...props} {...sharedProps} />
) |
||
| <Button {...props} {...sharedProps} /> | ||
| ) : ( | ||
| <IconButton {...props} {...sharedProps} /> | ||
| ) | ||
| }, | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| :root { | ||
| --reactist-field-height: 32px; | ||
| } | ||
|
|
||
| .container { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] This adds a second source of truth for the input chrome that still exists in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Migrations are not in scope here — we will integrate in a follow-up PR. |
||
| /* sizing */ | ||
| height: var(--reactist-field-height); | ||
|
|
||
| /* slot-to-control gap (only takes effect between rendered children) */ | ||
| gap: 6px; | ||
|
|
||
| /* default outer padding; shrunk on the side(s) where a slot is present */ | ||
| padding-inline: 10px; | ||
| } | ||
|
|
||
| /* Conditional outer padding. When a slot is present on a side, shrink | ||
| * the outer padding on that side; the wrapper's flex `gap` then provides | ||
| * the visual spacing between the slot and the control. */ | ||
| .container:has(.startSlot) { | ||
| padding-left: 6px; | ||
| } | ||
|
|
||
| .container:has(.endSlot) { | ||
| padding-right: 4px; | ||
| } | ||
|
|
||
| .control { | ||
| display: contents; | ||
| } | ||
|
|
||
| /* The wrapped control inherits chrome styling so that native elements | ||
| * (input/select/textarea) render flush with the surrounding wrapper. */ | ||
| .control > * { | ||
| /* layout */ | ||
| flex: 1; | ||
| box-sizing: border-box; | ||
| margin: 0; | ||
| padding: 0; | ||
| width: 100%; | ||
|
|
||
| /* border */ | ||
| border: none; | ||
| outline: none; /* focus state is handled by the wrapper border */ | ||
|
|
||
| /* color */ | ||
| background: transparent; | ||
| color: inherit; | ||
| } | ||
|
|
||
| .slot { | ||
| /* color */ | ||
| color: var(--reactist-field-slot-content); | ||
| } | ||
|
|
||
| /* | ||
| * Compact 24×24 action button variant for use inside `endSlot`. The 3px | ||
| * border-radius and reduced min-width make it fit the field chrome alongside a | ||
| * 16px icon glyph. The tripled class boosts specificity above Button's | ||
| * size-class rules (e.g. `.baseButton.size-normal`) so the height override | ||
| * wins regardless of stylesheet load order. | ||
| */ | ||
| .controlActionButton.controlActionButton.controlActionButton { | ||
| --reactist-btn-height: 24px; | ||
| border-radius: 3px; | ||
| min-width: var(--reactist-btn-height); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2]
ControlActionButtoncreates a second compact field-action implementation alongside the existingTextField/PasswordFieldpath:PasswordFieldstill renders a rawIconButton, andtext-field.module.cssalready shrinks slotted buttons to 24px. That leaves two parallel ways to express the same field affordance, with styling drift already possible. Please migrate the existing field action usage to this wrapper here, or move the 24px field-action treatment into a shared button size/token that both paths reuse.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Migrations are not in scope here — we will integrate in a follow-up PR.