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 <[email protected]>
Co-authored-by: daniel-lxs <[email protected]>

Author: 3 people

webview-ui/src/components/chat/ApiConfigSelector.tsx

@@ -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))}
-
-								{/* 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>
+					{/* 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>
+							)}
+
+							{/* 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">

webview-ui/src/components/chat/__tests__/ApiConfigSelector.spec.tsx

@@ -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()
+	})
 })