microsoft / fluentui

Fluent UI web represents a collection of utilities, React components, and web components for building web applications.
https://react.fluentui.dev
Other
18.52k stars 2.73k forks source link

Field #19627

Closed ecraig12345 closed 1 year ago

ecraig12345 commented 3 years ago

Preparation

Implementation

Validation

ecraig12345 commented 2 years ago

Apparently I started working on an RFC for this a couple months ago but never got it to PR stage. Here's a preliminary version. It would be good to narrow down the options before making an actual RFC.

RFC: Field approach for @fluentui/react-components

Summary

We need a consistent way to handle label, description, and error message/state across all types of input components (Input, Slider, Checkbox, etc). The approach proposed here is to make a Field component with slots and children.

Background

Other libraries (including v8 and v0) have a variety of patterns for associating labels/etc with inputs. Some common examples (actual component names used vary):

// Each type of input component has props for label/etc
// (possibly with shared implementation)
<TextInput label="some field" description="much detail" required value="whatever" />

// Basic component wrapping the input + ___Field (or similar) variant with label/etc as props/slots
<TextInput value="whatever" />
<TextInputField label="some field" description="much detail" required value="whatever" />

// Wrapper field with components for each child
<FormField>
  <FormFieldLabel>some field</FormFieldLabel>
  <Input value="whatever" />
  <FormFieldHelperText>much detail</FormFieldHelperText>
</FormField>

// Wrapper field with props/slots for label/etc and input as child
<FormField label="some field" description="much detail" required>
  <TextInput value="whatever" />
</FormField>

More detailed list of examples: https://hackmd.io/OulpnA9eTEebdAc_0bVhMQ

Problem statement

We need a consistent way to handle label, description, and error message/state across all types of input components.

Desirable features:

Open Questions

Detailed Design or Proposal

There are multiple options under consideration, listed somewhat in order of the author's preference.

Option A: Field with input as child and slots for other parts

Have a field with the input as a child and slots for other parts. This control would be called Field to clarify that it's not limited to use within a <form>.

(Note: Input in the examples is specifically our text input component. You could substitute any input field component.)

<Field
  className="rootClass"
  label="hello"
  description={<div>fancy hello</div>}
  errorMessage="aka.ms/nohello"
  required
>
  <Input value="whatever" ... />
</Field>

Likely props/slots include:

Option B: Field with slots for input/label/description

<Field
  className="rootClass"
  input={<Input value="whatever" />}
  label="hello"
  description={<div>fancy hello</div>}
  errorMessage="aka.ms/nohello"
  required
/>

This shares similar pros/cons of the first solution but is a bit more awkward API.

Option C: Field with children for input/label/description

Theoretically Field uses an internal context that our form label, error, description, and input/combobox/etc components use to set ids, error state, etc.

<Field id={requiredProp} required validationSomething={something?}>
  <FieldLabel>Email</FieldLabel>
  <Input />
  <FieldDescription>work email</FieldDescription>
  <FieldError>please enter a valid email</FieldError>
</Field>

<Field id={requiredProp2}>
  <FieldLabel>Country</FieldLabel>
  <FieldDescription>the world's only three countries</FieldDescription>
  <Select>
      <Option>USA</Option>
      <Option>UK</Option>
      <Option>India</Option>
  </Select>
</Field>

This might make it easier for people to customize the layout, but it creates challenges with knowing how to automatically apply a11y attributes or do automatic layout.

Probably-discarded solutions

Option D: Field hook

type FieldSlots = {
  input: IntrinsicShorthandProps<'input'>;
  label: IntrinsicShorthandProps<'label'>;
  description: IntrinsicShorthandProps<'span'>;
  errorText: IntrinsicShorthandProps<'span'>;
  required: unknown; // TBD
};

interface FieldProps extends ComponentProps<FieldSlots, 'input'> {
  labelPosition?: 'start' | 'end' | 'top' | 'bottom';
  // note: `size` is a native input prop name
  fieldSize?: 'small' | 'medium' | 'large';
  /** custom override for ID (generated ID used by default) */
  id?: string;
  // and some validation props maybe
}

interface FieldState {
  // stuff
}

const useField = <TProps extends FieldProps, TState extends FieldState>(props: TProps): TState => {
  // return field-related slots and state
};

const useFieldStyles = <TState extends FieldState>(state: TState) => {
  // styles for positioning/spacing of label, field, description, etc
};

// const renderField = <TState extends FieldState>(state: TState) => {
//   const { slots, slotProps } = getSlots<FieldSlots>(state, fieldShorthandProps);

// }

Option E: Input + InputField

Input stays as-is (basic component)

InputField

// some nuances TBD due to primary slot issues
type InputFieldSlots = FieldSlots & InputSlots;

interface InputProps extends InputProps, FieldProps {}

interface InputState extends InputState, FieldState {}

const useInputField = (props: InputProps, ref: React.Ref<HTMLInputElement>): InputState => {};

Option E1: InputField with DOM root as primary

<InputField
  className="rootClass"
  input={{
    // Input props
    className: 'inputClass',
  }}
/>

Option E2: InputField with input element as primary

<InputField
  className="inputClass"
  // how to give the root a class? same problem all over again :(
/>

Option F: Input includes Field features

downside: larger bundles

type InputSlots = FieldSlots & {
  /*more slots*/
};

interface InputProps extends FieldProps {
  /*more props*/
}

interface InputState extends FieldState {
  /*more state*/
}

const useInput = (props: InputProps, ref: React.Ref<HTMLInputElement>): InputState => {};
ecraig12345 commented 2 years ago

Additional very preliminary notes about possible approaches for field groups...

Field Group + Label

The issue: group labels for groups of checkboxes and radios are often treated as if they were the same as field labels for text inputs, comboboxes, sliders, etc.

Group label option 1

We could make the FormField and FieldLabel components able to handle being a single field vs. a group of fields:

<FormField fieldType={FieldType.Group}>
  <FieldLabel>Favorite pet:</FieldLabel>
  <Radio label="Cat" />
  <Radio label="Dog" />
  <Radio label="Tribble" />
</FormField>

simplified output:

<fieldset>
  <legend>Favorite Pet:</legend>
  <label for="radio1">Cat</label>
  <input type="radio" id="radio1" name="group" />

  <label for="radio2">Dog</label>
  <input type="radio" id="radio2" name="group" />

  <label for="radio3">Tribble</label>
  <input type="radio" id="radio3" name="group" />
</fieldset>

Group label option 2

We could provide a more general FieldGroup component that could be used with checkboxes/radios, and could also be used to group other sets of components together (e.g. text fields in an address).

<FormGroup>
  <FormGroupLabel>Address</FormGroupLabel>
  <FormField>
    <FieldLabel>Street Address</FieldLabel>
    <InputField />
  </FormField>
  <FormField>
    <FieldLabel>City</FieldLabel>
    <InputField />
  </FormField>
  <FormField>
    <FieldLabel>Zip code</FieldLabel>
    <InputField />
  </FormField>
</FormGroup>

simplified output:

<fieldset>
  <legend>Address</legend>

  <div class="formfield">
    <label for="id1">Street Address</label>
    <input type="text" id="id1" />
  </div>

  <div class="formfield">
    <label for="id2">City</label>
    <input type="text" id="id2" />
  </div>

  <div class="formfield">
    <label for="id3">Zip code</label>
    <input type="text" id="id3" />
  </div>
</fieldset>
behowell commented 1 year ago

Field released as stable in #27493.