Skip to content

Commit

Permalink
feat(Select): add warning when name is missing from component props
Browse files Browse the repository at this point in the history
  • Loading branch information
booc0mtaco committed Sep 15, 2023
1 parent 5ae15ac commit 51226de
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 13 deletions.
2 changes: 2 additions & 0 deletions src/components/Select/Select.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ function InteractiveExampleUsingChildren(props: Props) {
className={clsx(!compact && 'w-60')}
data-testid="dropdown"
disabled={disabled}
name="interactive-select"
onChange={setSelectedOption}
optionsAlign={optionsAlign}
optionsClassName={optionsClassName}
Expand Down Expand Up @@ -99,6 +100,7 @@ function InteractiveExampleUsingFunctionChildren() {
as="div"
className="w-60"
data-testid="dropdown"
name="interactive-with-children"
onChange={setSelectedOption}
value={selectedOption}
>
Expand Down
12 changes: 10 additions & 2 deletions src/components/Select/Select.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,11 @@ describe('<Select />', () => {

it('throws an error if children is used without labeling', () => {
const dropdownWithChildrenAndLabelText = (
<Select onChange={() => undefined} value={exampleOptions[0]}>
<Select
name="throwing-select"
onChange={() => undefined}
value={exampleOptions[0]}
>
<Select.Button>Select</Select.Button>

<Select.Options>
Expand All @@ -86,7 +90,11 @@ describe('<Select />', () => {

it('does not throw an error if select uses <Select.Label>', () => {
const dropdownWithDropdownLabel = (
<Select onChange={() => undefined} value={exampleOptions[0]}>
<Select
name="non-throwing-select"
onChange={() => undefined}
value={exampleOptions[0]}
>
<Select.Label>Options:</Select.Label>
<Select.Button>Select</Select.Button>

Expand Down
39 changes: 28 additions & 11 deletions src/components/Select/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ type PropsWithRenderProp<RenderPropArg> = {
as?: ElementType;
};

let showNameWarning = true;

type SelectProps = ExtractProps<typeof Listbox> &
PopoverOptions & {
/**
Expand All @@ -41,11 +43,10 @@ type SelectProps = ExtractProps<typeof Listbox> &
*/
className?: string;
/**
* The style of the select.
*
* Compact renders select trigger button that is only as wide as the content.
* Name of the form element, which triggers the generation of a hidden form field.
* @see https://headlessui.com/react/listbox#using-with-html-forms
*/
variant?: VariantType;
name?: string;
/**
* Align select's popover to the left (default) or right of the trigger button's bottom edge.
* @deprecated
Expand All @@ -58,6 +59,12 @@ type SelectProps = ExtractProps<typeof Listbox> &
* include the width property to define the options menu width.
*/
optionsClassName?: string;
/**
* The style of the select.
*
* Compact renders select trigger button that is only as wide as the content.
*/
variant?: VariantType;
};

function childrenHaveLabelComponent(children?: ReactNode): boolean {
Expand Down Expand Up @@ -95,28 +102,38 @@ const SelectContext = React.createContext<SelectContextType>({});
*/
export function Select(props: SelectProps) {
const {
className,
children,
'aria-label': ariaLabel,
// Defaulting to null is required to explicitly state that this component is controlled, and prevents warning from Headless
value = null,
variant,
children,
className,
modifiers = defaultPopoverModifiers,
name,
onFirstUpdate,
optionsAlign,
optionsClassName,
placement = 'bottom-start',
modifiers = defaultPopoverModifiers,
strategy,
onFirstUpdate,
// Defaulting to null is required to explicitly state that this component is controlled, and prevents warning from Headless
value = null,
variant,
...other
} = props;

if (process.env.NODE_ENV !== 'production' && !name && showNameWarning) {
console.warn(
"%c`Select` won't render a form field unless you include a `name` prop.\n\n See https://headlessui.com/react/listbox#using-with-html-forms for more information",
'font-weight: bold',
);
showNameWarning = false;
}

// Translate old optionsAlign to placement values
// TODO: when removing optionsAlign, also remove this
const optionsPlacement: SelectProps['placement'] = optionsAlign
? ({ left: 'bottom-start', right: 'bottom-end' }[
optionsAlign
] as SelectProps['placement'])
: placement;

const [referenceElement, setReferenceElement] = useState(null);
const [popperElement, setPopperElement] = useState(null);
const { styles: popperStyles, attributes: popperAttributes } = usePopper(
Expand Down

0 comments on commit 51226de

Please sign in to comment.