-
Notifications
You must be signed in to change notification settings - Fork 470
feat(ui): add headless input variant #9646
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
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,2 @@ | ||
| --- | ||
| --- |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,2 @@ | ||
| export { Input } from './input'; | ||
| export type { InputProps } from './input'; | ||
| export type { InputProps, InputVariant } from './input'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,19 @@ describe('Mosaic Input', () => { | |
| expect(screen.getByRole('textbox', { name: 'Name' })).toHaveAttribute('data-size', size); | ||
| }); | ||
|
|
||
| it('removes field chrome with the headless variant', () => { | ||
| render( | ||
| <Input | ||
| variant='headless' | ||
| aria-label='Search' | ||
| />, | ||
| ); | ||
|
|
||
| const input = screen.getByRole('textbox', { name: 'Search' }); | ||
| expect(input).toHaveClass('cl-input'); | ||
| expect(input).toHaveAttribute('data-variant', 'headless'); | ||
| }); | ||
|
Comment on lines
+27
to
+38
Contributor
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. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Test the headless style behavior, not only the DOM contract. These assertions pass if As per coding guidelines, unit tests are required for all new functionality and must verify edge cases. 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
| it('reflects and forwards the disabled state', () => { | ||
| render( | ||
| <Input | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,13 +9,18 @@ import { reset } from '../../utils/reset.styles'; | |
| import { useOptionalFieldControlProps } from '../field/field.context'; | ||
| import { sizes, styles } from './input.styles'; | ||
|
|
||
| export type InputVariant = 'default' | 'headless'; | ||
|
Contributor
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. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Document the new public Add JSDoc that describes the As per coding guidelines, all public APIs must be documented with JSDoc comments. 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
| export interface InputProps extends Omit<MosaicComponentProps<'input'>, 'size'> { | ||
| size?: 'sm' | 'md' | 'lg'; | ||
| /** Removes field chrome so a parent composition can provide it. @default 'default' */ | ||
| variant?: InputVariant; | ||
| } | ||
|
|
||
| export const Input = React.forwardRef<HTMLInputElement, InputProps>(function MosaicInput( | ||
| { | ||
| size = 'md', | ||
| variant = 'default', | ||
| disabled: disabledProp, | ||
| required: requiredProp, | ||
| render, | ||
|
|
@@ -52,8 +57,15 @@ export const Input = React.forwardRef<HTMLInputElement, InputProps>(function Mos | |
| 'aria-labelledby': fieldProps?.['aria-labelledby'] ?? ariaLabelledBy, | ||
| 'aria-describedby': fieldProps?.['aria-describedby'] ?? ariaDescribedBy, | ||
| ...mergeStyleProps( | ||
| themeProps('input', { size, disabled }), | ||
| stylex.props(reset.base, inputStyles.base, styles.base, sizes[size], disabled && inputStyles.disabled), | ||
| themeProps('input', { size, variant, disabled }), | ||
| stylex.props( | ||
| reset.base, | ||
| styles.base, | ||
| sizes[size], | ||
| variant === 'default' && inputStyles.base, | ||
| variant === 'headless' && styles.headless, | ||
| variant === 'default' && disabled && inputStyles.disabled, | ||
| ), | ||
| className, | ||
| style, | ||
| ), | ||
|
|
||
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.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve a visible focus indicator for headless inputs.
When a standalone headless input receives keyboard focus,
outline: 'none'removes the browser focus indicator. The headless path skipsinputStyles.base, and no replacement focus rule is shown. Remove this declaration or add a:focus-visibleindicator.As per coding guidelines, implement proper focus management for keyboard navigation in React components.
🤖 Prompt for AI Agents
Source: Coding guidelines