Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 104 additions & 0 deletions apps/app/src/components/ui/resource-pagination.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
// @vitest-environment jsdom

import { cleanup, fireEvent, render, screen } from "@testing-library/react";
import { useResourcePagination } from "@bb/shared-ui/resource-pagination";
import { afterEach, describe, expect, it } from "vitest";

const ROWS = Array.from({ length: 30 }, (_, index) => index + 1);
const SELECTABLE_PAGES = [0, 1, 2];

function Probe({
pageSize,
resetKey,
rowCount = ROWS.length,
}: {
pageSize: number;
resetKey?: string;
rowCount?: number;
}) {
const pagination = useResourcePagination(ROWS.slice(0, rowCount), {
pageSize,
resetKey,
});
return (
<>
<span data-testid="page">{pagination.page}</span>
<span data-testid="rows">{pagination.items.join(",")}</span>
{SELECTABLE_PAGES.map((page) => (
<button
key={page}
type="button"
onClick={() => pagination.setPage(page)}
>
{`go to ${page}`}
</button>
))}
</>
);
}

function selectedPage(): number {
return Number(screen.getByTestId("page").textContent);
}

function visibleRows(): number[] {
const rows = screen.getByTestId("rows").textContent ?? "";
return rows === "" ? [] : rows.split(",").map(Number);
}

function goToPage(page: number): void {
fireEvent.click(screen.getByRole("button", { name: `go to ${page}` }));
}

afterEach(() => {
cleanup();
});

describe("useResourcePagination", () => {
/**
* The selected page used to be mirrored back into state from an effect. That
* discarded the page the user picked as soon as a measured page size changed,
* and — because the write-back carried a pre-interaction page — a click that
* beat the effect was silently reverted. Surviving this round trip is the
* observable proof that nothing but setPage writes the selection.
*/
it("rescales the selection across page-size changes instead of overwriting it", () => {
const { rerender } = render(<Probe pageSize={10} />);
goToPage(1);
expect(visibleRows()).toEqual(ROWS.slice(10, 20));

// Row 11 stays in view at the larger page size...
rerender(<Probe pageSize={15} />);
expect(selectedPage()).toBe(0);
expect(visibleRows()).toEqual(ROWS.slice(0, 15));

// ...and remeasuring back restores the page the user actually chose.
rerender(<Probe pageSize={10} />);
expect(selectedPage()).toBe(1);
expect(visibleRows()).toEqual(ROWS.slice(10, 20));
});

it("resets to the first page for each new projection", () => {
const { rerender } = render(<Probe pageSize={10} resetKey="all" />);
goToPage(2);
expect(selectedPage()).toBe(2);

rerender(<Probe pageSize={10} resetKey="filtered" />);
expect(selectedPage()).toBe(0);

// Returning to an earlier projection is still a new projection, not a
// reason to resurrect the page that was selected under it.
rerender(<Probe pageSize={10} resetKey="all" />);
expect(selectedPage()).toBe(0);
});

it("clamps the page when live data shrinks past it", () => {
const { rerender } = render(<Probe pageSize={10} />);
goToPage(2);
expect(visibleRows()).toEqual(ROWS.slice(20, 30));

rerender(<Probe pageSize={10} rowCount={12} />);
expect(selectedPage()).toBe(1);
expect(visibleRows()).toEqual(ROWS.slice(10, 12));
});
});
43 changes: 20 additions & 23 deletions packages/shared-ui/src/components/ui/resource-pagination.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ export function useResourceViewportPageSize(
* Client-side pagination for the bounded arrays returned by resource APIs.
* The selected page resets with the projection and clamps when live data
* shrinks, while retaining the current page across ordinary data refreshes.
* A changing page size rescales the selection to keep the same rows in view
* rather than resetting it.
*/
export function useResourcePagination<Item>(
items: readonly Item[],
Expand All @@ -142,35 +144,30 @@ export function useResourcePagination<Item>(
);
const resetKey = options.resetKey ?? "";
const pageCount = Math.max(1, Math.ceil(items.length / pageSize));
const [pageState, setPageState] = useState({
resetKey,
page: 0,
pageSize,
});
// Anchors the selected page to the page size it was selected under. Only an
// explicit setPage or a new projection writes it; the rendered page is derived
// from it. Mirroring the derived page back from an effect used to drop clicks:
// a viewport measurement that changed pageSize left a queued write-back
// holding the pre-click page, which then overwrote the interaction.
const [anchor, setAnchor] = useState({ resetKey, page: 0, pageSize });

// A new projection starts at its first page. This adjusts state during render
// rather than from an effect so the reset cannot land after an interaction.
if (anchor.resetKey !== resetKey) {
setAnchor({ resetKey, page: 0, pageSize });
}

const requestedPage =
pageState.resetKey !== resetKey
anchor.resetKey !== resetKey
? 0
: pageState.pageSize === pageSize
? pageState.page
: Math.floor((pageState.page * pageState.pageSize) / pageSize);
: anchor.pageSize === pageSize
? anchor.page
: Math.floor((anchor.page * anchor.pageSize) / pageSize);
const page = Math.min(requestedPage, pageCount - 1);

useEffect(() => {
setPageState((current) => {
if (
current.resetKey === resetKey &&
current.page === page &&
current.pageSize === pageSize
) {
return current;
}
return { resetKey, page, pageSize };
});
}, [page, pageSize, resetKey]);

const setPage = useCallback(
(nextPage: number) => {
setPageState({
setAnchor({
resetKey,
page: Math.max(0, Math.min(Math.floor(nextPage), pageCount - 1)),
pageSize,
Expand Down
29 changes: 25 additions & 4 deletions plugins/workflows/src/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,34 @@ const DOCUMENTATION_EXTENSIONS = new Set([
".tsx",
]);
const IGNORED_DOCUMENTATION_DIRECTORIES = new Set([
".git",
".turbo",
"coverage",
"dist",
"node_modules",
]);

/**
* This walks the whole checkout while the `packages` CI shard runs every other
* package's tests concurrently, so it has to ignore paths those tests own.
* Dot-directories cover VCS internals, tool caches, and the scratch trees
* siblings create inside their own package roots — the plugin registry's
* `.vendor-fixture-*` and agent-runtime's `.bb-codex-outside-*`. None are
* project documentation, and descending into them both races their cleanup and
* can report a generated copy of a file instead of its real source.
*/
function isScannableDirectory(name: string): boolean {
return !name.startsWith(".") && !IGNORED_DOCUMENTATION_DIRECTORIES.has(name);
}

/** Paths a concurrent test deletes mid-walk are not project documentation. */
function readIfPresent(path: string): string {
try {
return readFileSync(path, "utf8");
} catch (error) {
if ((error as NodeJS.ErrnoException).code === "ENOENT") return "";
throw error;
}
}

function documentationFiles(root: string): string[] {
const files: string[] = [];
const pending = [root];
Expand All @@ -32,7 +53,7 @@ function documentationFiles(root: string): string[] {
if (entry.isSymbolicLink()) continue;
const path = resolve(directory, entry.name);
if (entry.isDirectory()) {
if (!IGNORED_DOCUMENTATION_DIRECTORIES.has(entry.name)) {
if (isScannableDirectory(entry.name)) {
pending.push(path);
}
} else if (
Expand Down Expand Up @@ -194,7 +215,7 @@ describe("workflows CLI argument validation", () => {
const root = resolve(process.cwd(), "../..");
const removedCommand = ["bb workflows", "catalog"].join(" ");
const matches = documentationFiles(root)
.filter((path) => readFileSync(path, "utf8").includes(removedCommand))
.filter((path) => readIfPresent(path).includes(removedCommand))
.map((path) => relative(root, path))
.sort();
expect(matches).toEqual([]);
Expand Down
Loading