diff --git a/e2e/server-catalog.spec.ts b/e2e/server-catalog.spec.ts index bcbb006..6789c04 100644 --- a/e2e/server-catalog.spec.ts +++ b/e2e/server-catalog.spec.ts @@ -108,22 +108,23 @@ test.describe("Server catalog page", () => { await expect.poll(() => new URL(page.url()).searchParams.get("search")).toBe("notes"); }); - test("filters servers by category through the filters dialog", async ({ page }) => { + test("filters servers by category through the filters popover", async ({ page }) => { await mockCatalog(page, [OPEN_CONNECTED, OPEN_AVAILABLE]); await page.goto(APP.SERVER_CATALOG); await page.waitForLoadState("networkidle"); await page.getByRole("button", { name: /^Filters(, \d+ active)?$/ }).click(); - const dialog = page.getByRole("dialog", { name: "Add filters" }); - await expect(dialog).toBeVisible(); + const popover = page.getByRole("dialog", { name: "Filters" }); + await expect(popover).toBeVisible(); - const categorySection = dialog.getByRole("group", { name: "Categories" }); - await categorySection.getByRole("radio", { name: "Select..." }).click(); + const categorySection = popover.getByRole("group", { name: "Categories" }); + await categorySection.getByRole("radio", { name: "Select" }).click(); await categorySection.getByRole("checkbox", { name: "Productivity" }).check(); - await dialog.getByRole("button", { name: "Add filters" }).click(); - await expect(dialog).toHaveCount(0); + // Filters commit as they are ticked, so the popover stays open over a grid + // that has already narrowed. + await expect(popover).toBeVisible(); await expect.poll(() => new URL(page.url()).searchParams.get("category")).toBe("Productivity"); await expect(page.getByRole("heading", { name: "Public Notes" })).toBeVisible(); await expect(page.getByRole("heading", { name: "Globalping" })).toHaveCount(0); @@ -309,4 +310,34 @@ test.describe("Server catalog page", () => { await page.getByRole("button", { name: "Dismiss notification" }).click(); await expect(page.getByRole("heading", { name: "Server catalog" })).toBeFocused(); }); + + test("scrolls the filter options without also scrolling the panel", async ({ page }) => { + // 700px is where the panel used to start scrolling behind the already + // scrolling options grid, putting two scrollbars on screen at once. + await page.setViewportSize({ width: 1400, height: 700 }); + await mockCatalog( + page, + Array.from({ length: 40 }, (_, index) => ({ + ...OPEN_AVAILABLE, + id: `overflow-${index}`, + name: `Server ${index}`, + provider: `Provider ${String(index).padStart(2, "0")}`, + })), + ); + + await page.goto(APP.SERVER_CATALOG); + await page.waitForLoadState("networkidle"); + await page.getByRole("button", { name: /^Filters(, \d+ active)?$/ }).click(); + await page.getByRole("group", { name: "Providers" }).waitFor(); + + const scrollState = await page.evaluate(() => { + const panel = document.querySelector("[data-slot=popover-content]") as HTMLElement; + const grid = panel.querySelector("[role=group] > div:last-child") as HTMLElement; + const scrolls = (el: HTMLElement) => el.scrollHeight > el.clientHeight; + return { panel: scrolls(panel), grid: scrolls(grid) }; + }); + + expect(scrollState.grid).toBe(true); + expect(scrollState.panel).toBe(false); + }); }); diff --git a/src/components/plugins/PluginToolbar.tsx b/src/components/plugins/PluginToolbar.tsx index 679c436..996ca4c 100644 --- a/src/components/plugins/PluginToolbar.tsx +++ b/src/components/plugins/PluginToolbar.tsx @@ -86,6 +86,7 @@ function PluginFiltersPopover({ const id = useId(); const filtersTitleId = `${id}-title`; const hookTriggerId = `${id}-hook`; + const tagsLabelId = `${id}-tags-label`; return ( @@ -94,7 +95,7 @@ function PluginFiltersPopover({ type="button" variant="ghost" size="sm" - className="w-fit gap-2 self-center text-xs text-secondary-foreground" + className="w-fit shrink-0 gap-2 text-xs text-secondary-foreground" aria-label={intl.formatMessage( { id: "plugins.catalog.filtersActive" }, { count: activeFilterCount }, @@ -109,8 +110,14 @@ function PluginFiltersPopover({ )} - -
+ {/* End-aligned because the trigger sits at the toolbar's right edge, where a + start-aligned panel would expand past the viewport. */} + +

{intl.formatMessage({ id: "plugins.catalog.filters" })}

@@ -121,7 +128,7 @@ function PluginFiltersPopover({ )}
-
+
@@ -148,28 +155,46 @@ function PluginFiltersPopover({
{availableTags.length > 0 && ( -
- + // role=group rather than fieldset: a rendered legend is not subtracted + // from the height flex assigns its fieldset, so the box overflows it. +
+ {intl.formatMessage({ id: "plugins.catalog.tags" })} - -
+ + {/* A grid rather than CSS columns: a height-capped multi-column box + overflows sideways into new columns instead of scrolling down. + + Container queries measure the panel's content box, which is 2rem + of padding and 2px of border narrower than the widths set above: + 18/24/32rem of panel leave 15.875/21.875/29.875rem to query. The + thresholds have to sit inside those, so they read a step lower + than the panel width that triggers them. */} +
{availableTags.map((tag, index) => { const checkboxId = `${id}-tag-${index}`; return ( -
+
onToggleTag(tag, checked === true)} /> -
); })}
-
+
)} @@ -189,14 +214,14 @@ export function PluginToolbar({
-
+
diff --git a/src/components/server-catalog/CatalogToolbar.tsx b/src/components/server-catalog/CatalogToolbar.tsx index b7acf77..0d58342 100644 --- a/src/components/server-catalog/CatalogToolbar.tsx +++ b/src/components/server-catalog/CatalogToolbar.tsx @@ -1,43 +1,29 @@ -import { useCallback, useId, useState } from "react"; +import { useCallback, useId, useState, type ReactNode } from "react"; import { Filter } from "lucide-react"; import { useIntl } from "react-intl"; import { Button } from "@/components/ui/button"; import { CardTag } from "@/components/ui/card-tag"; import { Checkbox } from "@/components/ui/checkbox"; -import { - Dialog, - DialogClose, - DialogContent, - DialogFooter, - DialogHeader, - DialogTitle, - DialogTrigger, -} from "@/components/ui/dialog"; import { Label } from "@/components/ui/label"; import { ListSearch } from "@/components/ui/list-search"; +import { Popover, PopoverContent, PopoverTrigger } from "@/components/ui/popover"; import { RadioGroup, RadioGroupItem } from "@/components/ui/radio-group"; +import { cn } from "@/lib/utils"; const ALL_MODE = "all"; const SELECT_MODE = "select"; -export interface CatalogFilterDraft { - category: string[]; - provider: string[]; - tags: string[]; -} +export type CatalogFilterSection = "category" | "provider" | "tags"; -type CatalogFilterSection = keyof CatalogFilterDraft; type CatalogSectionMode = typeof ALL_MODE | typeof SELECT_MODE; type CatalogSectionModes = Record; -function getSectionModes(draft: CatalogFilterDraft): CatalogSectionModes { - return { - category: draft.category.length > 0 ? SELECT_MODE : ALL_MODE, - provider: draft.provider.length > 0 ? SELECT_MODE : ALL_MODE, - tags: draft.tags.length > 0 ? SELECT_MODE : ALL_MODE, - }; -} +const DEFAULT_MODES: CatalogSectionModes = { + category: ALL_MODE, + provider: SELECT_MODE, + tags: ALL_MODE, +}; interface CatalogToolbarProps { search: string; @@ -51,9 +37,16 @@ interface CatalogToolbarProps { activeFilterCount: number; onSearchChange: (value: string) => void; onInstalledChange: (installedOnly: boolean) => void; - onApply: (draft: CatalogFilterDraft) => void; + onToggleOption: (section: CatalogFilterSection, option: string, checked: boolean) => void; + onClearSection: (section: CatalogFilterSection) => void; + onClearAll: () => void; } +type CatalogFiltersPopoverProps = Omit< + CatalogToolbarProps, + "search" | "installedOnly" | "onSearchChange" | "onInstalledChange" +>; + function CatalogViewToggle({ installedOnly, onChange, @@ -98,9 +91,12 @@ function CatalogFilterSectionFields({ options, selected, mode, + expanded, allLabel, selectLabel, + headerAction, onModeChange, + onExpand, onToggle, }: { idPrefix: string; @@ -109,16 +105,35 @@ function CatalogFilterSectionFields({ options: string[]; selected: string[]; mode: CatalogSectionMode; + expanded: boolean; allLabel: string; selectLabel: string; + headerAction?: ReactNode; onModeChange: (mode: string) => void; + onExpand: () => void; onToggle: (option: string, checked: boolean) => void; }) { + const intl = useIntl(); + const countId = `${idPrefix}-selected-count`; + const showsOptions = mode === SELECT_MODE && expanded; + return ( -
- - {legend} - + // role=group rather than fieldset so the section heading can share a row with + // the panel-level Clear all button, which a legend cannot do. + // + // Only the open section yields when the panel runs out of room, so its grid + // takes the squeeze and scrolls instead of the panel scrolling too. +
+
+ + {legend} + + {headerAction} +
@@ -127,28 +142,58 @@ function CatalogFilterSectionFields({ {allLabel}
-
- +
+ {/* Clicking Select also re-expands a section that was collapsed when + another one was opened, so onValueChange alone is not enough: Radix + does not fire it when the already-checked radio is clicked again. + The label forwards its click to this button, so it is covered too. */} + 0 ? countId : undefined} + /> + {selected.length > 0 && ( + <> + + + {intl.formatMessage( + { id: "mcpServer.catalog.selectedCount" }, + { count: selected.length }, + )} + + + )}
- {mode === SELECT_MODE && ( - // Multi-column rather than a grid so options read alphabetically down - // each column, as the design lays them out. -
+ {showsOptions && ( + // A grid rather than CSS columns: a height-capped multi-column box + // overflows sideways into new columns instead of scrolling down. +
{options.map((option, index) => { const checkboxId = `${idPrefix}-option-${index}`; return ( -
+
onToggle(option, checked === true)} /> -
@@ -156,11 +201,11 @@ function CatalogFilterSectionFields({ })}
)} -
+
); } -function CatalogFiltersDialog({ +function CatalogFiltersPopover({ category, provider, selectedTags, @@ -168,68 +213,67 @@ function CatalogFiltersDialog({ providers, availableTags, activeFilterCount, - onApply, -}: Omit) { + onToggleOption, + onClearSection, + onClearAll, +}: CatalogFiltersPopoverProps) { const intl = useIntl(); const id = useId(); - const [open, setOpen] = useState(false); - const initialDraft: CatalogFilterDraft = { category, provider, tags: selectedTags }; - const [draft, setDraft] = useState(initialDraft); - const [modes, setModes] = useState(() => getSectionModes(initialDraft)); + const [modes, setModes] = useState(DEFAULT_MODES); + // Only one section shows its options at a time; the rest collapse to their + // Select row and selection count, so every section stays reachable without + // scrolling the panel. + const [expanded, setExpanded] = useState("provider"); - // Seeded only when the dialog opens. The page re-renders on every debounced - // search keystroke, so syncing the draft in an effect would discard edits that - // are still in progress. + // Seeded when the popover opens so a section the user collapsed during an + // earlier visit does not stay collapsed over a selection made since. Providers + // keeps its previous mode instead: All and an empty Select both commit + // provider=[], so there is nothing to derive an explicit All back from. const handleOpenChange = useCallback( (nextOpen: boolean) => { - if (nextOpen) { - const committed: CatalogFilterDraft = { category, provider, tags: selectedTags }; - setDraft(committed); - setModes(getSectionModes(committed)); - } - setOpen(nextOpen); + if (!nextOpen) return; + setModes((previous) => ({ + category: category.length > 0 ? SELECT_MODE : ALL_MODE, + provider: previous.provider, + tags: selectedTags.length > 0 ? SELECT_MODE : ALL_MODE, + })); + setExpanded("provider"); }, - [category, provider, selectedTags], + [category.length, selectedTags.length], ); - const setSectionMode = useCallback((section: CatalogFilterSection, mode: string) => { - const nextMode: CatalogSectionMode = mode === SELECT_MODE ? SELECT_MODE : ALL_MODE; - setModes((previous) => ({ ...previous, [section]: nextMode })); - // Switching a section back to All clears that section and leaves the others - // untouched. Switching to Select keeps whatever was already ticked. - if (nextMode === ALL_MODE) { - setDraft((previous) => ({ ...previous, [section]: [] })); - } - }, []); - - const toggleSectionOption = useCallback( - (section: CatalogFilterSection, option: string, checked: boolean) => { - // Ticking a box always implies Select mode for that section. - if (checked) setModes((previous) => ({ ...previous, [section]: SELECT_MODE })); - setDraft((previous) => { - const current = previous[section]; - return { - ...previous, - [section]: checked ? [...current, option] : current.filter((item) => item !== option), - }; - }); + const setSectionMode = useCallback( + (section: CatalogFilterSection, mode: string) => { + const nextMode: CatalogSectionMode = mode === SELECT_MODE ? SELECT_MODE : ALL_MODE; + setModes((previous) => ({ ...previous, [section]: nextMode })); + // Switching a section back to All drops that section's filter and leaves + // the others untouched. + if (nextMode === ALL_MODE) { + onClearSection(section); + setExpanded((previous) => (previous === section ? null : previous)); + return; + } + setExpanded(section); }, - [], + [onClearSection], ); - const handleApply = useCallback(() => { - onApply(draft); - setOpen(false); - }, [draft, onApply]); + // Clear all returns the panel to the state a fresh open would show rather than + // leaving every section collapsed behind an All radio. + const handleClearAll = useCallback(() => { + onClearAll(); + setModes(DEFAULT_MODES); + setExpanded("provider"); + }, [onClearAll]); return ( - - + + - - - - - - - + + {/* End-aligned because the trigger sits at the toolbar's right edge, where a + start-aligned panel would expand past the viewport. The panel carries no + visible title, so it is named for assistive tech instead. */} + + 0 && ( + + ) + } + onModeChange={(mode) => setSectionMode("provider", mode)} + onExpand={() => setExpanded("provider")} + onToggle={(option, checked) => onToggleOption("provider", option, checked)} + /> -
- setSectionMode("provider", mode)} - onToggle={(option, checked) => toggleSectionOption("provider", option, checked)} - /> + setSectionMode("category", mode)} + onExpand={() => setExpanded("category")} + onToggle={(option, checked) => onToggleOption("category", option, checked)} + /> + {availableTags.length > 0 && ( setSectionMode("category", mode)} - onToggle={(option, checked) => toggleSectionOption("category", option, checked)} + idPrefix={`${id}-tags`} + legendId={`${id}-tags-legend`} + legend={intl.formatMessage({ id: "mcpServer.catalog.tags" })} + options={availableTags} + selected={selectedTags} + mode={modes.tags} + expanded={expanded === "tags"} + allLabel={intl.formatMessage({ id: "mcpServer.catalog.allTagsOption" })} + selectLabel={intl.formatMessage({ id: "mcpServer.catalog.selectTags" })} + onModeChange={(mode) => setSectionMode("tags", mode)} + onExpand={() => setExpanded("tags")} + onToggle={(option, checked) => onToggleOption("tags", option, checked)} /> - - {availableTags.length > 0 && ( - setSectionMode("tags", mode)} - onToggle={(option, checked) => toggleSectionOption("tags", option, checked)} - /> - )} -
- - - - - - - - - + )} + + ); } @@ -324,17 +372,17 @@ export function CatalogToolbar({
-
+
- +
); diff --git a/src/components/ui/popover.tsx b/src/components/ui/popover.tsx index 663d965..1984348 100644 --- a/src/components/ui/popover.tsx +++ b/src/components/ui/popover.tsx @@ -15,6 +15,7 @@ function PopoverContent({ className, align = "center", sideOffset = 4, + collisionPadding = 16, ...props }: React.ComponentProps) { return ( @@ -23,8 +24,11 @@ function PopoverContent({ data-slot="popover-content" align={align} sideOffset={sideOffset} + collisionPadding={collisionPadding} className={cn( - "z-50 w-72 origin-(--radix-popover-content-transform-origin) rounded-md border bg-popover p-4 text-popover-foreground shadow-md outline-hidden data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 data-[state=open]:animate-in data-[state=open]:fade-in-0 data-[state=open]:zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95", + // Clamped to the space Radix measured between the trigger and the + // viewport edge, so a wide or tall panel can never run off screen. + "scrollbar-thin z-50 max-h-(--radix-popover-content-available-height) w-72 max-w-[calc(100vw-2rem)] origin-(--radix-popover-content-transform-origin) overflow-y-auto rounded-md border bg-popover p-4 text-popover-foreground shadow-md outline-hidden data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2 data-[state=open]:animate-in data-[state=open]:fade-in-0 data-[state=open]:zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95", className, )} {...props} diff --git a/src/components/ui/sidebar.tsx b/src/components/ui/sidebar.tsx index 271ece8..8e41718 100644 --- a/src/components/ui/sidebar.tsx +++ b/src/components/ui/sidebar.tsx @@ -24,6 +24,9 @@ const SIDEBAR_WIDTH = "16rem"; const SIDEBAR_WIDTH_MOBILE = "18rem"; const SIDEBAR_WIDTH_ICON = "3rem"; const SIDEBAR_KEYBOARD_SHORTCUT = "b"; +// Collapses to a sheet below Tailwind's `lg`. Must stay in step with the `lg:` +// utilities that show the docked sidebar, or both would render at once. +const SIDEBAR_COLLAPSE_BREAKPOINT = 1024; type SidebarContextProps = { state: "expanded" | "collapsed"; @@ -59,7 +62,7 @@ function SidebarProvider({ open?: boolean; onOpenChange?: (open: boolean) => void; }) { - const isMobile = useIsMobile(); + const isMobile = useIsMobile(SIDEBAR_COLLAPSE_BREAKPOINT); const [openMobile, setOpenMobile] = React.useState(false); // This is the internal state of the sidebar. @@ -197,7 +200,7 @@ function Sidebar({ return (