Skip to content

Commit

Permalink
Refactor Select documentation to no longer mention SelectNext
Browse files Browse the repository at this point in the history
  • Loading branch information
acelaya committed Jul 17, 2024
1 parent 048942b commit c1aba0c
Show file tree
Hide file tree
Showing 17 changed files with 187 additions and 260 deletions.
2 changes: 1 addition & 1 deletion docs/examples.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@ You can then reference this file from the web-based pattern library, passing the
There are some considerations and limitations when working with example files.

- They all need to have the `tsx` extension.
- Nested directories are not supported, so it's a good idea to add contextual prefixes to example files. For example, all files related with `SelectNext` can be prefixed with `select-next` to make sure they are all grouped together.
- Nested directories are not supported, so it's a good idea to add contextual prefixes to example files. For example, all files related with `Select` can be prefixed with `select` to make sure they are all grouped together.
- Example files need to have a Preact component as their default export.
- When generating the source code snippet, import statements are stripped out, to avoid internal module references which are not relevant for the library consumers.
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ function SelectOption<T>({
}: SelectOptionProps<T>) {
const selectContext = useContext(SelectContext);
if (!selectContext) {
throw new Error('Select.Option can only be used as Select child');
throw new Error(
'Select.Option can only be used as Select or MultiSelect child',
);
}

const { selectValue, value: currentValue, multiple } = selectContext;
Expand Down Expand Up @@ -134,7 +136,7 @@ function SelectOption<T>({
);
}

SelectOption.displayName = 'SelectNext.Option';
SelectOption.displayName = 'Select.Option';

/** Small space to apply between the toggle button and the listbox */
const LISTBOX_TOGGLE_GAP = '.25rem';
Expand Down Expand Up @@ -299,6 +301,9 @@ export type SelectProps<T> = BaseSelectProps & SingleValueProps<T>;

export type MultiSelectProps<T> = BaseSelectProps & MultiValueProps<T>;

/**
* @deprecated Use `Select` or `MultiSelect` components instead
*/
export type SelectNextProps<T> = (SelectProps<T> | MultiSelectProps<T>) & {
/**
* Whether this select should allow multi-selection or not.
Expand Down Expand Up @@ -479,6 +484,9 @@ function SelectMain<T>({
);
}

/**
* @deprecated Use `Select` or `MultiSelect` components instead
*/
export const SelectNext = Object.assign(SelectMain, {
Option: SelectOption,
displayName: 'SelectNext',
Expand All @@ -489,7 +497,7 @@ export const Select = Object.assign(
// Calling the function directly instead of returning a JSX element, to
// avoid an unnecessary extra layer in the component tree
// eslint-disable-next-line new-cap
return SelectNext({ ...props, multiple: false });
return SelectMain({ ...props, multiple: false });
},
{ Option: SelectOption, displayName: 'Select' },
);
Expand All @@ -499,7 +507,7 @@ export const MultiSelect = Object.assign(
// Calling the function directly instead of returning a JSX element, to
// avoid an unnecessary extra layer in the component tree
// eslint-disable-next-line new-cap
return SelectNext({ ...props, multiple: true });
return SelectMain({ ...props, multiple: true });
},
{ Option: SelectOption, displayName: 'MultiSelect' },
);
8 changes: 2 additions & 6 deletions src/components/input/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export { default as IconButton } from './IconButton';
export { default as Input } from './Input';
export { default as InputGroup } from './InputGroup';
export { default as OptionButton } from './OptionButton';
export { SelectNext, Select, MultiSelect } from './SelectNext';
export { SelectNext, Select, MultiSelect } from './Select';
export { default as Textarea } from './Textarea';

export type { ButtonProps } from './Button';
Expand All @@ -15,9 +15,5 @@ export type { IconButtonProps } from './IconButton';
export type { InputProps } from './Input';
export type { InputGroupProps } from './InputGroup';
export type { OptionButtonProps } from './OptionButton';
export type {
MultiSelectProps,
SelectNextProps,
SelectProps,
} from './SelectNext';
export type { MultiSelectProps, SelectNextProps, SelectProps } from './Select';
export type { TextareaProps } from './Textarea';
126 changes: 55 additions & 71 deletions src/components/input/test/SelectNext-test.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { checkAccessibility, waitFor } from '@hypothesis/frontend-testing';
import { mount } from 'enzyme';

import { MultiSelect, Select, SelectNext } from '../SelectNext';
import { MultiSelect, Select, SelectNext } from '../Select';

describe('SelectNext', () => {
describe('Select', () => {
let wrappers;
const items = [
{ id: '1', name: 'All students' },
Expand All @@ -18,17 +18,17 @@ describe('SelectNext', () => {
* @param {number} [options.paddingTop] - Extra padding top for the container.
* Defaults to 0.
* @param {boolean} [options.optionsChildrenAsCallback] -
* Whether to renders SelectNext.Option children with callback notation.
* Whether to render Select.Option children with callback notation.
* Used primarily to test and cover both branches.
* Defaults to true.
* @param {MultiSelect | Select | SelectNext} [options.Component] -
* The actual "select" component to use. Defaults to `SelectNext`.
* The actual "select" component to use. Defaults to `Select`.
*/
const createComponent = (props = {}, options = {}) => {
const {
paddingTop = 0,
optionsChildrenAsCallback = true,
Component = SelectNext,
Component = Select,
} = options;
const container = document.createElement('div');
container.style.paddingTop = `${paddingTop}px`;
Expand Down Expand Up @@ -99,29 +99,18 @@ describe('SelectNext', () => {
const clickOption = (wrapper, id) =>
wrapper.find(`[data-testid="option-${id}"]`).simulate('click');

[
{
title: 'changes selected value when an option is clicked',
},
{
title:
'changes selected value when an option is clicked, via Select alias',
options: { Component: Select },
},
].forEach(({ title, options = {} }) => {
it(title, () => {
const onChange = sinon.stub();
const wrapper = createComponent({ onChange }, options);
it('changes selected value when an option is clicked', () => {
const onChange = sinon.stub();
const wrapper = createComponent({ onChange });

clickOption(wrapper, 3);
assert.calledWith(onChange.lastCall, items[2]);
clickOption(wrapper, 3);
assert.calledWith(onChange.lastCall, items[2]);

clickOption(wrapper, 5);
assert.calledWith(onChange.lastCall, items[4]);
clickOption(wrapper, 5);
assert.calledWith(onChange.lastCall, items[4]);

clickOption(wrapper, 1);
assert.calledWith(onChange.lastCall, items[0]);
});
clickOption(wrapper, 1);
assert.calledWith(onChange.lastCall, items[0]);
});

it('does not change selected value when a disabled option is clicked', () => {
Expand Down Expand Up @@ -332,12 +321,11 @@ describe('SelectNext', () => {
});
});

context('when Option is rendered outside of SelectNext', () => {
context('when Option is rendered outside of Select', () => {
it('throws an error', () => {
assert.throws(
() =>
mount(<SelectNext.Option value="1">{() => '1'}</SelectNext.Option>),
'Select.Option can only be used as Select child',
() => mount(<Select.Option value="1">{() => '1'}</Select.Option>),
'Select.Option can only be used as Select or MultiSelect child',
);
});
});
Expand Down Expand Up @@ -406,16 +394,22 @@ describe('SelectNext', () => {
});
});

context('when multi-selection is enabled', () => {
context('SelectNext', () => {
// This path can only be tested via SelectNext
it('throws if multiple is true and the value is not an array', async () => {
assert.throws(
() => createComponent({ multiple: true }),
() => createComponent({ multiple: true }, { Component: SelectNext }),
'When `multiple` is true, the value must be an array',
);
});
});

context('MultiSelect', () => {
it('keeps listbox open when an option is selected if multiple is true', async () => {
const wrapper = createComponent({ multiple: true, value: [] });
const wrapper = createComponent(
{ value: [] },
{ Component: MultiSelect },
);

toggleListbox(wrapper);
assert.isFalse(isListboxClosed(wrapper));
Expand All @@ -426,43 +420,32 @@ describe('SelectNext', () => {
assert.isFalse(isListboxClosed(wrapper));
});

[
{
title:
'allows multiple items to be selected when the value is an array',
extraProps: { multiple: true },
},
{
title: 'allows same behavior via MultiSelect alias',
options: { Component: MultiSelect },
},
].forEach(({ title, extraProps = {}, options = {} }) => {
it(title, () => {
const onChange = sinon.stub();
const wrapper = createComponent(
{
value: [items[0], items[2]],
onChange,
...extraProps,
},
options,
);
it('allows multiple items to be selected', () => {
const onChange = sinon.stub();
const wrapper = createComponent(
{
value: [items[0], items[2]],
onChange,
},
{ Component: MultiSelect },
);

toggleListbox(wrapper);
clickOption(wrapper, 2);
toggleListbox(wrapper);
clickOption(wrapper, 2);

// When a not-yet-selected item is clicked, it will be selected
assert.calledWith(onChange, [items[0], items[2], items[1]]);
});
// When a not-yet-selected item is clicked, it will be selected
assert.calledWith(onChange, [items[0], items[2], items[1]]);
});

it('allows deselecting already selected options', () => {
const onChange = sinon.stub();
const wrapper = createComponent({
multiple: true,
value: [items[0], items[2]],
onChange,
});
const wrapper = createComponent(
{
value: [items[0], items[2]],
onChange,
},
{ Component: MultiSelect },
);

toggleListbox(wrapper);
clickOption(wrapper, 3);
Expand All @@ -473,11 +456,13 @@ describe('SelectNext', () => {

it('resets selection when option value is nullish and select value is an array', () => {
const onChange = sinon.stub();
const wrapper = createComponent({
multiple: true,
value: [items[0], items[2]],
onChange,
});
const wrapper = createComponent(
{
value: [items[0], items[2]],
onChange,
},
{ Component: MultiSelect },
);

toggleListbox(wrapper);
wrapper.find(`[data-testid="reset-option"]`).simulate('click');
Expand Down Expand Up @@ -510,16 +495,15 @@ describe('SelectNext', () => {
},
},
{
name: 'Open Multi-Select listbox',
name: 'Open MultiSelect listbox',
content: () => {
const wrapper = createComponent(
{
buttonContent: 'Select',
'aria-label': 'Select',
value: [items[1], items[3]],
multiple: true,
},
{ optionsChildrenAsCallback: false },
{ optionsChildrenAsCallback: false, Component: MultiSelect },
);
toggleListbox(wrapper);

Expand Down
21 changes: 10 additions & 11 deletions src/pattern-library/components/patterns/input/InputGroupPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
CopyIcon,
ArrowLeftIcon,
ArrowRightIcon,
SelectNext,
Select,
} from '../../../../';
import Library from '../../Library';

Expand Down Expand Up @@ -52,7 +52,10 @@ export default function InputGroupPage() {
<code>IconButton</code>
</li>
<li>
<code>SelectNext</code>
<code>Select</code>
</li>
<li>
<code>MultiSelect</code>
</li>
</ul>
<Library.Demo
Expand All @@ -66,15 +69,11 @@ export default function InputGroupPage() {
title="Previous"
variant="dark"
/>
<SelectNext
value="1"
buttonContent="Apple"
onChange={() => {}}
>
<SelectNext.Option value="1">Apple</SelectNext.Option>
<SelectNext.Option value="2">Banana</SelectNext.Option>
<SelectNext.Option value="3">Cherries</SelectNext.Option>
</SelectNext>
<Select value="1" buttonContent="Apple" onChange={() => {}}>
<Select.Option value="1">Apple</Select.Option>
<Select.Option value="2">Banana</Select.Option>
<Select.Option value="3">Cherries</Select.Option>
</Select>
<IconButton
icon={ArrowRightIcon}
title="Next"
Expand Down
Loading

0 comments on commit c1aba0c

Please sign in to comment.