Skip to content

Commit 93c1b6f

Browse files
daniel-lxsdaniel-lxs
committed
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
1 parent 50c8d9a commit 93c1b6f

File tree

2 files changed

+45
-62
lines changed

2 files changed

+45
-62
lines changed

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

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -193,37 +193,30 @@ export const ApiConfigSelector = ({
193193
</div>
194194
)}
195195

196-
{/* Config list */}
196+
{/* Config list - single scroll container */}
197197
{filteredConfigs.length === 0 && searchValue ? (
198198
<div className="py-2 px-3 text-sm text-vscode-foreground/70">{t("common:ui.no_results")}</div>
199199
) : (
200-
<>
201-
{/* Pinned configs - fixed at top, not scrollable */}
200+
<div className="max-h-[300px] overflow-y-auto">
201+
{/* Pinned configs - sticky header */}
202202
{pinnedConfigs.length > 0 && (
203-
<div className="py-1">
203+
<div
204+
className={cn(
205+
"sticky top-0 z-10 bg-vscode-dropdown-background py-1",
206+
unpinnedConfigs.length > 0 && "border-b border-vscode-dropdown-foreground/10",
207+
)}
208+
aria-label="Pinned configurations">
204209
{pinnedConfigs.map((config) => renderConfigItem(config, true))}
205210
</div>
206211
)}
207212

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 */}
213+
{/* Unpinned configs */}
214214
{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-
}}>
215+
<div className="py-1" aria-label="All configurations">
223216
{unpinnedConfigs.map((config) => renderConfigItem(config, false))}
224217
</div>
225218
)}
226-
</>
219+
</div>
227220
)}
228221

229222
{/* Bottom bar with buttons on left and title on right */}

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

Lines changed: 33 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -461,32 +461,23 @@ describe("ApiConfigSelector", () => {
461461

462462
const popoverContent = screen.getByTestId("popover-content")
463463

464-
// Check for Config 1, 2, 3 being visible (pinned) - use getAllByText since there might be multiple
464+
// Should have a single scroll container with max-h-[300px] and overflow-y-auto
465+
const scrollContainer = popoverContent.querySelector(".max-h-\\[300px\\].overflow-y-auto")
466+
expect(scrollContainer).toBeInTheDocument()
467+
468+
// Check for pinned configs sticky header
469+
const pinnedStickyHeader = scrollContainer?.querySelector(".sticky.top-0.z-10.bg-vscode-dropdown-background")
470+
expect(pinnedStickyHeader).toBeInTheDocument()
471+
expect(pinnedStickyHeader).toHaveAttribute("aria-label", "Pinned configurations")
472+
473+
// Check for Config 1, 2, 3 being visible in the sticky header (pinned)
465474
expect(screen.getAllByText("Config 1").length).toBeGreaterThan(0)
466475
expect(screen.getAllByText("Config 2").length).toBeGreaterThan(0)
467476
expect(screen.getAllByText("Config 3").length).toBeGreaterThan(0)
468477

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")
478+
// Verify pinned container contains the pinned configs
479+
if (pinnedStickyHeader) {
480+
const elements = pinnedStickyHeader.querySelectorAll(".flex-shrink-0")
490481
const pinnedConfigTexts = Array.from(elements)
491482
.map((el) => (el as Element).textContent)
492483
.filter((text) => text?.startsWith("Config"))
@@ -496,17 +487,12 @@ describe("ApiConfigSelector", () => {
496487
expect(pinnedConfigTexts).toContain("Config 3")
497488
}
498489

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-
}
490+
// Check for unpinned configs section
491+
const unpinnedSection = scrollContainer?.querySelector('[aria-label="All configurations"]')
492+
expect(unpinnedSection).toBeInTheDocument()
506493

507-
// Verify separator exists between pinned and unpinned
508-
const separator = popoverContent.querySelector(".h-px")
509-
expect(separator).toBeInTheDocument()
494+
// Verify separator exists as border on pinned section when unpinned configs exist
495+
expect(pinnedStickyHeader).toHaveClass("border-b")
510496
})
511497

512498
test("displays all configs in scrollable container when no configs are pinned", () => {
@@ -529,20 +515,24 @@ describe("ApiConfigSelector", () => {
529515

530516
const popoverContent = screen.getByTestId("popover-content")
531517

532-
// Should have only one scrollable container with all configs
533-
const scrollableContainer = popoverContent.querySelector(".overflow-y-auto.py-1")
534-
expect(scrollableContainer).toBeInTheDocument()
518+
// Should have a single scroll container with max-h-[300px] and overflow-y-auto
519+
const scrollContainer = popoverContent.querySelector(".max-h-\\[300px\\].overflow-y-auto")
520+
expect(scrollContainer).toBeInTheDocument()
521+
522+
// No pinned section should exist when no configs are pinned
523+
const pinnedSection = scrollContainer?.querySelector(".sticky.top-0")
524+
expect(pinnedSection).not.toBeInTheDocument()
535525

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")
526+
// Should have unpinned configs section with all configs
527+
const unpinnedSection = scrollContainer?.querySelector('[aria-label="All configurations"]')
528+
expect(unpinnedSection).toBeInTheDocument()
539529

540-
// All configs should be in the scrollable container
541-
const allConfigRows = scrollableContainer?.querySelectorAll(".group")
530+
// All configs should be in the unpinned section
531+
const allConfigRows = unpinnedSection?.querySelectorAll(".group")
542532
expect(allConfigRows?.length).toBe(10)
543533

544-
// No separator should exist
545-
const separator = popoverContent.querySelector(".h-px.bg-vscode-dropdown-foreground\\/10")
546-
expect(separator).not.toBeInTheDocument()
534+
// No separator should exist when no pinned configs (no sticky header exists)
535+
const stickyHeader = scrollContainer?.querySelector(".sticky.top-0")
536+
expect(stickyHeader).not.toBeInTheDocument()
547537
})
548538
})

0 commit comments

Comments
 (0)