diff --git a/web/lib/opal/src/components/buttons/link-button/styles.css b/web/lib/opal/src/components/buttons/link-button/styles.css index 93e6e75a47..f9091b981c 100644 --- a/web/lib/opal/src/components/buttons/link-button/styles.css +++ b/web/lib/opal/src/components/buttons/link-button/styles.css @@ -9,8 +9,10 @@ `currentColor` so `text-text-03` on the root cascades through. ============================================================================ */ +/* `relative` contains the absolute `sr-only` "(opens in new tab)" span. + Without it the span escapes any scroll container and grows ``. */ .opal-link-button { - @apply inline-flex flex-row items-center gap-0.5 text-text-03; + @apply relative inline-flex flex-row items-center gap-0.5 text-text-03; background: transparent; border: 0; padding: 0; diff --git a/web/lib/opal/src/components/dropdown/Dropdown.test.tsx b/web/lib/opal/src/components/dropdown/Dropdown.test.tsx index 8b70102733..da7cbfa1f9 100644 --- a/web/lib/opal/src/components/dropdown/Dropdown.test.tsx +++ b/web/lib/opal/src/components/dropdown/Dropdown.test.tsx @@ -3,6 +3,7 @@ import { render, screen } from "@tests/setup/test-utils"; import "@testing-library/jest-dom"; import userEvent from "@testing-library/user-event"; import { Dropdown, InputTypeIn } from "@opal/components"; +import { scrollWithinList } from "@opal/components/dropdown/list"; import type { DropdownItem, DropdownMenuItem, @@ -845,3 +846,84 @@ describe("Dropdown view registry", () => { ).toHaveAttribute("aria-checked", "true"); }); }); + +interface ListGeometry { + /** The row's viewport top; the scroller's view spans 100–300. */ + rowTop: number; + margin?: number; +} + +/** + * A scroller and one row with stubbed geometry: jsdom has no layout. The + * scroller's view is 100–300 in the viewport, scrolled 50px; rows are 40px. + */ +function listGeometry({ rowTop, margin = 0 }: ListGeometry) { + const scroller = document.createElement("div"); + scroller.className = "opal-dropdown-scroll"; + const row = document.createElement("div"); + scroller.appendChild(row); + document.body.appendChild(scroller); + Object.defineProperty(scroller, "clientHeight", { value: 200 }); + Object.defineProperty(scroller, "scrollTop", { value: 50, writable: true }); + scroller.getBoundingClientRect = () => new DOMRect(0, 100, 200, 200); + row.getBoundingClientRect = () => new DOMRect(0, rowTop, 200, 40); + const computed = window.getComputedStyle; + jest.spyOn(window, "getComputedStyle").mockImplementation((el, pseudo) => + el === row + ? Object.assign(computed(el, pseudo), { + scrollMarginBlockStart: `${margin}px`, + scrollMarginBlockEnd: `${margin}px`, + }) + : computed(el, pseudo) + ); + return { scroller, row }; +} + +describe("scrollWithinList", () => { + afterEach(() => { + jest.restoreAllMocks(); + document.body.innerHTML = ""; + }); + + test("center puts the row in the middle of the view", () => { + const { scroller, row } = listGeometry({ rowTop: 260 }); + scrollWithinList(row, "center"); + // Content top 210, centred in 200px: 210 - (200 - 40) / 2. + expect(scroller.scrollTop).toBe(130); + }); + + test("nearest leaves a row clear of the edges where it is", () => { + const { scroller, row } = listGeometry({ rowTop: 150, margin: 12 }); + scrollWithinList(row, "nearest"); + expect(scroller.scrollTop).toBe(50); + }); + + test("nearest keeps a row's scroll margin clear of the bottom edge", () => { + // Fully in view, but its margin reaches under the bottom fade. + const { scroller, row } = listGeometry({ rowTop: 250, margin: 12 }); + scrollWithinList(row, "nearest"); + // Content top 200: 200 + 40 + 12 - 200. + expect(scroller.scrollTop).toBe(52); + }); + + test("nearest keeps a row's scroll margin clear of the top edge", () => { + const { scroller, row } = listGeometry({ rowTop: 90, margin: 12 }); + scrollWithinList(row, "nearest"); + // Content top 40: 40 - 12. + expect(scroller.scrollTop).toBe(28); + }); +}); + +describe("Dropdown scrolling", () => { + // Opening around the selection waits for floating-ui to place the list, + // which never happens in jsdom; `scrollWithinList`'s own tests cover it. + test("walking the rows never scrolls outside the list", async () => { + jest.mocked(Element.prototype.scrollIntoView).mockClear(); + const user = setupUser(); + render(); + await user.click(screen.getByRole("button", { name: "Fruit" })); + await user.keyboard("{ArrowDown}{ArrowDown}{ArrowDown}{ArrowUp}"); + // scrollIntoView scrolls every ancestor of the portal, the page too. + expect(Element.prototype.scrollIntoView).not.toHaveBeenCalled(); + }); +}); diff --git a/web/lib/opal/src/components/dropdown/list.tsx b/web/lib/opal/src/components/dropdown/list.tsx index d3827b1153..e93ce16358 100644 --- a/web/lib/opal/src/components/dropdown/list.tsx +++ b/web/lib/opal/src/components/dropdown/list.tsx @@ -138,7 +138,10 @@ export const DropdownList = forwardRef( const searchRef = useRef(null); const hasSearch = searchField !== undefined; useEffect(() => { - if (isOpen && hasSearch) searchRef.current?.focus(); + // preventScroll: the field can be anywhere before floating-ui places + // the list, and a focus scroll would move the page behind the portal. + if (isOpen && hasSearch) + searchRef.current?.focus({ preventScroll: true }); }, [isOpen, hasSearch, viewKey]); const listRef = useRef(null); @@ -202,9 +205,12 @@ export const DropdownList = forwardRef( // highlights never scroll: the list must not move under the mouse. useEffect(() => { if (!isOpen || !keyboardNav || highlightedIndex < 0) return; - liveCard(listRef.current) - ?.querySelector(`[data-index="${highlightedIndex}"]`) - ?.scrollIntoView({ block: "nearest", behavior: "instant" }); + scrollWithinList( + liveCard(listRef.current)?.querySelector( + `[data-index="${highlightedIndex}"]` + ), + "nearest" + ); }, [highlightedIndex, isOpen, keyboardNav]); // Opening shows the selection: the (first) selected row is centred in @@ -217,7 +223,7 @@ export const DropdownList = forwardRef( const selected = liveCard(listRef.current)?.querySelector( '[role="option"][aria-selected="true"]' ); - selected?.scrollIntoView({ block: "center", behavior: "instant" }); + scrollWithinList(selected, "center"); }, [isOpen, isPositioned]); const cardProps: CardProps = { @@ -343,6 +349,43 @@ function liveCard(root: HTMLElement | null): HTMLElement | null { return root?.querySelector(".opal-dropdown-card") ?? null; } +/** + * Scrolls a row into view inside the list's own scroller only. + * `scrollIntoView` also scrolls every ancestor of the portal, `` + * included even under `overflow: hidden`, which moves the whole page. + * Like `scrollIntoView`, "nearest" keeps the row's `scroll-margin-block` + * clear of the edge, so a stop never parks under the mask fade. + */ +export function scrollWithinList( + row: Element | null | undefined, + block: "nearest" | "center" +): void { + const scroller: HTMLElement | null | undefined = row?.closest( + ".opal-dropdown-scroll" + ); + if (!row || !scroller) return; + const rowRect: DOMRect = row.getBoundingClientRect(); + const viewTop: number = + scroller.getBoundingClientRect().top + scroller.clientTop; + const viewBottom: number = viewTop + scroller.clientHeight; + // The row's top in the scroller's content coordinates. + const rowTop: number = rowRect.top - viewTop + scroller.scrollTop; + if (block === "center") { + scroller.scrollTop = rowTop - (scroller.clientHeight - rowRect.height) / 2; + return; + } + const style: CSSStyleDeclaration = window.getComputedStyle(row); + const marginStart: number = + Number.parseFloat(style.scrollMarginBlockStart) || 0; + const marginEnd: number = Number.parseFloat(style.scrollMarginBlockEnd) || 0; + if (rowRect.top - marginStart < viewTop) { + scroller.scrollTop = rowTop - marginStart; + } else if (rowRect.bottom + marginEnd > viewBottom) { + scroller.scrollTop = + rowTop + rowRect.height + marginEnd - scroller.clientHeight; + } +} + /** What a card renders: the search field and the rows of one view. */ interface CardProps { listId: string;