Skip to content

Commit

Permalink
Ensure options with value undefined are selected
Browse files Browse the repository at this point in the history
  • Loading branch information
acelaya committed Aug 21, 2024
1 parent cb89a41 commit 0b32fde
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 16 deletions.
31 changes: 25 additions & 6 deletions src/components/input/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
useContext,
useId,
useLayoutEffect,
useMemo,
useRef,
useState,
} from 'preact/hooks';
Expand Down Expand Up @@ -77,9 +78,23 @@ function SelectOption<T>({
}

const { selectValue, value: currentValue, multiple } = selectContext;
const selected =
!disabled &&
((multiple && currentValue.includes(value)) || currentValue === value);
const selected = useMemo(() => {
if (disabled) {
return false;
}

if (!multiple) {
return currentValue === value;
}

// In multi-select, the option should be marked as selected for values
// which are explicitly part of the array, or for `undefined` values if the
// array is empty
return (
currentValue.includes(value) ||
(currentValue.length === 0 && value === undefined)
);
}, [currentValue, disabled, multiple, value]);

const selectOneValue = useCallback(() => {
const options: SelectValueOptions = { closeListbox: true };
Expand All @@ -96,10 +111,14 @@ function SelectOption<T>({
return;
}

const options: SelectValueOptions = { closeListbox: false };
const options: SelectValueOptions = {
// Close listbox only if selected value is a "clear" option. Clear options
// are those with `undefined` value
closeListbox: value === undefined,
};

// In multi-select, clear selection for nullish values
if (!value) {
// In multi-select, clear selection for `undefined` values
if (value === undefined) {
selectValue([], options);
return;
}
Expand Down
36 changes: 26 additions & 10 deletions src/components/input/test/Select-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,12 @@ describe('Select', () => {
const wrapper = mount(
<Component value={undefined} onChange={sinon.stub()} {...props}>
<Component.Option value={undefined}>
<span data-testid="option-reset">Reset</span>
{({ selected }) => (
<span data-testid="option-reset">
Reset
{selected && <span data-testid="selected-option" />}
</span>
)}
</Component.Option>
{items.map(item => (
<Component.Option
Expand Down Expand Up @@ -136,6 +141,11 @@ describe('Select', () => {
checkbox.getDOMNode().dispatchEvent(new KeyboardEvent('keydown', { key }));
}

const isOptionSelected = (wrapper, id) =>
wrapper
.find(`[data-testid="option-${id}"]`)
.exists('[data-testid="selected-option"]');

it('changes selected value when an option is clicked', () => {
const onChange = sinon.stub();
const wrapper = createComponent({ onChange });
Expand Down Expand Up @@ -186,16 +196,12 @@ describe('Select', () => {

it('marks the right item as selected', () => {
const wrapper = createComponent({ value: items[2] });
const isOptionSelected = id =>
wrapper
.find(`[data-testid="option-${id}"]`)
.exists('[data-testid="selected-option"]');

assert.isFalse(isOptionSelected(1));
assert.isFalse(isOptionSelected(2));
assert.isTrue(isOptionSelected(3));
assert.isFalse(isOptionSelected(4));
assert.isFalse(isOptionSelected(5));
assert.isFalse(isOptionSelected(wrapper, 1));
assert.isFalse(isOptionSelected(wrapper, 2));
assert.isTrue(isOptionSelected(wrapper, 3));
assert.isFalse(isOptionSelected(wrapper, 4));
assert.isFalse(isOptionSelected(wrapper, 5));
});

it('marks the right item as disabled', () => {
Expand Down Expand Up @@ -478,6 +484,16 @@ describe('Select', () => {
});
});

[
{ value: [], isResetSelected: true },
{ value: [items[0], items[2]], isResetSelected: false },
].forEach(({ value, isResetSelected }) => {
it('marks reset option as selected when value is empty', () => {
const wrapper = createComponent({ value }, { Component: MultiSelect });
assert.equal(isOptionSelected(wrapper, 'reset'), isResetSelected);
});
});

['Enter', ' '].forEach(key => {
it(`does not change selected value when ${key} is pressed in an option's checkbox`, () => {
const onChange = sinon.stub();
Expand Down

0 comments on commit 0b32fde

Please sign in to comment.