Skip to content

Commit 50c8d9a

Browse files
committed
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
1 parent 97331bc commit 50c8d9a

File tree

2 files changed

+141
-20
lines changed

2 files changed

+141
-20
lines changed

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

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -194,26 +194,37 @@ export const ApiConfigSelector = ({
194194
)}
195195

196196
{/* Config list */}
197-
<div className="max-h-[300px] overflow-y-auto">
198-
{filteredConfigs.length === 0 && searchValue ? (
199-
<div className="py-2 px-3 text-sm text-vscode-foreground/70">
200-
{t("common:ui.no_results")}
201-
</div>
202-
) : (
203-
<div className="py-1">
204-
{/* Pinned configs */}
205-
{pinnedConfigs.map((config) => renderConfigItem(config, true))}
206-
207-
{/* Separator between pinned and unpinned */}
208-
{pinnedConfigs.length > 0 && unpinnedConfigs.length > 0 && (
209-
<div className="mx-1 my-1 h-px bg-vscode-dropdown-foreground/10" />
210-
)}
211-
212-
{/* Unpinned configs */}
213-
{unpinnedConfigs.map((config) => renderConfigItem(config, false))}
214-
</div>
215-
)}
216-
</div>
197+
{filteredConfigs.length === 0 && searchValue ? (
198+
<div className="py-2 px-3 text-sm text-vscode-foreground/70">{t("common:ui.no_results")}</div>
199+
) : (
200+
<>
201+
{/* Pinned configs - fixed at top, not scrollable */}
202+
{pinnedConfigs.length > 0 && (
203+
<div className="py-1">
204+
{pinnedConfigs.map((config) => renderConfigItem(config, true))}
205+
</div>
206+
)}
207+
208+
{/* Separator between pinned and unpinned */}
209+
{pinnedConfigs.length > 0 && unpinnedConfigs.length > 0 && (
210+
<div className="mx-1 my-1 h-px bg-vscode-dropdown-foreground/10" />
211+
)}
212+
213+
{/* Unpinned configs - scrollable */}
214+
{unpinnedConfigs.length > 0 && (
215+
<div
216+
className="overflow-y-auto py-1"
217+
style={{
218+
maxHeight:
219+
pinnedConfigs.length > 0
220+
? `calc(300px - ${pinnedConfigs.length * 36}px)`
221+
: "300px",
222+
}}>
223+
{unpinnedConfigs.map((config) => renderConfigItem(config, false))}
224+
</div>
225+
)}
226+
</>
227+
)}
217228

218229
{/* Bottom bar with buttons on left and title on right */}
219230
<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

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,4 +435,114 @@ describe("ApiConfigSelector", () => {
435435
// Search value should be maintained
436436
expect(searchInput.value).toBe("Config")
437437
})
438+
439+
test("pinned configs remain fixed at top while unpinned configs scroll", () => {
440+
// Create a list with many configs to test scrolling
441+
const manyConfigs = Array.from({ length: 15 }, (_, i) => ({
442+
id: `config${i + 1}`,
443+
name: `Config ${i + 1}`,
444+
modelId: `model-${i + 1}`,
445+
}))
446+
447+
const props = {
448+
...defaultProps,
449+
listApiConfigMeta: manyConfigs,
450+
pinnedApiConfigs: {
451+
config1: true,
452+
config2: true,
453+
config3: true,
454+
},
455+
}
456+
457+
render(<ApiConfigSelector {...props} />)
458+
459+
const trigger = screen.getByTestId("dropdown-trigger")
460+
fireEvent.click(trigger)
461+
462+
const popoverContent = screen.getByTestId("popover-content")
463+
464+
// Check for Config 1, 2, 3 being visible (pinned) - use getAllByText since there might be multiple
465+
expect(screen.getAllByText("Config 1").length).toBeGreaterThan(0)
466+
expect(screen.getAllByText("Config 2").length).toBeGreaterThan(0)
467+
expect(screen.getAllByText("Config 3").length).toBeGreaterThan(0)
468+
469+
// Find all containers with py-1 class
470+
const configContainers = popoverContent.querySelectorAll(".py-1")
471+
472+
// Should have 2 containers: one for pinned (non-scrollable) and one for unpinned (scrollable)
473+
expect(configContainers.length).toBeGreaterThanOrEqual(1)
474+
475+
// Find the non-scrollable container (pinned configs)
476+
let pinnedContainer: Element | null = null
477+
let unpinnedContainer: Element | null = null
478+
479+
configContainers.forEach((container) => {
480+
if (container.classList.contains("overflow-y-auto")) {
481+
unpinnedContainer = container
482+
} else {
483+
pinnedContainer = container
484+
}
485+
})
486+
487+
// Verify pinned container exists and contains the pinned configs
488+
if (pinnedContainer) {
489+
const elements = (pinnedContainer as Element).querySelectorAll(".flex-shrink-0")
490+
const pinnedConfigTexts = Array.from(elements)
491+
.map((el) => (el as Element).textContent)
492+
.filter((text) => text?.startsWith("Config"))
493+
494+
expect(pinnedConfigTexts).toContain("Config 1")
495+
expect(pinnedConfigTexts).toContain("Config 2")
496+
expect(pinnedConfigTexts).toContain("Config 3")
497+
}
498+
499+
// Verify unpinned container exists and is scrollable
500+
expect(unpinnedContainer).toBeInTheDocument()
501+
if (unpinnedContainer) {
502+
expect((unpinnedContainer as Element).classList.contains("overflow-y-auto")).toBe(true)
503+
// Check that the unpinned container has the correct max-height
504+
expect((unpinnedContainer as Element).getAttribute("style")).toContain("max-height")
505+
}
506+
507+
// Verify separator exists between pinned and unpinned
508+
const separator = popoverContent.querySelector(".h-px")
509+
expect(separator).toBeInTheDocument()
510+
})
511+
512+
test("displays all configs in scrollable container when no configs are pinned", () => {
513+
const manyConfigs = Array.from({ length: 10 }, (_, i) => ({
514+
id: `config${i + 1}`,
515+
name: `Config ${i + 1}`,
516+
modelId: `model-${i + 1}`,
517+
}))
518+
519+
const props = {
520+
...defaultProps,
521+
listApiConfigMeta: manyConfigs,
522+
pinnedApiConfigs: {}, // No pinned configs
523+
}
524+
525+
render(<ApiConfigSelector {...props} />)
526+
527+
const trigger = screen.getByTestId("dropdown-trigger")
528+
fireEvent.click(trigger)
529+
530+
const popoverContent = screen.getByTestId("popover-content")
531+
532+
// Should have only one scrollable container with all configs
533+
const scrollableContainer = popoverContent.querySelector(".overflow-y-auto.py-1")
534+
expect(scrollableContainer).toBeInTheDocument()
535+
536+
// Check max-height is 300px when no pinned configs
537+
expect(scrollableContainer?.getAttribute("style")).toContain("max-height")
538+
expect(scrollableContainer?.getAttribute("style")).toContain("300px")
539+
540+
// All configs should be in the scrollable container
541+
const allConfigRows = scrollableContainer?.querySelectorAll(".group")
542+
expect(allConfigRows?.length).toBe(10)
543+
544+
// No separator should exist
545+
const separator = popoverContent.querySelector(".h-px.bg-vscode-dropdown-foreground\\/10")
546+
expect(separator).not.toBeInTheDocument()
547+
})
438548
})

0 commit comments

Comments
 (0)