fix: keep pinned models fixed at top of scrollable list (#8813)

* fix: keep pinned models fixed at top of scrollable list

- Separated pinned and unpinned configs into different containers
- Pinned configs now stay fixed at the top
- Only unpinned configs are scrollable
- Added tests to verify the fixed behavior

Fixes #8812

* fix: resolve sticky header visual artifact in ApiConfigSelector

- Changed sticky header background from bg-vscode-editorWidget-background to bg-vscode-dropdown-background to match popover container
- Moved separator logic into sticky container as conditional bottom border to prevent scroll artifacts
- Updated tests to match new separator structure
- All 21 tests passing

---------

Co-authored-by: Roo Code <roomote@roocode.com>
Co-authored-by: daniel-lxs <ricciodaniel98@gmail.com>
This commit is contained in:
roomote[bot] 2025-11-04 22:01:06 -05:00 committed by GitHub
parent f1caf3f36e
commit 200a7c8aa9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 124 additions and 20 deletions

View file

@ -193,27 +193,31 @@ export const ApiConfigSelector = ({
</div>
)}
{/* Config list */}
<div className="max-h-[300px] overflow-y-auto">
{filteredConfigs.length === 0 && searchValue ? (
<div className="py-2 px-3 text-sm text-vscode-foreground/70">
{t("common:ui.no_results")}
</div>
) : (
<div className="py-1">
{/* Pinned configs */}
{pinnedConfigs.map((config) => renderConfigItem(config, true))}
{/* Config list - single scroll container */}
{filteredConfigs.length === 0 && searchValue ? (
<div className="py-2 px-3 text-sm text-vscode-foreground/70">{t("common:ui.no_results")}</div>
) : (
<div className="max-h-[300px] overflow-y-auto">
{/* Pinned configs - sticky header */}
{pinnedConfigs.length > 0 && (
<div
className={cn(
"sticky top-0 z-10 bg-vscode-dropdown-background py-1",
unpinnedConfigs.length > 0 && "border-b border-vscode-dropdown-foreground/10",
)}
aria-label="Pinned configurations">
{pinnedConfigs.map((config) => renderConfigItem(config, true))}
</div>
)}
{/* Separator between pinned and unpinned */}
{pinnedConfigs.length > 0 && unpinnedConfigs.length > 0 && (
<div className="mx-1 my-1 h-px bg-vscode-dropdown-foreground/10" />
)}
{/* Unpinned configs */}
{unpinnedConfigs.map((config) => renderConfigItem(config, false))}
</div>
)}
</div>
{/* Unpinned configs */}
{unpinnedConfigs.length > 0 && (
<div className="py-1" aria-label="All configurations">
{unpinnedConfigs.map((config) => renderConfigItem(config, false))}
</div>
)}
</div>
)}
{/* Bottom bar with buttons on left and title on right */}
<div className="flex flex-row items-center justify-between px-2 py-2 border-t border-vscode-dropdown-border">

View file

@ -435,4 +435,104 @@ describe("ApiConfigSelector", () => {
// Search value should be maintained
expect(searchInput.value).toBe("Config")
})
test("pinned configs remain fixed at top while unpinned configs scroll", () => {
// Create a list with many configs to test scrolling
const manyConfigs = Array.from({ length: 15 }, (_, i) => ({
id: `config${i + 1}`,
name: `Config ${i + 1}`,
modelId: `model-${i + 1}`,
}))
const props = {
...defaultProps,
listApiConfigMeta: manyConfigs,
pinnedApiConfigs: {
config1: true,
config2: true,
config3: true,
},
}
render(<ApiConfigSelector {...props} />)
const trigger = screen.getByTestId("dropdown-trigger")
fireEvent.click(trigger)
const popoverContent = screen.getByTestId("popover-content")
// Should have a single scroll container with max-h-[300px] and overflow-y-auto
const scrollContainer = popoverContent.querySelector(".max-h-\\[300px\\].overflow-y-auto")
expect(scrollContainer).toBeInTheDocument()
// Check for pinned configs sticky header
const pinnedStickyHeader = scrollContainer?.querySelector(".sticky.top-0.z-10.bg-vscode-dropdown-background")
expect(pinnedStickyHeader).toBeInTheDocument()
expect(pinnedStickyHeader).toHaveAttribute("aria-label", "Pinned configurations")
// Check for Config 1, 2, 3 being visible in the sticky header (pinned)
expect(screen.getAllByText("Config 1").length).toBeGreaterThan(0)
expect(screen.getAllByText("Config 2").length).toBeGreaterThan(0)
expect(screen.getAllByText("Config 3").length).toBeGreaterThan(0)
// Verify pinned container contains the pinned configs
if (pinnedStickyHeader) {
const elements = pinnedStickyHeader.querySelectorAll(".flex-shrink-0")
const pinnedConfigTexts = Array.from(elements)
.map((el) => (el as Element).textContent)
.filter((text) => text?.startsWith("Config"))
expect(pinnedConfigTexts).toContain("Config 1")
expect(pinnedConfigTexts).toContain("Config 2")
expect(pinnedConfigTexts).toContain("Config 3")
}
// Check for unpinned configs section
const unpinnedSection = scrollContainer?.querySelector('[aria-label="All configurations"]')
expect(unpinnedSection).toBeInTheDocument()
// Verify separator exists as border on pinned section when unpinned configs exist
expect(pinnedStickyHeader).toHaveClass("border-b")
})
test("displays all configs in scrollable container when no configs are pinned", () => {
const manyConfigs = Array.from({ length: 10 }, (_, i) => ({
id: `config${i + 1}`,
name: `Config ${i + 1}`,
modelId: `model-${i + 1}`,
}))
const props = {
...defaultProps,
listApiConfigMeta: manyConfigs,
pinnedApiConfigs: {}, // No pinned configs
}
render(<ApiConfigSelector {...props} />)
const trigger = screen.getByTestId("dropdown-trigger")
fireEvent.click(trigger)
const popoverContent = screen.getByTestId("popover-content")
// Should have a single scroll container with max-h-[300px] and overflow-y-auto
const scrollContainer = popoverContent.querySelector(".max-h-\\[300px\\].overflow-y-auto")
expect(scrollContainer).toBeInTheDocument()
// No pinned section should exist when no configs are pinned
const pinnedSection = scrollContainer?.querySelector(".sticky.top-0")
expect(pinnedSection).not.toBeInTheDocument()
// Should have unpinned configs section with all configs
const unpinnedSection = scrollContainer?.querySelector('[aria-label="All configurations"]')
expect(unpinnedSection).toBeInTheDocument()
// All configs should be in the unpinned section
const allConfigRows = unpinnedSection?.querySelectorAll(".group")
expect(allConfigRows?.length).toBe(10)
// No separator should exist when no pinned configs (no sticky header exists)
const stickyHeader = scrollContainer?.querySelector(".sticky.top-0")
expect(stickyHeader).not.toBeInTheDocument()
})
})