-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix: center active mode in selector dropdown on open #7883
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,8 @@ export const ModeSelector = ({ | |
| const [open, setOpen] = React.useState(false) | ||
| const [searchValue, setSearchValue] = React.useState("") | ||
| const searchInputRef = React.useRef<HTMLInputElement>(null) | ||
| const selectedItemRef = React.useRef<HTMLDivElement>(null) | ||
| const scrollContainerRef = React.useRef<HTMLDivElement>(null) | ||
| const portalContainer = useRooPortal("roo-portal") | ||
| const { hasOpenedModeSelector, setHasOpenedModeSelector } = useExtensionState() | ||
| const { t } = useAppTranslation() | ||
|
|
@@ -149,10 +151,37 @@ export const ModeSelector = ({ | |
| [trackModeSelectorOpened], | ||
| ) | ||
|
|
||
| // Auto-focus search input when popover opens. | ||
| // Auto-focus search input and scroll to selected item when popover opens. | ||
| React.useEffect(() => { | ||
| if (open && searchInputRef.current) { | ||
| searchInputRef.current.focus() | ||
| if (open) { | ||
| // Focus search input | ||
| if (searchInputRef.current) { | ||
| searchInputRef.current.focus() | ||
| } | ||
|
|
||
| requestAnimationFrame(() => { | ||
| if (selectedItemRef.current && scrollContainerRef.current) { | ||
| const container = scrollContainerRef.current | ||
| const item = selectedItemRef.current | ||
|
|
||
| // Calculate positions | ||
| const containerHeight = container.clientHeight | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider extracting this scroll calculation logic into a utility function like |
||
| const itemTop = item.offsetTop | ||
| const itemHeight = item.offsetHeight | ||
|
|
||
| // Center the item in the container | ||
| const scrollPosition = itemTop - containerHeight / 2 + itemHeight / 2 | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be valuable to add test coverage for this scroll-to-center behavior? I notice the existing tests don't verify that the selected item is actually scrolled into view when the dropdown opens. |
||
|
|
||
| // Ensure we don't scroll past boundaries | ||
| const maxScroll = container.scrollHeight - containerHeight | ||
| const finalScrollPosition = Math.min(Math.max(0, scrollPosition), maxScroll) | ||
|
|
||
| container.scrollTo({ | ||
| top: finalScrollPosition, | ||
| behavior: "instant", | ||
daniel-lxs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }) | ||
| } | ||
| }) | ||
| } | ||
| }, [open]) | ||
|
|
||
|
|
@@ -223,36 +252,40 @@ export const ModeSelector = ({ | |
| )} | ||
|
|
||
| {/* Mode List */} | ||
| <div className="max-h-[300px] overflow-y-auto"> | ||
| <div ref={scrollContainerRef} className="max-h-[300px] overflow-y-auto"> | ||
| {filteredModes.length === 0 && searchValue ? ( | ||
| <div className="py-2 px-3 text-sm text-vscode-foreground/70"> | ||
| {t("chat:modeSelector.noResults")} | ||
| </div> | ||
| ) : ( | ||
| <div className="py-1"> | ||
| {filteredModes.map((mode) => ( | ||
| <div | ||
| key={mode.slug} | ||
| onClick={() => handleSelect(mode.slug)} | ||
| className={cn( | ||
| "px-3 py-1.5 text-sm cursor-pointer flex items-center", | ||
| "hover:bg-vscode-list-hoverBackground", | ||
| mode.slug === value | ||
| ? "bg-vscode-list-activeSelectionBackground text-vscode-list-activeSelectionForeground" | ||
| : "", | ||
| )} | ||
| data-testid="mode-selector-item"> | ||
| <div className="flex-1 min-w-0"> | ||
| <div className="font-bold truncate">{mode.name}</div> | ||
| {mode.description && ( | ||
| <div className="text-xs text-vscode-descriptionForeground truncate"> | ||
| {mode.description} | ||
| </div> | ||
| {filteredModes.map((mode) => { | ||
| const isSelected = mode.slug === value | ||
| return ( | ||
| <div | ||
| key={mode.slug} | ||
| ref={isSelected ? selectedItemRef : null} | ||
| onClick={() => handleSelect(mode.slug)} | ||
| className={cn( | ||
| "px-3 py-1.5 text-sm cursor-pointer flex items-center", | ||
| "hover:bg-vscode-list-hoverBackground", | ||
| isSelected | ||
| ? "bg-vscode-list-activeSelectionBackground text-vscode-list-activeSelectionForeground" | ||
| : "", | ||
| )} | ||
| data-testid="mode-selector-item"> | ||
| <div className="flex-1 min-w-0"> | ||
| <div className="font-bold truncate">{mode.name}</div> | ||
| {mode.description && ( | ||
| <div className="text-xs text-vscode-descriptionForeground truncate"> | ||
| {mode.description} | ||
| </div> | ||
| )} | ||
| </div> | ||
| {isSelected && <Check className="ml-auto size-4 p-0.5" />} | ||
| </div> | ||
| {mode.slug === value && <Check className="ml-auto size-4 p-0.5" />} | ||
| </div> | ||
| ))} | ||
| ) | ||
| })} | ||
| </div> | ||
| )} | ||
| </div> | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.