From 5864cc73a49d8c88820d5feaf17d4874c665fd7b Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Thu, 24 Sep 2026 10:47:54 +0500 Subject: [PATCH 1/3] fix(portal): preserve skill search during selection Keep the active skill query and result list open while users select multiple matching skills. Position the scrollable dropdown within the available viewport space and cover both behaviors with focused tests and a Storybook scenario. Closes #1284 Signed-off-by: Asad Ali --- .../src/components/SkillPicker.stories.tsx | 56 ++++ .../src/components/SkillPicker.test.tsx | 101 +++++++ apps/portal/src/components/SkillPicker.tsx | 268 ++++++++++-------- docs/architecture/skills.md | 5 + 4 files changed, 318 insertions(+), 112 deletions(-) create mode 100644 apps/portal/src/components/SkillPicker.stories.tsx create mode 100644 apps/portal/src/components/SkillPicker.test.tsx diff --git a/apps/portal/src/components/SkillPicker.stories.tsx b/apps/portal/src/components/SkillPicker.stories.tsx new file mode 100644 index 000000000..f69bef7f2 --- /dev/null +++ b/apps/portal/src/components/SkillPicker.stories.tsx @@ -0,0 +1,56 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { useState } from "react"; +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { expect, userEvent } from "storybook/test"; +import { http, HttpResponse } from "msw"; +import type { SkillDocument } from "@/types"; +import { SkillPicker } from "./SkillPicker"; + +const skills = Array.from({ length: 13 }, (_, index) => ({ + _id: `azure/cosmos-kit/cosmos-${String(index + 1).padStart(2, "0")}`, + name: `Cosmos DB skill ${index + 1}`, + description: "Azure Cosmos DB agent guidance", +})) as SkillDocument[]; + +function SkillPickerHarness() { + const [selected, setSelected] = useState([]); + return ( +
+ +
+ ); +} + +const meta = { + component: SkillPicker, + render: () => , + args: { + selected: [], + onChange: () => {}, + }, + parameters: { + msw: { + handlers: [ + http.get("*/api/v1/skills", () => HttpResponse.json(skills)), + http.get("*/api/v1/skills/search/external", () => HttpResponse.json([])), + http.get("*/api/v1/skills/*/revisions", () => HttpResponse.json([])), + ], + }, + }, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +export const MultiSelectSearch: Story = { + play: async ({ canvas }) => { + const input = await canvas.findByPlaceholderText("Search for skills…"); + await userEvent.type(input, "cosmos"); + await userEvent.click(await canvas.findByRole("button", { name: /cosmos-01/i })); + + await expect(input).toHaveValue("cosmos"); + await expect(canvas.getByRole("button", { name: /cosmos-02/i })).toBeVisible(); + }, +}; diff --git a/apps/portal/src/components/SkillPicker.test.tsx b/apps/portal/src/components/SkillPicker.test.tsx new file mode 100644 index 000000000..75133bb43 --- /dev/null +++ b/apps/portal/src/components/SkillPicker.test.tsx @@ -0,0 +1,101 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +// @vitest-environment happy-dom +import { useState } from "react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import type { SkillDocument } from "@/types"; + +const skills = [ + { + _id: "azure/cosmos-kit/cosmos-query", + name: "Cosmos query", + description: "Query Azure Cosmos DB", + }, + { + _id: "azure/cosmos-kit/cosmos-index", + name: "Cosmos index", + description: "Tune Azure Cosmos DB indexes", + }, +] as SkillDocument[]; + +const listSkills = vi.fn(async () => skills); +const searchExternalSkills = vi.fn(async (_query: string, _limit?: number) => []); +const listSkillRevisions = vi.fn(async (_slug: string) => []); + +vi.mock("@/lib/api", () => ({ + api: { + listSkills: () => listSkills(), + searchExternalSkills: (query: string, limit?: number) => searchExternalSkills(query, limit), + listSkillRevisions: (slug: string) => listSkillRevisions(slug), + }, +})); + +import { SkillPicker } from "./SkillPicker"; + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); + +function PickerHarness() { + const [selected, setSelected] = useState([]); + return ; +} + +function renderPicker() { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + return render( + + + , + ); +} + +describe("SkillPicker", () => { + it("keeps the search and matching results open while selecting several skills", async () => { + renderPicker(); + const input = await screen.findByPlaceholderText("Search for skills…"); + + fireEvent.change(input, { target: { value: "cosmos" } }); + fireEvent.click(await screen.findByRole("button", { name: /cosmos-query/i })); + + expect((input as HTMLInputElement).value).toBe("cosmos"); + expect(screen.getByRole("button", { name: /cosmos-index/i })).toBeDefined(); + + fireEvent.click(screen.getByRole("button", { name: /cosmos-index/i })); + expect((input as HTMLInputElement).value).toBe("cosmos"); + expect(screen.getAllByText("azure/cosmos-kit/cosmos-query").length).toBeGreaterThan(1); + expect(screen.getAllByText("azure/cosmos-kit/cosmos-index").length).toBeGreaterThan(1); + }); + + it("opens the scrollable result list above an input near the viewport bottom", async () => { + renderPicker(); + const input = await screen.findByPlaceholderText("Search for skills…"); + vi.spyOn(input, "getBoundingClientRect").mockReturnValue({ + x: 0, + y: 440, + top: 440, + right: 400, + bottom: 476, + left: 0, + width: 400, + height: 36, + toJSON: () => ({}), + }); + Object.defineProperty(window, "innerHeight", { configurable: true, value: 500 }); + + fireEvent.focus(input); + + const result = await screen.findByRole("button", { name: /cosmos-query/i }); + await waitFor(() => { + const dropdown = result.parentElement?.parentElement; + expect(dropdown?.className).toContain("bottom-full"); + expect(result.parentElement?.style.maxHeight).toBe("256px"); + }); + }); +}); diff --git a/apps/portal/src/components/SkillPicker.tsx b/apps/portal/src/components/SkillPicker.tsx index f6b0fca26..f1e3703df 100644 --- a/apps/portal/src/components/SkillPicker.tsx +++ b/apps/portal/src/components/SkillPicker.tsx @@ -85,6 +85,11 @@ interface SkillPickerProps { disabled?: boolean; } +type DropdownLayout = { + side: "above" | "below"; + maxHeight: number; +}; + // --------------------------------------------------------------------------- // Component // --------------------------------------------------------------------------- @@ -97,6 +102,10 @@ export function SkillPicker({ selected, onChange, importOnly = false, disabled = const [open, setOpen] = useState(false); const [highlightIdx, setHighlightIdx] = useState(0); const [manualOpen, setManualOpen] = useState(false); + const [dropdownLayout, setDropdownLayout] = useState({ + side: "below", + maxHeight: 256, + }); const debouncedQuery = useDebounce(query, 300); @@ -175,6 +184,38 @@ export function SkillPicker({ selected, onChange, importOnly = false, disabled = return () => document.removeEventListener("mousedown", handler); }, []); + // Keep the result list within the viewport. The picker often appears near + // the bottom of long profile forms, where a fixed downward dropdown would + // otherwise be clipped. + const updateDropdownLayout = useCallback(() => { + const rect = inputRef.current?.getBoundingClientRect(); + if (!rect) return; + + const viewportGap = 8; + const preferredHeight = 256; + const spaceBelow = Math.max(0, window.innerHeight - rect.bottom - viewportGap); + const spaceAbove = Math.max(0, rect.top - viewportGap); + const side = spaceBelow >= Math.min(preferredHeight, spaceAbove) ? "below" : "above"; + const availableSpace = side === "below" ? spaceBelow : spaceAbove; + + setDropdownLayout({ + side, + maxHeight: Math.min(preferredHeight, availableSpace), + }); + }, []); + + useEffect(() => { + if (!open) return; + + updateDropdownLayout(); + window.addEventListener("resize", updateDropdownLayout); + window.addEventListener("scroll", updateDropdownLayout, true); + return () => { + window.removeEventListener("resize", updateDropdownLayout); + window.removeEventListener("scroll", updateDropdownLayout, true); + }; + }, [open, selected.length, updateDropdownLayout]); + // ─── Import mutation (for external skills) ────────────────────────── const importMutation = useMutation({ mutationFn: (result: SkillSearchResult) => @@ -215,7 +256,6 @@ export function SkillPicker({ selected, onChange, importOnly = false, disabled = } else { onChange([...selected, id]); // Add without hash (latest) } - setQuery(""); inputRef.current?.focus(); }, [selected, selectedMap, onChange, importOnly], @@ -322,123 +362,127 @@ export function SkillPicker({ selected, onChange, importOnly = false, disabled = {fetchingExternal && debouncedQuery.length >= 2 && ( )} - - )} - {/* Dropdown */} - {showDropdown && ( -
-
- {/* Internal results */} - {internalMatches.length > 0 && ( - <> - {debouncedQuery.length >= 2 && filteredExternal.length > 0 && ( -
- Imported -
- )} - {internalMatches.map((skill, idx) => { - const isSelected = selectedMap.has(skill._id); - return ( - - ); - })} - - )} - - {/* External results */} - {debouncedQuery.length >= 2 && filteredExternal.length > 0 && ( - <> -
- - External Registry -
- {filteredExternal.map((result, i) => { - const globalIdx = internalMatches.length + i; - return ( -
setHighlightIdx(globalIdx)} - className={`flex items-center gap-2 px-2 py-1.5 rounded-sm text-sm transition-colors ${ - globalIdx === highlightIdx ? "bg-accent text-accent-foreground" : "" - }`} - > - -
-
- {result.id} - {result.installs != null && ( - - {result.installs.toLocaleString()} installs - - )} -
-

- {result.name}{result.description ? ` — ${result.description}` : ""} -

-
- -
- ); - })} - - )} - - {/* Loading indicator for external */} - {debouncedQuery.length >= 2 && fetchingExternal && filteredExternal.length === 0 && ( -
- - Searching external registry… -
- )} + + {skill._id} + + {skill.name}{skill.description ? ` — ${skill.description}` : ""} + + + ); + })} + + )} + + {/* External results */} + {debouncedQuery.length >= 2 && filteredExternal.length > 0 && ( + <> +
+ + External Registry +
+ {filteredExternal.map((result, i) => { + const globalIdx = internalMatches.length + i; + return ( +
setHighlightIdx(globalIdx)} + className={`flex items-center gap-2 px-2 py-1.5 rounded-sm text-sm transition-colors ${ + globalIdx === highlightIdx ? "bg-accent text-accent-foreground" : "" + }`} + > + +
+
+ {result.id} + {result.installs != null && ( + + {result.installs.toLocaleString()} installs + + )} +
+

+ {result.name}{result.description ? ` — ${result.description}` : ""} +

+
+ +
+ ); + })} + + )} + + {/* Loading indicator for external */} + {debouncedQuery.length >= 2 && fetchingExternal && filteredExternal.length === 0 && ( +
+ + Searching external registry… +
+ )} - {/* Empty state */} - {items.length === 0 && !fetchingExternal && query.trim() && ( -
- No matching skills found -
- )} + {/* Empty state */} + {items.length === 0 && !fetchingExternal && query.trim() && ( +
+ No matching skills found +
+ )} +
- + )} + )} {/* Manual add section */} diff --git a/docs/architecture/skills.md b/docs/architecture/skills.md index 187bc1d32..2c8000d45 100644 --- a/docs/architecture/skills.md +++ b/docs/architecture/skills.md @@ -118,6 +118,11 @@ Used when the user doesn't know which repo contains the skill they want. 3. User selects a result from skills.sh 4. API imports it the same way as Path 1 — registers the skill and resolves content from GitHub +When selecting skills for a profile, the Portal keeps the current search term +and result list open after each selection so related skills can be added in one +pass. The result list scrolls within the available viewport space and opens +above the search field when there is not enough room below it. + The skill's `origin` is set to `"skills-sh"` to indicate it was discovered through that registry. ### Key distinction From 257919a03a67e7e4b41da59de0855f2246d3138a Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Thu, 24 Sep 2026 14:31:30 +0500 Subject: [PATCH 2/3] test(portal): scope the skill picker story Signed-off-by: Asad Ali --- apps/portal/src/components/SkillPicker.stories.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/portal/src/components/SkillPicker.stories.tsx b/apps/portal/src/components/SkillPicker.stories.tsx index f69bef7f2..aa93d7d2b 100644 --- a/apps/portal/src/components/SkillPicker.stories.tsx +++ b/apps/portal/src/components/SkillPicker.stories.tsx @@ -6,6 +6,7 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; import { expect, userEvent } from "storybook/test"; import { http, HttpResponse } from "msw"; import type { SkillDocument } from "@/types"; +import { setSelectedProjectIdHolder } from "@/lib/project-scope"; import { SkillPicker } from "./SkillPicker"; const skills = Array.from({ length: 13 }, (_, index) => ({ @@ -15,6 +16,7 @@ const skills = Array.from({ length: 13 }, (_, index) => ({ })) as SkillDocument[]; function SkillPickerHarness() { + setSelectedProjectIdHolder("demo-project"); const [selected, setSelected] = useState([]); return (
From d9c6ec6d2aaab933ccc49964c22e7e3bb56b1546 Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Thu, 24 Sep 2026 20:17:12 +0500 Subject: [PATCH 3/3] fix(portal): address skill picker review feedback Signed-off-by: Asad Ali --- .../src/components/SkillPicker.stories.tsx | 25 ++++-- .../src/components/SkillPicker.test.tsx | 37 ++++++++- apps/portal/src/components/SkillPicker.tsx | 81 +++++++++++++++---- docs/architecture/skills.md | 5 +- 4 files changed, 120 insertions(+), 28 deletions(-) diff --git a/apps/portal/src/components/SkillPicker.stories.tsx b/apps/portal/src/components/SkillPicker.stories.tsx index aa93d7d2b..c42a44bd5 100644 --- a/apps/portal/src/components/SkillPicker.stories.tsx +++ b/apps/portal/src/components/SkillPicker.stories.tsx @@ -1,12 +1,12 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { useState } from "react"; +import { useEffect, useState } from "react"; import type { Meta, StoryObj } from "@storybook/react-vite"; -import { expect, userEvent } from "storybook/test"; +import { expect, userEvent, within } from "storybook/test"; import { http, HttpResponse } from "msw"; import type { SkillDocument } from "@/types"; -import { setSelectedProjectIdHolder } from "@/lib/project-scope"; +import { getSelectedProjectId, setSelectedProjectIdHolder } from "@/lib/project-scope"; import { SkillPicker } from "./SkillPicker"; const skills = Array.from({ length: 13 }, (_, index) => ({ @@ -16,8 +16,18 @@ const skills = Array.from({ length: 13 }, (_, index) => ({ })) as SkillDocument[]; function SkillPickerHarness() { - setSelectedProjectIdHolder("demo-project"); const [selected, setSelected] = useState([]); + const [scopeReady, setScopeReady] = useState(false); + + useEffect(() => { + const previousProjectId = getSelectedProjectId(); + setSelectedProjectIdHolder("demo-project"); + setScopeReady(true); + return () => setSelectedProjectIdHolder(previousProjectId); + }, []); + + if (!scopeReady) return null; + return (
@@ -47,12 +57,13 @@ export default meta; type Story = StoryObj; export const MultiSelectSearch: Story = { - play: async ({ canvas }) => { + play: async ({ canvas, canvasElement }) => { + const page = within(canvasElement.ownerDocument.body); const input = await canvas.findByPlaceholderText("Search for skills…"); await userEvent.type(input, "cosmos"); - await userEvent.click(await canvas.findByRole("button", { name: /cosmos-01/i })); + await userEvent.click(await page.findByRole("button", { name: /cosmos-01/i })); await expect(input).toHaveValue("cosmos"); - await expect(canvas.getByRole("button", { name: /cosmos-02/i })).toBeVisible(); + await expect(page.getByRole("button", { name: /cosmos-02/i })).toBeVisible(); }, }; diff --git a/apps/portal/src/components/SkillPicker.test.tsx b/apps/portal/src/components/SkillPicker.test.tsx index 75133bb43..f3adf3fd4 100644 --- a/apps/portal/src/components/SkillPicker.test.tsx +++ b/apps/portal/src/components/SkillPicker.test.tsx @@ -74,7 +74,7 @@ describe("SkillPicker", () => { }); it("opens the scrollable result list above an input near the viewport bottom", async () => { - renderPicker(); + const { container } = renderPicker(); const input = await screen.findByPlaceholderText("Search for skills…"); vi.spyOn(input, "getBoundingClientRect").mockReturnValue({ x: 0, @@ -93,9 +93,40 @@ describe("SkillPicker", () => { const result = await screen.findByRole("button", { name: /cosmos-query/i }); await waitFor(() => { - const dropdown = result.parentElement?.parentElement; - expect(dropdown?.className).toContain("bottom-full"); + const dropdown = document.querySelector("[data-skill-picker-portal]") as HTMLElement | null; + expect(dropdown?.parentElement).toBe(document.body); + expect(container.contains(dropdown)).toBe(false); + expect(dropdown?.style.position).toBe("fixed"); + expect(dropdown?.style.bottom).toBe("64px"); + expect(dropdown?.style.top).toBe(""); expect(result.parentElement?.style.maxHeight).toBe("256px"); }); }); + + it("constrains the portaled result list to the space below the input", async () => { + renderPicker(); + const input = await screen.findByPlaceholderText("Search for skills…"); + vi.spyOn(input, "getBoundingClientRect").mockReturnValue({ + x: 20, + y: 8, + top: 8, + right: 420, + bottom: 44, + left: 20, + width: 400, + height: 36, + toJSON: () => ({}), + }); + Object.defineProperty(window, "innerHeight", { configurable: true, value: 180 }); + + fireEvent.focus(input); + + const result = await screen.findByRole("button", { name: /cosmos-query/i }); + await waitFor(() => { + const dropdown = document.querySelector("[data-skill-picker-portal]") as HTMLElement | null; + expect(dropdown?.style.top).toBe("48px"); + expect(dropdown?.style.bottom).toBe(""); + expect(result.parentElement?.style.maxHeight).toBe("128px"); + }); + }); }); diff --git a/apps/portal/src/components/SkillPicker.tsx b/apps/portal/src/components/SkillPicker.tsx index f1e3703df..1d7abbe0d 100644 --- a/apps/portal/src/components/SkillPicker.tsx +++ b/apps/portal/src/components/SkillPicker.tsx @@ -2,6 +2,7 @@ // Licensed under the MIT License. import { useState, useMemo, useRef, useEffect, useCallback } from "react"; +import { createPortal } from "react-dom"; import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; import { api } from "@/lib/api"; import { Badge } from "@/components/ui/badge"; @@ -87,6 +88,10 @@ interface SkillPickerProps { type DropdownLayout = { side: "above" | "below"; + left: number; + width: number; + top?: number; + bottom?: number; maxHeight: number; }; @@ -96,6 +101,7 @@ type DropdownLayout = { export function SkillPicker({ selected, onChange, importOnly = false, disabled = false }: SkillPickerProps) { const queryClient = useQueryClient(); const containerRef = useRef(null); + const dropdownRef = useRef(null); const inputRef = useRef(null); const [query, setQuery] = useState(""); @@ -104,6 +110,9 @@ export function SkillPicker({ selected, onChange, importOnly = false, disabled = const [manualOpen, setManualOpen] = useState(false); const [dropdownLayout, setDropdownLayout] = useState({ side: "below", + left: 0, + width: 0, + top: 0, maxHeight: 256, }); @@ -176,7 +185,12 @@ export function SkillPicker({ selected, onChange, importOnly = false, disabled = // ─── Outside click ────────────────────────────────────────────────── useEffect(() => { const handler = (e: MouseEvent) => { - if (containerRef.current && !containerRef.current.contains(e.target as Node)) { + const target = e.target as Node; + if ( + containerRef.current && + !containerRef.current.contains(target) && + !dropdownRef.current?.contains(target) + ) { setOpen(false); } }; @@ -184,9 +198,8 @@ export function SkillPicker({ selected, onChange, importOnly = false, disabled = return () => document.removeEventListener("mousedown", handler); }, []); - // Keep the result list within the viewport. The picker often appears near - // the bottom of long profile forms, where a fixed downward dropdown would - // otherwise be clipped. + // Keep the result list within the viewport. The fixed, portaled layer avoids + // clipping by scrollable profile forms and dialogs. const updateDropdownLayout = useCallback(() => { const rect = inputRef.current?.getBoundingClientRect(); if (!rect) return; @@ -198,21 +211,47 @@ export function SkillPicker({ selected, onChange, importOnly = false, disabled = const side = spaceBelow >= Math.min(preferredHeight, spaceAbove) ? "below" : "above"; const availableSpace = side === "below" ? spaceBelow : spaceAbove; - setDropdownLayout({ + const nextLayout: DropdownLayout = { side, + left: rect.left, + width: rect.width, + ...(side === "below" + ? { top: rect.bottom + 4 } + : { bottom: window.innerHeight - rect.top + 4 }), maxHeight: Math.min(preferredHeight, availableSpace), - }); + }; + + setDropdownLayout((current) => + current.side === nextLayout.side && + current.left === nextLayout.left && + current.width === nextLayout.width && + current.top === nextLayout.top && + current.bottom === nextLayout.bottom && + current.maxHeight === nextLayout.maxHeight + ? current + : nextLayout, + ); }, []); useEffect(() => { if (!open) return; + let rafId: number | undefined; + + const scheduleDropdownLayout = () => { + if (rafId !== undefined) return; + rafId = window.requestAnimationFrame(() => { + rafId = undefined; + updateDropdownLayout(); + }); + }; updateDropdownLayout(); - window.addEventListener("resize", updateDropdownLayout); - window.addEventListener("scroll", updateDropdownLayout, true); + window.addEventListener("resize", scheduleDropdownLayout); + window.addEventListener("scroll", scheduleDropdownLayout, true); return () => { - window.removeEventListener("resize", updateDropdownLayout); - window.removeEventListener("scroll", updateDropdownLayout, true); + if (rafId !== undefined) window.cancelAnimationFrame(rafId); + window.removeEventListener("resize", scheduleDropdownLayout); + window.removeEventListener("scroll", scheduleDropdownLayout, true); }; }, [open, selected.length, updateDropdownLayout]); @@ -363,12 +402,21 @@ export function SkillPicker({ selected, onChange, importOnly = false, disabled = )} - {/* Dropdown */} - {showDropdown && ( + {/* Dropdown: portaled to avoid clipping by scrollable ancestors. */} + {showDropdown && createPortal(
{/* Internal results */} @@ -480,7 +528,8 @@ export function SkillPicker({ selected, onChange, importOnly = false, disabled =
)}
-
+
, + document.body, )} )} diff --git a/docs/architecture/skills.md b/docs/architecture/skills.md index 2c8000d45..588d13e01 100644 --- a/docs/architecture/skills.md +++ b/docs/architecture/skills.md @@ -120,8 +120,9 @@ Used when the user doesn't know which repo contains the skill they want. When selecting skills for a profile, the Portal keeps the current search term and result list open after each selection so related skills can be added in one -pass. The result list scrolls within the available viewport space and opens -above the search field when there is not enough room below it. +pass. The result list is rendered in a fixed portal so scrollable forms cannot +clip it; it stays within the available viewport space and opens above the +search field when there is not enough room below it. The skill's `origin` is set to `"skills-sh"` to indicate it was discovered through that registry.