mirror of
https://github.com/onyx-dot-app/onyx.git
synced 2026-10-19 01:50:51 -05:00
fix(opal): stop dropdowns from scrolling the page when they open (#15597)
This commit is contained in:
1 parent
09a6a311ab
commit
897500ebf7
3 files changed
+133
-6
No files matched your search
@@ -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 `<body>`. */
|
||||
.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;
|
||||
|
||||
@@ -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(<ButtonPickerHarness />);
|
||||
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();
|
||||
});
|
||||
});
|
||||
@@ -138,7 +138,10 @@ export const DropdownList = forwardRef<HTMLDivElement, DropdownListProps>(
|
||||
const searchRef = useRef<HTMLInputElement>(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<HTMLDivElement | null>(null);
|
||||
@@ -202,9 +205,12 @@ export const DropdownList = forwardRef<HTMLDivElement, DropdownListProps>(
|
||||
// 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<HTMLDivElement, DropdownListProps>(
|
||||
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<HTMLElement>(".opal-dropdown-card") ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Scrolls a row into view inside the list's own scroller only.
|
||||
* `scrollIntoView` also scrolls every ancestor of the portal, `<body>`
|
||||
* 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<HTMLElement>(
|
||||
".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;
|
||||
|
||||
Reference in new issue
Block a user