SelectDropdown: cleanup code and migrate CSS to webpack #35086

…r keys Changes `instanceId` to be an instance property assigned during construction. No need to put it in state, as it never changes. Also, don't use `instanceId` to create `key` values. These don't need to be globally unique. Use them only for DOM element IDs that are used to link elements together with ARIA attributes.

We can always read from `this.props`. Even in constructor, because we called `super( props )`.

Add comments about what the method does, and fix a bug where a separator item (specified as `null` in `options`) can potentially crash the `find`.

Removes need for smart `tabIndex` logic: hidden elements are automatically excluded from tab order. `visibility: hidden`, as opposed to `display: none`, still takes the element into account when doing layout. Therefore, the dropdown header will continue to have the width of the widest option in the list.

Replace string ref with a modern one, and create a `focusLink` instance method to provide API for the parent component that wants to set focus. Replace access to internal `itemRef.refs.itemLink` property.

Use an array of refs to focusable items, set them with a callback, move the `refIndex` increment inline to calling expression.

`key` is needed only when passing array of children. In this case, the children are written as JSX and `key` doesn't need to be added on cloning and mapping 1:1.

Only `DropdownItem` needs a ref (it's focusable and the ref is used to move focus on up and down keyboard navigation) and an `onClick` handler. Separator and Label are neither focusable nor clickable.

Clicks on label and separator should not bubble to the parent element and cause the dropdown to close. This patch adds a missing handler to the separator component (label is already OK). Also adds an a11y role to specify that the element is not interactive despite having an `onClick` handler.

Can be done by assinging an instance property instead of full constructor. Also, `getInitialSelectedItem` already handles the case where `options` prop is not present or empty.

Closing the popup when receiving new props doesn't seem to make much sense. And the initial selected value is set only on initial mount and further changes of the prop are ignored. That's the common behavior of initial-ish props on uncontrolled components. For example, native `<input defaultValue="x" />` renders input box with "x" and doesn't change the value on further rerenders with a different prop.

…edText or Icon In the `getSelectedText` and `getSelectedIcon` getters, the currently selected value is always in `this.state.selected`. No need to default to the initial value. Also, use `_.get` instead of `_.result`, as `icon` and `label` are not functions.

…elds

…down._ convention

…tion

Some of them were quite awful, spying on internal method calls or mocking the whole component instance instead of testing on the real component and checking its state and JSDOM rendering.

Otherwise, the options element is clickable although it has `visibility: hidden` and captures clicks on controls that are underneath it.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SelectDropdown: cleanup code and migrate CSS to webpack #35086

SelectDropdown: cleanup code and migrate CSS to webpack #35086

Commits on Aug 16, 2019