Skip to content

Commit c9c005e

Browse files
committed
refactor: Read Select group state from selection model
Select now passes singleSelection(selectedOptions) to useSelect, so group state for both Select and Multiselect comes from the selection models. useSelect's own group-state formula (getOptionGroupState and its values set) is removed. The selection prop stays optional and defaults to singleSelection(selectedOptions), because use-select.test.ts calls the hook without it. The default is the same model Select passes, so there is one source of group state. Behaviour is unchanged: existing tests pass unmodified, and the new group-state tests pass on both the old and new code.
1 parent 1267692 commit c9c005e

3 files changed

Lines changed: 142 additions & 26 deletions

File tree

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
import React, { createRef } from 'react';
4+
import { render } from '@testing-library/react';
5+
6+
import Select from '../../../lib/components/select';
7+
import createWrapper from '../../../lib/components/test-utils/dom';
8+
import { renderHook } from '../../__tests__/render-hook';
9+
import { singleSelection } from '../../internal/components/listbox/selection/single-selection';
10+
import { flattenOptions } from '../../internal/components/option/utils/flatten-options';
11+
import { OptionDefinition, OptionGroup } from '../../types/option';
12+
import { SelectProps } from '../interfaces';
13+
import { useSelect } from '../utils/use-select';
14+
15+
import selectableStyles from '../../../lib/components/internal/components/selectable-item/styles.css.js';
16+
17+
const values = (...values: string[]) => values.map(value => ({ value }));
18+
19+
const renderUseSelect = (
20+
options: ReadonlyArray<OptionDefinition | OptionGroup>,
21+
selectedOptions: ReadonlyArray<OptionDefinition>,
22+
withSelection = true
23+
) => {
24+
const { flatOptions } = flattenOptions(options);
25+
const hook = renderHook(useSelect, {
26+
initialProps: {
27+
selectedOptions,
28+
selection: withSelection ? singleSelection(selectedOptions) : undefined,
29+
updateSelectedOption: () => {},
30+
options: flatOptions,
31+
externalRef: createRef(),
32+
filteringType: 'none',
33+
fireLoadItems: () => {},
34+
setFilteringValue: () => {},
35+
statusType: 'finished' as const,
36+
},
37+
});
38+
const { getOptionProps } = hook.result.current;
39+
return { flatOptions, getOptionProps };
40+
};
41+
42+
const groupStates = (
43+
options: ReadonlyArray<OptionDefinition | OptionGroup>,
44+
selectedOptions: ReadonlyArray<OptionDefinition>
45+
) => {
46+
const { flatOptions, getOptionProps } = renderUseSelect(options, selectedOptions);
47+
return flatOptions
48+
.map((option, index) => ({ option, props: getOptionProps(option, index) }))
49+
.filter(({ option }) => option.type === 'parent')
50+
.map(({ props }) => ({ selected: props.selected, indeterminate: props.indeterminate }));
51+
};
52+
53+
describe('useSelect group state with single selection', () => {
54+
test('a group with one of several children selected is indeterminate, not selected', () => {
55+
expect(groupStates([{ label: 'Group', options: values('a', 'b', 'c') }], values('b'))).toEqual([
56+
{ selected: false, indeterminate: true },
57+
]);
58+
});
59+
60+
test('the same value in two groups makes both groups indeterminate', () => {
61+
expect(
62+
groupStates(
63+
[
64+
{ label: 'Group 1', options: values('a', 'shared') },
65+
{ label: 'Group 2', options: values('shared', 'b') },
66+
],
67+
values('shared')
68+
)
69+
).toEqual([
70+
{ selected: false, indeterminate: true },
71+
{ selected: false, indeterminate: true },
72+
]);
73+
});
74+
75+
test('a selected value that is not in the options leaves groups unselected', () => {
76+
expect(groupStates([{ label: 'Group', options: values('a', 'b') }], values('x'))).toEqual([
77+
{ selected: false, indeterminate: false },
78+
]);
79+
});
80+
81+
test('omitting selection gives the same option props as passing single selection', () => {
82+
const options = [{ value: 'top' }, { label: 'Group', options: values('a', 'b', 'c') }];
83+
const selectedOptions = values('b');
84+
const withModel = renderUseSelect(options, selectedOptions, true);
85+
const withoutModel = renderUseSelect(options, selectedOptions, false);
86+
// Ids come from a per-hook unique id, so they're left out of the comparison.
87+
const propsWithoutId = ({ getOptionProps, flatOptions }: ReturnType<typeof renderUseSelect>) =>
88+
flatOptions.map((option, index) => {
89+
const props = { ...getOptionProps(option, index) };
90+
delete props.id;
91+
return props;
92+
});
93+
expect(propsWithoutId(withoutModel)).toEqual(propsWithoutId(withModel));
94+
});
95+
});
96+
97+
describe('Select group row state', () => {
98+
const renderSelect = (props: Partial<SelectProps>) => {
99+
const { container } = render(<Select options={[]} selectedOption={null} onChange={() => null} {...props} />);
100+
return createWrapper(container).findSelect()!;
101+
};
102+
const rowOf = (element: HTMLElement) => element.closest(`.${selectableStyles['selectable-item']}`)!;
103+
104+
test('selecting a child in a multi-child group does not mark the group header as selected', () => {
105+
const wrapper = renderSelect({
106+
options: [{ value: 'top' }, { label: 'Group', options: values('a', 'b', 'c') }],
107+
selectedOption: { value: 'b' },
108+
});
109+
wrapper.openDropdown();
110+
const dropdown = wrapper.findDropdown();
111+
112+
expect(rowOf(dropdown.findGroup(1)!.getElement())).not.toHaveClass(selectableStyles.selected);
113+
expect(rowOf(dropdown.findOptionByValue('top')!.getElement())).not.toHaveClass(
114+
selectableStyles['next-item-selected']
115+
);
116+
expect(rowOf(dropdown.findOptionByValue('a')!.getElement())).not.toHaveClass(
117+
selectableStyles['previous-item-selected']
118+
);
119+
expect(rowOf(dropdown.findOptionByValue('b')!.getElement())).toHaveClass(selectableStyles.selected);
120+
});
121+
122+
test('selecting the only child of a group does not mark the group header as selected', () => {
123+
const wrapper = renderSelect({
124+
options: [{ label: 'Group', options: values('a') }],
125+
selectedOption: { value: 'a' },
126+
});
127+
wrapper.openDropdown();
128+
const dropdown = wrapper.findDropdown();
129+
130+
expect(rowOf(dropdown.findGroup(1)!.getElement())).not.toHaveClass(selectableStyles.selected);
131+
expect(rowOf(dropdown.findOptionByValue('a')!.getElement())).toHaveClass(selectableStyles.selected);
132+
});
133+
});

‎src/select/internal.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { getBaseProps } from '../internal/base-component';
1111
import { getBreakpointValue } from '../internal/breakpoints';
1212
import DropdownFooter from '../internal/components/dropdown-footer';
1313
import { useDropdownStatus } from '../internal/components/dropdown-status';
14+
import { singleSelection } from '../internal/components/listbox/selection/single-selection';
1415
import { useAnnouncement } from '../internal/components/listbox/use-announcement';
1516
import { checkOptionValueField } from '../internal/components/listbox/utils/check-option-value-field';
1617
import { prepareOptions } from '../internal/components/option/utils/prepare-options.js';
@@ -129,6 +130,7 @@ const InternalSelect = React.forwardRef(
129130
const controlId = formFieldContext.controlId ?? selfControlId;
130131

131132
const scrollToIndex = useRef<SelectListProps.SelectListRef>(null);
133+
const selectedOptions = selectedOption ? [selectedOption] : [];
132134
const {
133135
isOpen,
134136
highlightType,
@@ -145,7 +147,8 @@ const InternalSelect = React.forwardRef(
145147
focusActiveRef,
146148
closeDropdown,
147149
} = useSelect({
148-
selectedOptions: selectedOption ? [selectedOption] : [],
150+
selectedOptions,
151+
selection: singleSelection(selectedOptions),
149152
updateSelectedOption: option => fireNonCancelableEvent(onChange, { selectedOption: option }),
150153
options: filteredOptions,
151154
filteringType,

‎src/select/utils/use-select.ts‎

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { useUniqueId } from '@cloudscape-design/component-toolkit/internal';
88
import { InternalDropdownProps } from '../../dropdown/internal';
99
import { ButtonTriggerProps } from '../../internal/components/button-trigger';
1010
import { GroupState, SelectionModel } from '../../internal/components/listbox/selection/interfaces';
11+
import { singleSelection } from '../../internal/components/listbox/selection/single-selection';
1112
import { connectOptionsByValue } from '../../internal/components/listbox/utils/connect-options';
1213
import { DropdownOption } from '../../internal/components/option/interfaces';
1314
import { isGroup, isGroupInteractive, isInteractive } from '../../internal/components/option/utils/filter-options';
@@ -21,7 +22,7 @@ import useForwardFocus from '../../internal/hooks/forward-focus';
2122
import { usePrevious } from '../../internal/hooks/use-previous';
2223
import { DropdownStatusProps } from '../../types/dropdown-status';
2324
import { NonCancelableEventHandler } from '../../types/events';
24-
import { OptionDefinition, OptionGroup } from '../../types/option';
25+
import { OptionDefinition } from '../../types/option';
2526
import { SelectProps } from '../interfaces';
2627
import { FilterProps } from '../parts/filter';
2728
import { ItemProps } from '../parts/item';
@@ -43,7 +44,7 @@ interface UseSelectProps {
4344
setFilteringValue?: (filteringText: string) => void;
4445
useInteractiveGroups?: boolean;
4546
statusType: DropdownStatusProps.StatusType;
46-
/** When provided, group and select-all checkbox state come from it. Select doesn't pass one. */
47+
/** Source of group and select-all state. Defaults to single selection over `selectedOptions`. */
4748
selection?: SelectionModel;
4849
toggleAll?: () => void;
4950
dropdownRole?: SelectProps.DropdownRole;
@@ -67,7 +68,7 @@ export function useSelect({
6768
setFilteringValue,
6869
useInteractiveGroups = false,
6970
statusType,
70-
selection,
71+
selection = singleSelection(selectedOptions),
7172
toggleAll,
7273
dropdownRole,
7374
}: UseSelectProps) {
@@ -84,12 +85,6 @@ export function useSelect({
8485
const activeRef = hasFilter ? filterRef : menuRef;
8586
const __selectedOptions = connectOptionsByValue(options, selectedOptions);
8687
const __selectedOptionsSet = new Set(__selectedOptions);
87-
const __selectedValuesSet = selectedOptions.reduce((selectedValuesSet: Set<string>, item: OptionDefinition) => {
88-
if (item.value) {
89-
selectedValuesSet.add(item.value);
90-
}
91-
return selectedValuesSet;
92-
}, new Set<string>());
9388
const [
9489
{ highlightType, highlightedOption, highlightedIndex },
9590
{
@@ -271,23 +266,8 @@ export function useSelect({
271266
}
272267
return menuProps;
273268
};
274-
const getOptionGroupState = (group: OptionGroup): GroupState => {
275-
const totalSelected = group.options.filter(item => !!item.value && __selectedValuesSet.has(item.value)).length;
276-
const hasSelected = totalSelected > 0;
277-
const allSelected = totalSelected === group.options.length;
278-
return {
279-
selected: hasSelected && allSelected && useInteractiveGroups,
280-
indeterminate: hasSelected && !allSelected,
281-
};
282-
};
283-
284269
// Only called for group rows and select-all.
285-
const getGroupState = (option: DropdownOption): GroupState => {
286-
if (selection) {
287-
return selection.getGroupState(option);
288-
}
289-
return isGroup(option.option) ? getOptionGroupState(option.option) : { selected: false, indeterminate: false };
290-
};
270+
const getGroupState = (option: DropdownOption): GroupState => selection.getGroupState(option);
291271

292272
const getOptionProps = (option: DropdownOption, index: number) => {
293273
const isSelectAll = option.type === 'select-all';

0 commit comments

Comments
 (0)