IBX-7918: Dropdown Base and Single #48
Conversation
GrabowskiM
force-pushed
the
IBX-7918-dropdown
branch
from
4e35fdc to
a8b401c
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR introduces dropdown components by implementing a base dropdown component and a single-selection dropdown component. It also refactors the Expander component enum values from lowercase to PascalCase.
- Implements BaseDropdown component with search functionality for large item lists
- Creates DropdownSingleInput component for single-value selection
- Refactors ExpanderType enum values to use PascalCase naming convention
Reviewed Changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/storybook/decorators/DropdownDecorator.tsx | Adds Storybook decorator for dropdown component testing with configurable height |
| packages/components/src/partials/BaseDropdown/* | Implements base dropdown functionality with search, positioning, and item filtering |
| packages/components/src/components/InputText/InputTextInput/* | Adds ref support with focus method for programmatic input focus |
| packages/components/src/components/Expander/* | Updates enum values from lowercase to PascalCase (caret→Caret, chevron→Chevron) |
| packages/components/src/components/Dropdown/DropdownSingleInput/* | Implements single-selection dropdown with state management and item selection |
| packages/components/src/components/Accordion/Accordion.tsx | Updates Expander usage to use new PascalCase enum value |
| packages/components/package.json | Adds Popper.js dependencies for dropdown positioning |
| packages/assets/src/scss/inputs/_dropdown.scss | Adds comprehensive dropdown styling with hover, focus, and selection states |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| setIsOpen(true); | ||
| }, | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
The useImperativeHandle dependency array is missing the hasSearchInput variable. This could cause stale closure issues where the returned function references an outdated value.
| }, []); | |
| }, [hasSearchInput]); |
Copilot uses AI. Check for mistakes.
| if (isOpen && referenceElement) { | ||
| setItemsContainerWidth(referenceElement.offsetWidth); | ||
| } | ||
| }, [isOpen, popperElement]); |
There was a problem hiding this comment.
The useLayoutEffect dependency array includes popperElement but the effect only uses referenceElement. The dependency should be referenceElement instead of popperElement.
| }, [isOpen, popperElement]); | |
| }, [isOpen, referenceElement]); |
Copilot uses AI. Check for mistakes.
| const baseDropdownRef = useRef<BaseDropdownRef>(null); | ||
| const dropdownClassName = createCssClassNames({ | ||
| 'ids-dropdown--single': true, | ||
| [className]: true, |
There was a problem hiding this comment.
The className is always applied regardless of whether it's empty or not. This should use [className]: !!className to conditionally apply the className only when it's truthy.
| [className]: true, | |
| [className]: !!className, |
Copilot uses AI. Check for mistakes.
| const selectedItem = value ? items.find((item) => item.id === value) : null; | ||
|
|
||
| if (!selectedItem) { | ||
| return <div className={BASE_DROPDOWN_CLASS.PLACEHOLDER}>Select an item</div>; |
There was a problem hiding this comment.
The placeholder text 'Select an item' is hardcoded and not internationalized, while other parts of the code use the Translator context. This should use a translatable string for consistency.
Copilot uses AI. Check for mistakes.
| import type { Meta, StoryObj } from '@storybook/react'; | ||
|
|
||
| import { BASE_DROPDOWN_CLASS, BaseDropdown } from './'; | ||
| import DropdownDecorator from '../../../../../src/storybook/decorators/DropdownDecorator'; |
There was a problem hiding this comment.
double check if proper import
|
|
||
| const meta: Meta<typeof DropdownSingleInputStateful> = { | ||
| component: DropdownSingleInputStateful, | ||
| // tags: ['!dev'], |
Description:
For QA:
Documentation: