-
Notifications
You must be signed in to change notification settings - Fork 0
IBX-10842: Fix OverflowList jumps on dynamic items change #74
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.
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes layout instability in the OverflowList component when items change dynamically. The fix introduces a debounced resize handler and wraps overflow items in a fixed-width container to prevent visual jumps during recalculation.
- Added debounced resize handling to prevent rapid recalculations
- Wrapped OverflowList items in a fixed-width container to maintain stable layout during updates
- Integrated OverflowList into DropdownMultiInput to display selected items
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/components/src/hooks/useDebounce.ts | New custom hook for debouncing callbacks with configurable delay |
| packages/components/src/components/OverflowList/OverflowList.tsx | Fixed layout jumps by adding fixed-width wrapper and debounced resize handling |
| packages/components/src/components/Dropdown/DropdownMultiInput/DropdownMultiInput.tsx | Replaced simple text join with OverflowList component for selected items |
| packages/assets/src/scss/inputs/_dropdown.scss | Added flex-grow to selection info to support OverflowList integration |
| packages/assets/src/scss/_overflow-list.scss | Added wrapper styles with overflow hidden and flex display |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| return () => { | ||
| if (timeoutRef.current) { | ||
| clearTimeout(timeoutRef.current); | ||
| } | ||
| }; |
Copilot
AI
Oct 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cleanup function returned from the debounce callback is created on every call but never used by the caller. This creates unnecessary function instances and the cleanup won't be invoked automatically. Consider removing the return statement or documenting that callers should invoke the returned cleanup function.
| return () => { | |
| if (timeoutRef.current) { | |
| clearTimeout(timeoutRef.current); | |
| } | |
| }; | |
| // Removed cleanup function return; cleanup is handled internally. |
| setCurrentAction(Actions.CalculateItems); | ||
| }); | ||
| }), | ||
| [items.length], |
Copilot
AI
Oct 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ResizeObserver dependency array is missing 'debounce'. When the delay changes, a new debounce function is created but the ResizeObserver continues using the old one, which could lead to incorrect debounce timing. Add 'debounce' to the dependency array.
| [items.length], | |
| [items.length, debounce], |
Description:
For QA:
Documentation: