fix(eds-core-react): 🐛 Autocomplete - should start at selected index and preserve scroll position #3996
Conversation
…ed highlighted index and improved scrolling behavior starting at selected index
millus
requested review from
pomfrida,
torleifhalseth and
vnys
as code owners
…es in selected item styling and structure
millus
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR fixes UX issues in the Autocomplete component's single-select mode by preserving scroll position and starting navigation from the previously selected option.
- Adds scroll position preservation when closing and reopening the dropdown
- Implements starting navigation from the last selected index instead of always beginning at index 0
- Refactors state change handlers to support controlled highlighted index behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
Autocomplete.tsx |
Implements scroll position preservation and controlled highlighted index logic with new state variables and updated event handlers |
Autocomplete.test.tsx.snap |
Updates test snapshot to reflect changes in selected state and CSS class assignments |
| setTimeout(() => { | ||
| if (controlledHighlightedIndex === 0) { | ||
| rowVirtualizer.scrollToOffset?.(0) | ||
| } else if (rowVirtualizer.scrollToOffset && lastScrollOffset > 0) { | ||
| rowVirtualizer.scrollToOffset(lastScrollOffset) | ||
| } | ||
| const visibleIndexes = | ||
| rowVirtualizer.getVirtualItems?.().map((v) => v.index) || [] | ||
| if (!visibleIndexes.includes(controlledHighlightedIndex)) { | ||
| rowVirtualizer.scrollToIndex(controlledHighlightedIndex, { | ||
| align: allowSelectAll ? 'center' : 'auto', | ||
| }) | ||
| } | ||
| }, 10) |
There was a problem hiding this comment.
[nitpick] The setTimeout with a 10ms delay appears to be a workaround for timing issues. Consider using a more reliable approach like waiting for the virtual list to be ready or using a callback-based solution to avoid potential race conditions.
Copilot uses AI. Check for mistakes.
…n controlled prop change after copilot review
There was a problem hiding this comment.
Great work on this @millus. 👌 The UX and keyboard navigation feels very smooth now for single select.
While I understand from the issue discussion that multi-select was intentionally deferred, both scroll position preservation and smart start index would benefit multi-select users as well. Consider a user working through a long list who accidentally presses ESC - they should be able to reopen the dropdown at the same scroll position and continue navigation from a relevant item, regardless of selection mode.
For multi-select implementation:
- Scroll preservation: Move the logic outside the
if (!multiple)condition - Smart start index: Could use the last selected item as the starting point
Both changes would create consistent UX across selection modes while leveraging the existing logic you've built.
I'd also suggest adding unit tests in Autocomplete.test.tsx specifically for the scroll position preservation behaviour and keyboard navigation starting from selected items.
2 participants
Description
This PR fixes an issue regarding scroll position and start index when navigating single-select in the Autocomplete component.
Why
Users have expressed that the current behaviour in single-select is breaking with how expected UX for select option should be.
How
Related Issue
Resolves #3430