From 58c310891375b4ea87779d19241c48758910e852 Mon Sep 17 00:00:00 2001 From: Elizabeth Craig Date: Wed, 15 Dec 2021 15:24:50 -0800 Subject: [PATCH] Input: implement primary slot (#20863) --- .../src/stories/InputConverged.stories.tsx | 26 +++---- packages/react-input/Spec.md | 74 +++++++----------- packages/react-input/etc/react-input.api.md | 25 +++--- .../src/components/Input/Input.stories.tsx | 19 +---- .../src/components/Input/Input.test.tsx | 76 ++++++++++++++++++- .../src/components/Input/Input.types.ts | 62 ++++++++++++--- .../Input/__snapshots__/Input.test.tsx.snap | 2 + .../src/components/Input/renderInput.tsx | 3 +- .../src/components/Input/useInput.ts | 62 ++++++++++----- .../src/components/Input/useInputStyles.ts | 2 +- 10 files changed, 230 insertions(+), 121 deletions(-) diff --git a/apps/vr-tests/src/stories/InputConverged.stories.tsx b/apps/vr-tests/src/stories/InputConverged.stories.tsx index c34add42bd7844..b807e822c06019 100644 --- a/apps/vr-tests/src/stories/InputConverged.stories.tsx +++ b/apps/vr-tests/src/stories/InputConverged.stories.tsx @@ -5,8 +5,6 @@ import { Input } from '@fluentui/react-input'; import { Search20Regular, Dismiss20Regular } from '@fluentui/react-icons'; import { TestWrapperDecoratorFixedWidth } from '../utilities/TestWrapperDecorator'; -// TODO move input.* props to root once primary slot helper is integrated - storiesOf('Input Converged', module) .addDecorator(TestWrapperDecoratorFixedWidth) .addDecorator(story => ( @@ -23,22 +21,21 @@ storiesOf('Input Converged', module) {story()} )) - .addStory('Appearance: outline (default)', () => ) + .addStory('Appearance: outline (default)', () => ) .addStory('Appearance: underline', () => ( - + )) .addStory('Appearance: filledDarker', () => ( - + )) .addStory('Appearance: filledLighter', () => ( // filledLighter requires a background to show up (this is colorNeutralBackground3 in web light theme)
- +
)) - .addStory('Disabled', () => ) - // TODO move defaultValue prop to root - .addStory('With value', () => ); + .addStory('Disabled', () => ) + .addStory('With value', () => ); // Non-interactive stories storiesOf('Input Converged', module) @@ -48,21 +45,20 @@ storiesOf('Input Converged', module) {story()} )) - .addStory('Size: small', () => ) - .addStory('Size: large', () => ) + .addStory('Size: small', () => ) + .addStory('Size: large', () => ) .addStory('Inline', () => (

- Some text with {' '} - inline input + Some text with inline input

)) .addStory( 'contentBefore', - () => } input={{ placeholder: 'Placeholder' }} />, + () => } placeholder="Placeholder" />, { includeRtl: true }, ) .addStory( 'contentAfter', - () => } input={{ placeholder: 'Placeholder' }} />, + () => } placeholder="Placeholder" />, { includeRtl: true }, ); diff --git a/packages/react-input/Spec.md b/packages/react-input/Spec.md index 145d97f58fb833..47097dd3a9f124 100644 --- a/packages/react-input/Spec.md +++ b/packages/react-input/Spec.md @@ -67,83 +67,67 @@ In the future we may implement behavior variants, such as a password field with In this component, `input` is the primary slot. See notes under [Structure](#structure). -```ts -export type InputProps = InputCommons & Omit, 'children'>; -``` - ### Main props All native HTML `` props are supported. Since the `input` slot is primary (more on that later), top-level native props except `className` and `style` will go to the input. The top-level `ref` prop also points to the ``. This can be used for things like getting the current value or manipulating focus or selection (instead of explicitly exposing an imperative API). -Most custom props are defined in `InputCommons` since they're shared with `InputState`. +For the full current props, see the types file: +https://github.com/microsoft/fluentui/blob/master/packages/react-input/src/components/Input/Input.types.ts ```ts -export type InputCommons = { - /** - * If true, the field will have inline display, allowing it be used within text content. - * If false (the default), the field will have block display. - */ +// Simplified version of the props (including only summaries of custom props) +type SimplifiedInputProps = { + /** Toggle inline display instead of block */ inline?: boolean; - /** - * Controls the colors and borders of the field. - * @default 'outline' - */ + /** Controls the colors and borders of the field (default `outline`) */ appearance?: 'outline' | 'underline' | 'filledDarker' | 'filledLighter'; - /** - * Size of the input (changes the font size and spacing). - * @default 'medium' - */ + /** Size of the input (default `medium`) */ size?: 'small' | 'medium' | 'large'; + + /** Default value (uncontrolled) */ + defaultValue?: string; + + /** Controlled value */ + value?: string; + + /** Called when the user changes the value */ + onChange?: (ev: React.FormEvent, data: { value: string }) => void; }; ``` -`size` [overlaps with a native prop](https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/size) which sets the width of the field in "number of characters." This isn't ideal, but we're going with it since the native prop isn't very useful in practice, and it was hard to find another reasonable/consistent name for the visual size prop. It's also consistent with the approach used by most other libraries which have a prop for setting the visual size. (If anyone needs the native functionality, we could add an `htmlSize` prop in the future.) +Notes on native prop conflicts/overrides: + +- `size` [overlaps with a native prop](https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/size) which sets the width of the field in "number of characters." This isn't ideal, but we're going with it since the native prop isn't very useful in practice, and it was hard to find another reasonable/consistent name for the visual size prop. It's also consistent with the approach used by most other libraries which have a prop for setting the visual size. (If anyone needs the native functionality, we could add an `htmlSize` prop in the future.) +- `value` and `defaultValue` are defined in `InputHTMLAttributes` (from `@types/react`) as `string | ReadonlyArray | number` since the same props interface is used for all input element types. To reflect actual usage, we override the types to only accept strings. ### Slots Note that the field **does not** include a label, required indicator, description, or error message. -```ts -export type InputSlots = { - /** - * Wrapper element which visually appears to be the input and is used for borders, focus styling, etc. - * (A wrapper is needed to properly position `contentBefore` and `contentAfter` relative to `input`.) - */ - root: IntrinsicShorthandProps<'span'>; - - /** - * The actual `` element. `type="text"` will be automatically applied unless overridden. - * This is the "primary" slot, so top-level native props (except `className` and `style`) and the - * top-level `ref` will go here. - */ - input: IntrinsicShorthandProps<'input'>; - - /** Element before the input text, within the input border */ - contentBefore?: IntrinsicShorthandProps<'span'>; - - /** Element after the input text, within the input border */ - contentAfter?: IntrinsicShorthandProps<'span'>; -}; -``` +An overview of the slots is as follows. For the current slot types and full docs, see the types file: +https://github.com/microsoft/fluentui/blob/master/packages/react-input/src/components/Input/Input.types.ts + +- `root` (`span`): Wrapper which visually appears to be the input (needed to position `contentBefore` and `contentAfter` relative to the actual `input`) +- `input` (`input`, primary slot): The actual text input element +- `contentBefore` (`span`): Element before the input text, within the input border (most often used for icons) +- `contentAfter` (`span`): Element after the input text, within the input border (most often used for icons) ## Structure In this component, `input` is the primary slot. Per the [native element props/primary slot RFC](https://github.com/microsoft/fluentui/blob/master/rfcs/convergence/native-element-props.md), this means that most top-level props will go to `input`, but the top-level `className` and `style` will go to the actual root element. ```tsx -{ - /* Out of top-level native props, only `className` and `style` go here */ -} +// Out of top-level native props, only `className` and `style` go here {/* Primary slot. Top-level native props except `className` and `style` go here. */} -; + ``` Notes on the HTML rendering: diff --git a/packages/react-input/etc/react-input.api.md b/packages/react-input/etc/react-input.api.md index 3dd0aa44630823..c99d409200f3ec 100644 --- a/packages/react-input/etc/react-input.api.md +++ b/packages/react-input/etc/react-input.api.md @@ -16,35 +16,38 @@ export const Input: ForwardRefComponent; // @public (undocumented) export const inputClassName = "fui-Input"; -// @public (undocumented) -export type InputCommons = { +// @public +export type InputOnChangeData = { + value: string; +}; + +// @public +export type InputProps = Omit, 'children' | 'defaultValue' | 'onChange' | 'size' | 'value'> & { + children?: never; size?: 'small' | 'medium' | 'large'; inline?: boolean; appearance?: 'outline' | 'underline' | 'filledDarker' | 'filledLighter'; + defaultValue?: string; + value?: string; + onChange?: (ev: React_2.FormEvent, data: InputOnChangeData) => void; }; -// @public -export type InputProps = InputCommons & Omit, 'children'>; - -// @public -export const inputShorthandProps: (keyof InputSlots)[]; - // @public (undocumented) export type InputSlots = { root: IntrinsicShorthandProps<'span'>; - input: Omit, 'size'>; + input: IntrinsicShorthandProps<'input'>; contentBefore?: IntrinsicShorthandProps<'span'>; contentAfter?: IntrinsicShorthandProps<'span'>; }; // @public -export type InputState = InputCommons & ComponentState; +export type InputState = Required> & ComponentState; // @public export const renderInput: (state: InputState) => JSX.Element; // @public -export const useInput: (props: InputProps, ref: React_2.Ref) => InputState; +export const useInput: (props: InputProps, ref: React_2.Ref) => InputState; // @public export const useInputStyles: (state: InputState) => InputState; diff --git a/packages/react-input/src/components/Input/Input.stories.tsx b/packages/react-input/src/components/Input/Input.stories.tsx index 8934087ac76de4..339c9bb1c4ac38 100644 --- a/packages/react-input/src/components/Input/Input.stories.tsx +++ b/packages/react-input/src/components/Input/Input.stories.tsx @@ -1,8 +1,7 @@ -/// import * as React from 'react'; import { shorthands, makeStyles, mergeClasses } from '@fluentui/react-make-styles'; import { Input } from './Input'; -import { getNativeElementProps, useId } from '@fluentui/react-utilities'; +import { useId } from '@fluentui/react-utilities'; import { InputProps } from './Input.types'; import { ArgTypes } from '@storybook/react'; import { @@ -29,20 +28,10 @@ const icons = { dismiss: { small: Dismiss16Regular, medium: Dismiss20Regular, large: Dismiss24Regular }, }; -export const InputExamples = ( - args: Partial & React.InputHTMLAttributes & { storyFilledBackground: boolean }, -) => { +export const InputExamples = (args: InputProps & { storyFilledBackground: boolean }) => { const styles = useStyles(); const inputId1 = useId(); - // pass native input props to the internal input element and custom props to the root - const { storyFilledBackground, ...rest } = args; - const inputProps = getNativeElementProps('input', rest, ['size']); - const props: Partial = { input: inputProps }; - for (const prop of Object.keys(rest) as (keyof InputProps)[]) { - if (!(inputProps as Partial)[prop]) { - props[prop] = rest[prop]; - } - } + const { storyFilledBackground, ...props } = args; const SearchIcon = icons.search[props.size!]; const DismissIcon = icons.dismiss[props.size!]; @@ -90,8 +79,6 @@ const argTypes: ArgTypes = { }, // this one is for the example storyFilledBackground: { defaultValue: false, control: { type: 'boolean' } }, - // NOTE: these are not actually top-level props right now until RFC is resolved, - // so they get passed through in the example via the input slot placeholder: { defaultValue: 'placeholder', control: { type: 'text' } }, value: { control: { type: 'text' } }, disabled: { defaultValue: false, control: { type: 'boolean' } }, diff --git a/packages/react-input/src/components/Input/Input.test.tsx b/packages/react-input/src/components/Input/Input.test.tsx index 33e010de6af8c5..b4781fae647275 100644 --- a/packages/react-input/src/components/Input/Input.test.tsx +++ b/packages/react-input/src/components/Input/Input.test.tsx @@ -1,18 +1,88 @@ import * as React from 'react'; -import { render } from '@testing-library/react'; +import { render, RenderResult, fireEvent, screen } from '@testing-library/react'; import { Input } from './Input'; import { isConformant } from '../../common/isConformant'; +function getInput(): HTMLInputElement { + return screen.getByRole('textbox') as HTMLInputElement; +} + describe('Input', () => { + let renderedComponent: RenderResult | undefined; + + afterEach(() => { + if (renderedComponent) { + renderedComponent.unmount(); + renderedComponent = undefined; + } + }); + isConformant({ Component: Input, displayName: 'Input', + primarySlot: 'input', }); - // TODO add more tests here, and create visual regression tests in /apps/vr-tests - it('renders a default state', () => { const result = render(); expect(result.container).toMatchSnapshot(); }); + + it('respects value', () => { + renderedComponent = render(); + expect(getInput().value).toEqual('hello'); + }); + + it('respects updates to value', () => { + renderedComponent = render(); + expect(getInput().value).toEqual('hello'); + + renderedComponent.rerender(); + expect(getInput().value).toEqual('world'); + }); + + it('respects defaultValue', () => { + renderedComponent = render(); + expect(getInput().value).toEqual('hello'); + }); + + it('ignores updates to defaultValue', () => { + renderedComponent = render(); + expect(getInput().value).toEqual('hello'); + + renderedComponent.rerender(); + expect(getInput().value).toEqual('hello'); + }); + + it('prefers value over defaultValue', () => { + renderedComponent = render(); + expect(getInput().value).toEqual('hello'); + }); + + it('with value, calls onChange but does not update on text entry', () => { + const onChange = jest.fn(); + renderedComponent = render(); + const input = getInput(); + fireEvent.change(input, { target: { value: 'world' } }); + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange.mock.calls[0][1]).toEqual({ value: 'world' }); + expect(input.value).toBe('hello'); + }); + + it('with defaultValue, calls onChange and updates value on text entry', () => { + const onChange = jest.fn(); + renderedComponent = render(); + const input = getInput(); + fireEvent.change(input, { target: { value: 'world' } }); + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange.mock.calls[0][1]).toEqual({ value: 'world' }); + expect(input.value).toBe('world'); + }); + + it('does not call onChange when value prop updates', () => { + const onChange = jest.fn(); + renderedComponent = render(); + renderedComponent.rerender(); + expect(onChange).toHaveBeenCalledTimes(0); + }); }); diff --git a/packages/react-input/src/components/Input/Input.types.ts b/packages/react-input/src/components/Input/Input.types.ts index a935d2f3353967..d45012dd36d486 100644 --- a/packages/react-input/src/components/Input/Input.types.ts +++ b/packages/react-input/src/components/Input/Input.types.ts @@ -1,18 +1,24 @@ +import * as React from 'react'; import type { ComponentProps, ComponentState, IntrinsicShorthandProps } from '@fluentui/react-utilities'; export type InputSlots = { /** * Wrapper element which visually appears to be the input and is used for borders, focus styling, etc. * (A wrapper is needed to properly position `contentBefore` and `contentAfter` relative to `input`.) + * + * The root slot receives the `className` and `style` specified directly on the ``. + * All other top-level native props will be applied to the primary slot, `input`. */ root: IntrinsicShorthandProps<'span'>; /** * The actual `` element. `type="text"` will be automatically applied unless overridden. - * This is the "primary" slot, so top-level native props (except `className` and `style`) and the - * top-level `ref` will go here. + * + * This is the "primary" slot, so native props specified directly on the `` will go here + * (except `className` and `style`, which go to the `root` slot). The top-level `ref` will + * also go here. */ - input: Omit, 'size'>; + input: IntrinsicShorthandProps<'input'>; /** Element before the input text, within the input border */ contentBefore?: IntrinsicShorthandProps<'span'>; @@ -21,7 +27,17 @@ export type InputSlots = { contentAfter?: IntrinsicShorthandProps<'span'>; }; -export type InputCommons = { +/** + * Input Props + */ +export type InputProps = Omit< + ComponentProps, + // `children` is unsupported. The rest of these native props have customized definitions. + 'children' | 'defaultValue' | 'onChange' | 'size' | 'value' +> & { + /** Input can't have children. */ + children?: never; + /** * Size of the input (changes the font size and spacing). * @default 'medium' @@ -32,24 +48,48 @@ export type InputCommons = { size?: 'small' | 'medium' | 'large'; /** - * If true, the field will have inline display, allowing it be used within text content. - * If false (the default), the field will have block display. + * If true, the input will have inline display, allowing it be used within text content. + * If false (the default), the input will have block display. */ inline?: boolean; /** - * Controls the colors and borders of the field. + * Controls the colors and borders of the input. * @default 'outline' */ appearance?: 'outline' | 'underline' | 'filledDarker' | 'filledLighter'; + + /** + * Default value of the input. Provide this if the input should be an uncontrolled component + * which tracks its current state internally; otherwise, use `value`. + * + * (This prop is mutually exclusive with `value`.) + */ + defaultValue?: string; + + /** + * Current value of the input. Provide this if the input is a controlled component where you + * are maintaining its current state; otherwise, use `defaultValue`. + * + * (This prop is mutually exclusive with `defaultValue`.) + */ + value?: string; + + /** + * Called when the user changes the input's value. + */ + onChange?: (ev: React.FormEvent, data: InputOnChangeData) => void; }; /** - * Input Props + * State used in rendering Input */ -export type InputProps = InputCommons & Omit, 'children'>; +export type InputState = Required> & ComponentState; /** - * State used in rendering Input + * Data passed to the `onChange` callback when a user changes the input's value. */ -export type InputState = InputCommons & ComponentState; +export type InputOnChangeData = { + /** Updated input value from the user */ + value: string; +}; diff --git a/packages/react-input/src/components/Input/__snapshots__/Input.test.tsx.snap b/packages/react-input/src/components/Input/__snapshots__/Input.test.tsx.snap index 7c59ceb12a8887..401b8c53494544 100644 --- a/packages/react-input/src/components/Input/__snapshots__/Input.test.tsx.snap +++ b/packages/react-input/src/components/Input/__snapshots__/Input.test.tsx.snap @@ -7,6 +7,8 @@ exports[`Input renders a default state 1`] = ` > diff --git a/packages/react-input/src/components/Input/renderInput.tsx b/packages/react-input/src/components/Input/renderInput.tsx index e8e24be4352a26..0fb857da987eb2 100644 --- a/packages/react-input/src/components/Input/renderInput.tsx +++ b/packages/react-input/src/components/Input/renderInput.tsx @@ -1,13 +1,12 @@ import * as React from 'react'; import { getSlots } from '@fluentui/react-utilities'; -import { inputShorthandProps } from './useInput'; import type { InputSlots, InputState } from './Input.types'; /** * Render the final JSX of Input */ export const renderInput = (state: InputState) => { - const { slots, slotProps } = getSlots(state, inputShorthandProps); + const { slots, slotProps } = getSlots(state, ['input', 'contentBefore', 'contentAfter', 'root']); return ( diff --git a/packages/react-input/src/components/Input/useInput.ts b/packages/react-input/src/components/Input/useInput.ts index bff6a8c8662a60..592db5be248143 100644 --- a/packages/react-input/src/components/Input/useInput.ts +++ b/packages/react-input/src/components/Input/useInput.ts @@ -1,11 +1,11 @@ import * as React from 'react'; -import { getNativeElementProps, resolveShorthand } from '@fluentui/react-utilities'; -import type { InputProps, InputSlots, InputState } from './Input.types'; - -/** - * Array of all shorthand properties listed as the keys of InputSlots - */ -export const inputShorthandProps: (keyof InputSlots)[] = ['input', 'contentBefore', 'contentAfter', 'root']; +import { + getPartitionedNativeProps, + resolveShorthand, + useControllableState, + useEventCallback, +} from '@fluentui/react-utilities'; +import type { InputProps, InputState } from './Input.types'; /** * Create the state required to render Input. @@ -14,12 +14,24 @@ export const inputShorthandProps: (keyof InputSlots)[] = ['input', 'contentBefor * before being passed to renderInput. * * @param props - props from this instance of Input - * @param ref - reference to root HTMLInputElement of Input + * @param ref - reference to `` element of Input */ -export const useInput = (props: InputProps, ref: React.Ref): InputState => { - const { input, contentAfter, contentBefore, size, appearance, inline } = props; +export const useInput = (props: InputProps, ref: React.Ref): InputState => { + const { size = 'medium', appearance = 'outline', inline = false, onChange } = props; + + const [value, setValue] = useControllableState({ + state: props.value, + defaultState: props.defaultValue, + initialState: undefined, + }); - return { + const nativeProps = getPartitionedNativeProps({ + props, + primarySlotTagName: 'input', + excludedPropNames: ['size', 'onChange', 'value', 'defaultValue'], + }); + + const state: InputState = { size, appearance, inline, @@ -29,12 +41,28 @@ export const useInput = (props: InputProps, ref: React.Ref): InputS contentBefore: 'span', contentAfter: 'span', }, - input: resolveShorthand(input, { required: true }), - contentAfter: resolveShorthand(contentAfter), - contentBefore: resolveShorthand(contentBefore), - root: getNativeElementProps('span', { - ref, - ...props, + input: resolveShorthand(props.input, { + required: true, + defaultProps: { + type: 'text', + ref, + ...nativeProps.primary, + }, + }), + contentAfter: resolveShorthand(props.contentAfter), + contentBefore: resolveShorthand(props.contentBefore), + root: resolveShorthand(props.root, { + required: true, + defaultProps: nativeProps.root, }), }; + + state.input.value = value; + state.input.onChange = useEventCallback(ev => { + const newValue = ev.target.value; + onChange?.(ev, { value: newValue }); + setValue(newValue); + }); + + return state; }; diff --git a/packages/react-input/src/components/Input/useInputStyles.ts b/packages/react-input/src/components/Input/useInputStyles.ts index 7ecdb19820fa47..7a41401f10f143 100644 --- a/packages/react-input/src/components/Input/useInputStyles.ts +++ b/packages/react-input/src/components/Input/useInputStyles.ts @@ -221,7 +221,7 @@ const useContentStyles = makeStyles({ * Apply styling to the Input slots based on the state */ export const useInputStyles = (state: InputState): InputState => { - const { size = 'medium', appearance = 'outline' } = state; + const { size, appearance } = state; const disabled = state.input.disabled; const filled = appearance.startsWith('filled');