fix(ui): the page's end scrolls out from under Back to top
The button floats over the bottom-right corner, and at the end of a long table it covered the closing pager's '100' and 'All'. While it is up, PageScroller ends the page a button's height later (only then, so a full-height layout gets no scrollbar). Its shown state is a shared store read through a use… wrapper: a store's .use() called straight from a component is memoized by the React Compiler and never re-renders (contract: store.contract.test.ts).
This commit is contained in:
@@ -1,9 +1,10 @@
|
||||
import { useEffect, useRef, useState } from "react";
|
||||
import { useEffect, useRef } from "react";
|
||||
import { createPortal } from "react-dom";
|
||||
import { ArrowUp } from "lucide-react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { Pressable } from "./Pressable";
|
||||
import { SURFACE } from "./surfaces";
|
||||
import { backToTopShown, useBackToTopShown } from "./backToTopState";
|
||||
|
||||
// A floating "back to top" button that fades in once the current page's scroll container is
|
||||
// scrolled past a threshold, and smooth-scrolls it back to the top. Every page scrolls inside a
|
||||
@@ -12,7 +13,8 @@ import { SURFACE } from "./surfaces";
|
||||
// viewport-relative regardless of transformed ancestors.
|
||||
export default function BackToTop({ dep, threshold = 600 }: { dep?: unknown; threshold?: number }) {
|
||||
const { t } = useTranslation();
|
||||
const [show, setShow] = useState(false);
|
||||
// Shared, not local: PageScroller keeps the page's end clear of the button while it is up.
|
||||
const show = useBackToTopShown();
|
||||
const scrollerRef = useRef<HTMLElement | null>(null);
|
||||
|
||||
// Listen on the document in the CAPTURE phase and read the scroller off the event, rather than
|
||||
@@ -26,7 +28,7 @@ export default function BackToTop({ dep, threshold = 600 }: { dep?: unknown; thr
|
||||
const el = e.target as HTMLElement | null;
|
||||
if (!el || el.tagName !== "MAIN") return;
|
||||
scrollerRef.current = el;
|
||||
setShow(el.scrollTop > threshold);
|
||||
backToTopShown.set(el.scrollTop > threshold);
|
||||
};
|
||||
document.addEventListener("scroll", onScroll, { passive: true, capture: true });
|
||||
return () => document.removeEventListener("scroll", onScroll, { capture: true });
|
||||
@@ -38,8 +40,7 @@ export default function BackToTop({ dep, threshold = 600 }: { dep?: unknown; thr
|
||||
useEffect(() => {
|
||||
const el = document.querySelector("main");
|
||||
scrollerRef.current = el;
|
||||
// eslint-disable-next-line react-hooks/set-state-in-effect -- reading the DOM is the point — <main> is swapped by the page switch above us, and only after that commit can its scrollTop be read at all. Nothing derivable in render knows where the new page is scrolled to.
|
||||
setShow(!!el && el.scrollTop > threshold);
|
||||
backToTopShown.set(!!el && el.scrollTop > threshold);
|
||||
}, [dep, threshold]);
|
||||
|
||||
// A keyboard press hides the button it was on: the focus goes to the page it scrolled (its <main>
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
// through window.history.state (what ensureNavId/peekNavId read) — firing the nav bus (notifyNav)
|
||||
// stands in for a Back/Forward that doesn't re-render the scroller.
|
||||
import { describe, it, expect, vi, beforeAll, beforeEach, afterEach } from "vitest";
|
||||
import { render, fireEvent, cleanup } from "@testing-library/react";
|
||||
import { render, fireEvent, cleanup, act } from "@testing-library/react";
|
||||
|
||||
vi.mock("react-i18next", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("react-i18next")>();
|
||||
@@ -15,6 +15,7 @@ vi.mock("react-i18next", async (importOriginal) => {
|
||||
});
|
||||
|
||||
import PageScroller from "./PageScroller";
|
||||
import BackToTop from "./BackToTop";
|
||||
import { notifyNav } from "../../lib/history";
|
||||
|
||||
beforeAll(() => {
|
||||
@@ -97,3 +98,27 @@ describe("PageScroller scroll memory", () => {
|
||||
expect(main.scrollTop).toBe(300);
|
||||
});
|
||||
});
|
||||
|
||||
// E28 UAT: Back to top floats over the bottom-right corner, and at the end of a long table it covered
|
||||
// the closing pager's "100" and "All". While it is up, the page ends a button's height later.
|
||||
describe("PageScroller · room for Back to top", () => {
|
||||
it("ends the page later while the button is up, and not otherwise", () => {
|
||||
const { container } = render(
|
||||
<>
|
||||
<PageScroller scrollKey="room">
|
||||
<p>rows</p>
|
||||
</PageScroller>
|
||||
<BackToTop />
|
||||
</>,
|
||||
);
|
||||
const main = container.querySelector("main")!;
|
||||
const spacer = () => main.querySelector(':scope > [aria-hidden="true"]');
|
||||
expect(spacer()).toBeNull();
|
||||
main.scrollTop = 900;
|
||||
act(() => void main.dispatchEvent(new Event("scroll")));
|
||||
expect(spacer()?.className).toBe("h-18");
|
||||
main.scrollTop = 0;
|
||||
act(() => void main.dispatchEvent(new Event("scroll")));
|
||||
expect(spacer()).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,6 +2,7 @@ import { useCallback, useLayoutEffect, useRef, useState, type ReactNode } from "
|
||||
import { useScrollFade } from "../../lib/useScrollFade";
|
||||
import { FOCUS_REST_ATTRS, useFocusKeeper } from "../../lib/useFocusKeeper";
|
||||
import { StickyTopProvider } from "./stickyTop";
|
||||
import { useBackToTopShown } from "./backToTopState";
|
||||
import { CONTENT_ID } from "./landmarks";
|
||||
import { ensureNavId, peekNavId, subscribeNav } from "../../lib/history";
|
||||
|
||||
@@ -93,6 +94,7 @@ export default function PageScroller({
|
||||
return () => setStickyTops((n) => n - 1);
|
||||
}, []);
|
||||
const fade = useScrollFade(20, stickyTops === 0);
|
||||
const fabUp = useBackToTopShown();
|
||||
const fadeRef = fade.ref;
|
||||
const elRef = useRef<HTMLElement | null>(null);
|
||||
// Stable identity: an inline ref callback changes every render, which makes React detach and
|
||||
@@ -169,6 +171,10 @@ export default function PageScroller({
|
||||
className="flex-1 min-w-0 overflow-y-auto scrollbar-gutter-stable focus:outline-none"
|
||||
>
|
||||
<StickyTopProvider value={registerStickyTop}>{children}</StickyTopProvider>
|
||||
{/* While Back to top floats over the corner, the page ends a button's height (3rem + its 1.25rem
|
||||
inset) later, so its last row or pager can scroll out from under it. Only then: a spacer
|
||||
always there would give a full-height layout a scrollbar it does not need. */}
|
||||
{fabUp && <div aria-hidden="true" className="h-18" />}
|
||||
</main>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,10 @@
|
||||
import { createStore } from "../../lib/store";
|
||||
|
||||
// Whether the Back-to-top button is up. It floats over the page's bottom-right corner, so PageScroller
|
||||
// reads this to keep the END of the page clear of it while it shows: a table's closing pager sat under
|
||||
// the button — "100" and "All" covered (E28 UAT, the channel manager on a short window).
|
||||
export const backToTopShown = createStore(false);
|
||||
|
||||
export function useBackToTopShown(): boolean {
|
||||
return backToTopShown.use();
|
||||
}
|
||||
@@ -0,0 +1,25 @@
|
||||
import { readdirSync, readFileSync } from "node:fs";
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
// A store's `.use()` is a hook the React Compiler cannot see: its name is a method's, not `use…`'s.
|
||||
// Called straight from a COMPONENT, the compiled render memoized its result and never re-rendered on
|
||||
// a change — Back to top stayed hidden (E28 UAT, measured under vitest's compiler build). Inside a
|
||||
// function named `use…` it is compiled as a hook and works, so that is the only place it may be read.
|
||||
describe("a store's use()", () => {
|
||||
it("is read only inside a function named use…", () => {
|
||||
const root = new URL("../", import.meta.url);
|
||||
const offenders: string[] = [];
|
||||
for (const f of readdirSync(root, { recursive: true, encoding: "utf8" })) {
|
||||
if (!/\.tsx?$/.test(f) || f.includes(".test.")) continue;
|
||||
const src = readFileSync(new URL(f.replace(/\\/g, "/"), root), "utf8");
|
||||
for (const m of src.matchAll(/\b\w+\.use\(\)/g)) {
|
||||
const before = src.slice(0, m.index);
|
||||
const owners = [...before.matchAll(/(?:function\s+(\w+)|const\s+(\w+)\s*=\s*\()/g)];
|
||||
const owner = owners.at(-1);
|
||||
const name = owner?.[1] ?? owner?.[2] ?? "";
|
||||
if (!name.startsWith("use")) offenders.push(`${f}: ${m[0]} in ${name || "?"}`);
|
||||
}
|
||||
}
|
||||
expect(offenders).toEqual([]);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user