-
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 7 commits
861ded0
345d57d
35cbcf1
9757c41
66df893
fc5050d
fd3b6ea
7b9c4e2
957bcc0
b6842d3
d06b923
2682265
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 |
|---|---|---|
| @@ -1,30 +1,30 @@ | ||
| { | ||
| "recordVersion": 1, | ||
| "react-compiler-version": "1.0.0", | ||
| "files": { | ||
| "src/checkbox-field/checkbox-field.tsx": { | ||
| "CompileError": 1 | ||
| }, | ||
| "src/checkbox-field/use-fork-ref.ts": { | ||
| "CompileError": 1 | ||
| }, | ||
| "src/components/keyboard-shortcut/keyboard-shortcut.tsx": { | ||
| "CompileError": 1 | ||
| }, | ||
| "src/hooks/use-previous/use-previous.ts": { | ||
| "CompileError": 1 | ||
| }, | ||
| "src/menu/menu.tsx": { | ||
| "CompileError": 2 | ||
| }, | ||
| "src/tabs/tabs.tsx": { | ||
| "CompileError": 4 | ||
| }, | ||
| "src/tooltip/tooltip.tsx": { | ||
| "CompileError": 1 | ||
| }, | ||
| "src/utils/common-helpers.ts": { | ||
| "CompileError": 2 | ||
| } | ||
| "recordVersion": 1, | ||
| "react-compiler-version": "1.0.0", | ||
| "files": { | ||
| "src/checkbox-field/checkbox-field.tsx": { | ||
| "CompileError": 1 | ||
| }, | ||
| "src/checkbox-field/use-fork-ref.ts": { | ||
| "CompileError": 1 | ||
| }, | ||
| "src/components/keyboard-shortcut/keyboard-shortcut.tsx": { | ||
| "CompileError": 1 | ||
| }, | ||
| "src/hooks/use-previous/use-previous.ts": { | ||
| "CompileError": 1 | ||
| }, | ||
| "src/menu/menu.tsx": { | ||
| "CompileError": 2 | ||
| }, | ||
| "src/tabs/tabs.tsx": { | ||
| "CompileError": 4 | ||
| }, | ||
| "src/tooltip/tooltip.tsx": { | ||
| "CompileError": 1 | ||
| }, | ||
| "src/utils/common-helpers.ts": { | ||
| "CompileError": 2 | ||
| } | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| import * as React from 'react' | ||
|
|
||
| import classNames from 'classnames' | ||
|
|
||
| import { Button, IconButton } from '../button' | ||
|
|
||
| import styles from './control-presentation.module.css' | ||
|
|
||
| import type { ComponentProps } from 'react' | ||
|
|
||
| export type ControlActionButtonProps = | ||
| | ({ | ||
| children: React.ReactElement | ||
| } & Omit<ComponentProps<typeof Button>, 'variant' | 'size'>) | ||
| | ({ | ||
| icon?: React.ReactElement | ||
| } & Omit<ComponentProps<typeof IconButton>, '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]
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 ( |
||
| function ControlActionButton({ exceptionallySetClassName, ...props }, ref) { | ||
| 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 | ||
| ref={ref} | ||
| {...props} | ||
| variant="quaternary" | ||
| exceptionallySetClassName={classNames([ | ||
| styles.controlActionButton, | ||
| exceptionallySetClassName, | ||
| ])} | ||
| /> | ||
| ) : ( | ||
| <IconButton | ||
| ref={ref} | ||
| {...props} | ||
| variant="quaternary" | ||
| exceptionallySetClassName={classNames([ | ||
| styles.controlActionButton, | ||
| exceptionallySetClassName, | ||
| ])} | ||
| /> | ||
| ) | ||
| }, | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| :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 |
||
| /* 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 (see below) */ | ||
| padding-inline: 10px; | ||
|
|
||
| /* border */ | ||
| border: 1px solid var(--reactist-inputs-idle); | ||
|
|
||
| /* color */ | ||
| background: var(--reactist-field-background); | ||
| color: var(--reactist-field-content); | ||
|
|
||
| /* Read-only: matches native :read-only on form controls (scoped to | ||
| * <input> / <textarea> to avoid catching unrelated elements that | ||
| * default to :read-only), the ARIA convention (aria-readonly="true"), | ||
| * and the data-attr convention used by Ariakit/Radix-style components. | ||
| * The :not(='false') exclusion treats aria-readonly="false" / | ||
| * data-readonly="false" as "not read-only" per the ARIA spec. */ | ||
| &:has( | ||
|
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] The project guidelines explicitly state |
||
| input:read-only, | ||
| textarea:read-only, | ||
| [aria-readonly]:not([aria-readonly='false']), | ||
| [data-readonly]:not([data-readonly='false']) | ||
| ) { | ||
| background-color: var(--reactist-field-readonly-background); | ||
| color: var(--reactist-field-readonly-content); | ||
| } | ||
|
|
||
| /* Disabled: matches the native disabled attribute (form controls), | ||
| * the soft-disable ARIA convention (used by Reactist's own Button to | ||
| * keep elements focusable while announcing as disabled), and the | ||
| * data-attr convention used by Ariakit/Radix. */ | ||
| &:has( | ||
|
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] These |
||
| :disabled, | ||
| [aria-disabled]:not([aria-disabled='false']), | ||
| [data-disabled]:not([data-disabled='false']) | ||
| ) { | ||
| background-color: var(--reactist-field-disabled-background); | ||
| color: var(--reactist-field-disabled-content); | ||
| } | ||
|
|
||
| /* Interactive border (hover/focus) — suppressed when the control is | ||
| * in any disabled state (same three conventions as the disabled | ||
| * background-color rule above). */ | ||
| &:not( | ||
| :has( | ||
| :disabled, | ||
| [aria-disabled]:not([aria-disabled='false']), | ||
| [data-disabled]:not([data-disabled='false']) | ||
| ) | ||
| ) { | ||
| &:hover { | ||
| border-color: var(--reactist-inputs-hover); | ||
| } | ||
|
|
||
| &:focus-within { | ||
| border-color: var(--reactist-inputs-focus); | ||
| } | ||
| } | ||
|
|
||
| /* Invalid: matches the ARIA convention only. Native :invalid is | ||
| * deliberately omitted — it fires before user interaction (e.g. on | ||
| * any required-but-empty field at first paint), which is rarely the | ||
| * desired UX. Consumers that want HTML5-validation-driven errors | ||
| * forward :invalid → aria-invalid in their own component. */ | ||
| &:has([aria-invalid]:not([aria-invalid='false'])) { | ||
| border-color: var(--reactist-inputs-alert) !important; | ||
| } | ||
| } | ||
|
|
||
| /* 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; | ||
|
|
||
| & > * { | ||
| /* 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. | ||
| */ | ||
| .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] This wrapper narrows the underlying button APIs more than intended:
Buttonaccepts anyReactNodelabel andIconButtonaccepts its own icon type, but this type only allowsReactElementfor both. That means supported cases like<ControlActionButton>Clear</ControlActionButton>stop type-checking even though the inner component can render them. Reuse the underlying prop types (or widenchildren/icon) so the wrapper doesn't drop valid Button/IconButton inputs.