Skip to content

refactor: move internal parts and utils from select to listbox #5204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
amanabiy wants to merge 4 commits into from
Open

refactor: move internal parts and utils from select to listbox #5204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
23eae75
refactor: Move Select type-ahead hook to listbox
amanabiy Oct 29, 2026
586bfed
refactor: Share one option item between Select and Multiselect
amanabiy Oct 29, 2026
ccd5025
refactor: Move PlainList to listbox
amanabiy Oct 29, 2026
6c48dd2
test: Remove duplicate getItemProps test
amanabiy Oct 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pages/dropdown/list-with-sticky-item.page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import React, { useContext, useRef, useState } from 'react';
import Box from '~components/box';
import Button from '~components/button';
import Dropdown from '~components/dropdown';
import PlainList from '~components/internal/components/listbox/parts/plain-list';
import { DropdownOption } from '~components/internal/components/option/interfaces';
import PlainList from '~components/select/parts/plain-list';
import VirtualList from '~components/select/parts/virtual-list';
import SpaceBetween from '~components/space-between';

Expand Down Expand Up @@ -95,7 +95,7 @@ export default function MultiselectPage() {
filteringValue={''}
firstOptionSticky={true}
highlightType={{ type: 'mouse', moveFocus: false }}
useInteractiveGroups={true}
selectionType="multi"
/>
}
/>
Expand Down
11 changes: 5 additions & 6 deletions pages/select/item.permutations.page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@
// SPDX-License-Identifier: Apache-2.0
import React from 'react';

import Item, { ItemProps } from '~components/internal/components/listbox/parts/item';
import { DropdownOption } from '~components/internal/components/option/interfaces';
import Item from '~components/select/parts/item';
import { ItemProps } from '~components/select/parts/item';

import createPermutations from '../utils/permutations';
import PermutationsView from '../utils/permutations-view';
Expand Down Expand Up @@ -100,7 +99,7 @@ const permutations = createPermutations<Omit<ItemProps, 'index'>>([
highlighted: [false, true],
selected: [false, true],
highlightType: ['keyboard'],
hasCheckbox: [true],
selectionType: ['multi'],
},
{
option: [options.complexOption],
Expand All @@ -126,21 +125,21 @@ const permutations = createPermutations<Omit<ItemProps, 'index'>>([
},
{
option: [options.complexDisabledOptionWithCustomSvg],
hasCheckbox: [true],
selectionType: ['multi'],
},
{
option: [options.simpleOption, options.complexOption],
highlighted: [false, true],
highlightType: ['keyboard'],
selected: [true],
hasCheckbox: [false],
selectionType: ['single'],
},
{
option: [options.complexOptionGroup],
highlighted: [false],
highlightType: ['keyboard'],
selected: [true],
hasCheckbox: [false, true],
selectionType: ['single', 'multi'],
},
]);

Expand Down
27 changes: 0 additions & 27 deletions src/internal/components/listbox/__tests__/get-item-props.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ const defaultItemProps = {
index: 1,
getOptionProps: getOptionProps(false, false),
filteringValue: 'child',
checkboxes: false,
};

describe('getItemProps', () => {
Expand All @@ -68,29 +67,6 @@ describe('getItemProps', () => {
'data-test-index': 1,
'data-mouse-target': 1,
filteringValue: 'child',
hasCheckbox: false,
highlighted: false,
key: 1,
option: {
option: { label: 'Child 1' },
type: 'child',
},
selected: false,
});
});

test('should return props for the first child in the first group with checkboxes and keyboard', () => {
const props = getItemProps({
...defaultItemProps,
checkboxes: true,
});
expect(props).toEqual({
'data-child-index': 1,
'data-group-index': 1,
'data-test-index': 1,
'data-mouse-target': 1,
filteringValue: 'child',
hasCheckbox: true,
highlighted: false,
key: 1,
option: {
Expand All @@ -112,7 +88,6 @@ describe('getItemProps', () => {
'data-test-index': 1,
'data-mouse-target': 1,
filteringValue: 'child',
hasCheckbox: false,
highlighted: true,
key: 1,
option: {
Expand All @@ -136,7 +111,6 @@ describe('getItemProps', () => {
'data-test-index': 1,
'data-mouse-target': 1,
filteringValue: 'child',
hasCheckbox: false,
highlighted: false,
key: 1,
option: {
Expand All @@ -159,7 +133,6 @@ describe('getItemProps', () => {
'data-mouse-target': 3,
'data-test-index': 3,
filteringValue: 'child',
hasCheckbox: false,
highlighted: false,
key: 3,
option: {
Expand Down
12 changes: 6 additions & 6 deletions ...elect/__tests__/use-native-search.test.ts → .../listbox/__tests__/use-type-ahead.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

import { act, renderHook } from '../../__tests__/render-hook';
import type { DropdownOption } from '../../internal/components/option/interfaces';
import { useNativeSearch } from '../utils/use-native-search';
import { act, renderHook } from '../../../../__tests__/render-hook';
import type { DropdownOption } from '../../option/interfaces';
import { useTypeAhead } from '../use-type-ahead';

const options: DropdownOption[] = [
{ option: { label: 'option 1', value: 'option1' } },
Expand All @@ -19,8 +19,8 @@ const initialProps = {
highlightedOption: null,
};

describe('useNativeSearch', () => {
const hook = renderHook(useNativeSearch, {
describe('useTypeAhead', () => {
const hook = renderHook(useTypeAhead, {
initialProps: initialProps,
});

Expand All @@ -47,7 +47,7 @@ describe('useNativeSearch', () => {
});

test('should not highlight an option when isEnabled is false', () => {
const hook = renderHook(useNativeSearch, {
const hook = renderHook(useTypeAhead, {
initialProps: {
...initialProps,
isEnabled: false,
Expand Down
4 changes: 1 addition & 3 deletions src/internal/components/listbox/parts/get-item-props.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,14 @@ interface ItemProps {
index: number;
getOptionProps: any;
filteringValue: string;
checkboxes: boolean;
}

export const getItemProps = ({ option, index, getOptionProps, filteringValue, checkboxes = false }: ItemProps) => {
export const getItemProps = ({ option, index, getOptionProps, filteringValue }: ItemProps) => {
const optionProps = getOptionProps(option, index);
optionProps.filteringValue = filteringValue;
const { inGroupIndex, groupIndex, throughIndex } = getTestOptionIndexes(option) || {};
return {
...optionProps,
hasCheckbox: checkboxes,

@amanabiy amanabiy Oct 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasCheckbox is not needed anymore because on last PR I introduced selectionType: 'single' | 'mult'. The multi row always draws the checkbox and the single row never does.

['data-group-index']: groupIndex,
['data-child-index']: inGroupIndex,
['data-test-index']: throughIndex,
Expand Down
Loading
Loading