fix(admin): keep content list view state in the URL

[MA2153]

What does this PR do?

Reported by an editor and reproduced: on a collection's entry list, go to page 2 or 3, open an entry, then press the browser's back button — you land back on page 1.

page lived in ContentList's useState, and sort/search/status/author/date lived in ContentListPage's. Opening an entry unmounts the list, so back threw all of it away.

This moves the whole list view onto the route's search params, so back-navigation restores the view the editor left. List URLs become shareable and bookmarkable as a side effect.

• Defaults are never serialized. A pristine list is /content/posts with no query string; clearing a control drops its param again. Page is 1-based in the URL and omitted on page 1.
• Every filter/sort change resets the page — page 4 of one filter set indexes nothing in another, and the cursor chain restarts from a fresh first page anyway.
• History stays usable. Discrete clicks (page, sort, status, author) push an entry so back steps through them; the debounced search and the date inputs replace, since both fire on partial input rather than on commit.
• Params are validated, not trusted. ?sort=authorId, ?page=2.5, ?dateFrom=yesterday are dropped rather than forwarded to the API.

ContentList gains optional page/onPageChange and initialSearchQuery props, following the existing opt-in pattern used by sort/onSortChange and the filter callbacks. The component is exported from @emdash-cms/admin, so all three are additive — callers that don't pass them keep the current internal-state behavior and a working pager.

Not included: the All/Trash tab

activeTab is still useState in ContentList, so back always lands on All. It was out of scope for the reported bug and the trash tab has no pagination of its own.

The infrastructure does generalize to it — adding it later is the same three moves this PR already makes for page: a tab entry in parseContentListSearch, an activeTab/onTabChange controlled prop pair on ContentList mirroring page/onPageChange, and a page reset on tab switch. Happy to fold it in here if a reviewer would rather have it in one go.

Type of change

• Bug fix
• Feature (requires maintainer-approved Discussion)
• Refactor (no behavior change)
• Translation
• Documentation
• Performance improvement
• Tests
• Chore (dependencies, CI, tooling)

Checklist

• I have read CONTRIBUTING.md
• pnpm typecheck passes
• pnpm lint passes — pnpm lint:json reports 0 diagnostics
• pnpm test passes (or targeted tests for my change) — full packages/admin suite, 1251 passed
• pnpm format has been run
• I have added/updated tests for my changes (if applicable)
• User-visible strings in the admin UI are wrapped for translation (if applicable) — n/a, no new strings or markup; no messages.po changes included
• I have added a changeset (if this PR changes a published package)
• New features link to an approved Discussion — n/a, bug fix

AI-generated code disclosure

• This PR includes AI-generated code — model/tool: Claude Opus 5 (Claude Code)

Screenshots / test output

Failing tests written first, then the fix.

Four browser-level regression tests in tests/router.test.tsx — page survives back-navigation, filters and sort survive back-navigation, pristine URLs stay free of query params, and page + search term restore together. Nine parser unit tests in tests/router-search.test.ts, alongside the existing parseBylinesLocaleSearch ones.

Two notes for reviewers:

• My first draft of the pagination test passed against the broken code. The mocked ContentEditor in that file renders the entry title, so getByText("Post 21") matched the editor rather than the list. The assertions now target the page indicator and the row links. Worth knowing when adding tests to that file.
• The subtlest guard in the change is in handleSearchChange: ContentList reports its seeded search query once when the debounce settles on mount, and treating that as a user edit wipes the page just restored from the URL. I removed the guard, confirmed restores a page and a search term together fails, and put it back.

 Test Files  104 passed (104)
      Tests  1251 passed (1251)

Not verified by hand against a seeded site — the suite runs in Chromium and covers the round trip, but I did not drive the real admin through dev-bypass. No new UI strings or layout, so there is no RTL/Arabic surface to re-check.

🤖 Generated with Claude Code

[changeset-bot]

🦋 Changeset detected

Latest commit: c6cca1a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 17 packages

Name	Type
@emdash-cms/admin	Patch
emdash	Patch
@emdash-cms/cloudflare	Patch
@emdash-cms/sandbox-workerd	Patch
@emdash-cms/plugin-mcp-smoke	Patch
@emdash-cms/fixture-perf-site	Patch
@emdash-cms/perf-demo-site	Patch
@emdash-cms/cache-demo-site	Patch
@emdash-cms/do-demo-site	Patch
@emdash-cms/do-solo-demo-site	Patch
@emdash-cms/auth	Patch
@emdash-cms/blocks	Patch
@emdash-cms/gutenberg-to-portable-text	Patch
@emdash-cms/x402	Patch
create-emdash	Patch
@emdash-cms/auth-atproto	Patch
@emdash-cms/plugin-embeds	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[pkg-pr-new]

Open in StackBlitz

@emdash-cms/admin

npm i https://pkg.pr.new/@emdash-cms/admin@2246

@emdash-cms/auth

npm i https://pkg.pr.new/@emdash-cms/auth@2246

@emdash-cms/auth-atproto

npm i https://pkg.pr.new/@emdash-cms/auth-atproto@2246

@emdash-cms/blocks

npm i https://pkg.pr.new/@emdash-cms/blocks@2246

@emdash-cms/cloudflare

npm i https://pkg.pr.new/@emdash-cms/cloudflare@2246

@emdash-cms/contentful-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/contentful-to-portable-text@2246

emdash

npm i https://pkg.pr.new/emdash@2246

create-emdash

npm i https://pkg.pr.new/create-emdash@2246

@emdash-cms/gutenberg-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/gutenberg-to-portable-text@2246

@emdash-cms/plugin-cli

npm i https://pkg.pr.new/@emdash-cms/plugin-cli@2246

@emdash-cms/plugin-types

npm i https://pkg.pr.new/@emdash-cms/plugin-types@2246

@emdash-cms/registry-client

npm i https://pkg.pr.new/@emdash-cms/registry-client@2246

@emdash-cms/registry-lexicons

npm i https://pkg.pr.new/@emdash-cms/registry-lexicons@2246

@emdash-cms/registry-verification

npm i https://pkg.pr.new/@emdash-cms/registry-verification@2246

@emdash-cms/sandbox-workerd

npm i https://pkg.pr.new/@emdash-cms/sandbox-workerd@2246

@emdash-cms/x402

npm i https://pkg.pr.new/@emdash-cms/x402@2246

@emdash-cms/plugin-ai-moderation

npm i https://pkg.pr.new/@emdash-cms/plugin-ai-moderation@2246

@emdash-cms/plugin-atproto

npm i https://pkg.pr.new/@emdash-cms/plugin-atproto@2246

@emdash-cms/plugin-audit-log

npm i https://pkg.pr.new/@emdash-cms/plugin-audit-log@2246

@emdash-cms/plugin-color

npm i https://pkg.pr.new/@emdash-cms/plugin-color@2246

@emdash-cms/plugin-embeds

npm i https://pkg.pr.new/@emdash-cms/plugin-embeds@2246

@emdash-cms/plugin-field-kit

npm i https://pkg.pr.new/@emdash-cms/plugin-field-kit@2246

@emdash-cms/plugin-forms

npm i https://pkg.pr.new/@emdash-cms/plugin-forms@2246

@emdash-cms/plugin-webhook-notifier

npm i https://pkg.pr.new/@emdash-cms/plugin-webhook-notifier@2246

commit: c6cca1a

[emdashbot]

This is the right fix for the reported bug: moving the content-list view state into TanStack Router search params so opening an entry and using the browser back button restores the page, search, filters, and sort. The approach keeps the component backward-compatible by adding opt-in page/onPageChange and initialSearchQuery props to ContentList, and it validates query params before forwarding them to the API. I read the diff, the full changed files, and traced ContentList and its tests; the logic is sound, the pagination/sort/search wiring is consistent, regression tests are included (plus parser unit tests), and there is a package changeset.

No logic bugs, regressions, security issues, or AGENTS convention problems beyond comment discipline. The new router comments include PR-description-style paragraphs that belong in the PR body, not in the source file.

[MA2153]

Thanks — both comment notes addressed in 0d30ab6. The route-level block is now the suggested one-liner, and the page-reset paragraph is gone (the sort comment kept). I also trimmed the handleLocaleChange comment, which had the same PR-prose problem in the same diff. What is left is limited to non-obvious invariants: the updateSearch referential-stability constraint, the mount-time seeded-search guard, <input type="date"> firing per segment, and the 0-based/1-based page offset.

On the red Integration Tests check: it was unrelated to this PR, which touches only packages/admin. The failure was client/comments.test.ts timing out because its Astro dev server died during Vite pre-bundling with ENOTEMPTY ... node_modules/.vite/deps. Every per-suite server symlinks the same donor node_modules, so Vite's dep cache is shared across all of them and parallel servers race to re-optimize into it. Filed as #2247 with a suggested fix; it is a sibling of #1604 that the per-suite-fixture fix did not cover. The re-run on this branch is green.

[emdashbot]

This is the right fix for the reported bug: moving the content-list view state into TanStack Router search params so opening an entry and pressing back restores page, search, filters, and sort. The approach fits EmDash well — the new page/onPageChange and initialSearchQuery props are additive and keep ContentList backward-compatible, query keys are unchanged, query params are validated before being used, regression tests and parser unit tests are included, and there is a package changeset.

This is a re-review of my earlier COMMENTED review. No author reply was provided, and the comment-discipline issues I raised are still present in the current head. Beyond those source comments, I found no logic bugs, regressions, security issues, i18n/RTL leaks, broken API envelope handling, missing locale filtering, or new logged-out query work.

Two clear AGENTS.md comment violations remain and should be cleaned up before merge:

• packages/admin/src/router.tsx:313 has a PR-description sentence that just restates what the route/search schema already says.
• packages/admin/src/router.tsx:459 references issue #1288 in a prose comment.

Once those are trimmed, the code is good to land.

[emdashbot]

This is the right fix for the reported bug: moving the content-list view state (page, search, filters, sort, date range) into TanStack Router search params so opening an entry and pressing back restores the view the editor left. The approach fits EmDash well — the new page/onPageChange and initialSearchQuery props are additive and keep ContentList backward-compatible, query params are validated before use, defaults are omitted from the URL, and the regression tests cover the back-navigation round trip.

Since the prior COMMENTED review, one of the two comment-discipline issues has been cleaned up: the #1288 issue reference in router.tsx is gone. The other issue — the prose comment above ContentListSearch that restates the interface — is still present. I found no other logic bugs, regressions, security issues, i18n/RTL leaks, broken API envelope handling, missing locale filtering, or new logged-out query work.

Once the remaining restatement comment is removed, this is good to land.

[emdashbot]

This remains the right fix for the reported bug: moving content-list view state into TanStack Router search params so back-navigation restores the page/filters/sort/search the editor left. The additive page/onPageChange/initialSearchQuery props keep ContentList backward-compatible, defaults are omitted from the URL, params are validated before use, and the regression tests cover the back-navigation round trip.

Since the prior COMMENTED review, the #1288 issue-reference comment in router.tsx has been removed as noted. I rechecked the change for logic bugs, security (validated params, no new SQL), i18n/RTL surface (no new strings or layout), missing locale filtering, and extra logged-out query work; nothing new to report.

One AGENTS.md comment-discipline issue from the prior review is still present: the prose comment above ContentListSearch restates/justifies the interface design, which the file explicitly shows. Once that comment is removed, this is good to land.

[emdashbot]

This is the right fix for the reported bug: moving the content-list view state (page, search, filters, sort, date range) into TanStack Router search params means the browser back button restores the view an editor left. The additive, opt-in page/onPageChange/initialSearchQuery props keep ContentList backward-compatible, defaults are omitted from the URL, params are validated before use, page resets on filter/sort changes, and the regression tests cover the round-trip behavior. It fits EmDash's existing patterns and doesn't touch SQL, auth, or logged-out routes.

The prior review's one open item — a prose comment above ContentListSearch justifying the interface — has been removed, so that finding is resolved.

Remaining issues are mostly AGENTS.md comment-discipline cleanup:

• Several newly-added comments in packages/admin/src/router.tsx and packages/admin/tests/router.test.tsx justify design decisions, restate what the code already shows, or narrate the original bug/reviewer pitfall. AGENTS.md is explicit that comments should be for future readers, not PR summaries or reviewer messages. The line-anchored findings call out representative examples (lines 400-403, 416-417, 438-439, 718-719 in router.tsx and 809-811, 822-823, 840-841 in router.test.tsx). There are similar instances nearby (e.g., the dir/sort rationale at line 354-355) that should be trimmed in the same pass.

• A smaller correctness concern: ContentList seeds searchQuery from initialSearchQuery only on mount. If the route's q search param changes while the list is already mounted — for example, an in-app deep link from another page to /content/posts?q=foo — the input box stays at the old value and the debounced effect can push that stale value back into the URL. The current tests don't exercise external q changes while mounted. Consider making the search input react to URL-driven changes without breaking the debounced typing UX.

Recommended verdict is comment: the fix is sound and the prior open item is addressed, but the new comments need the cleanup pass described in AGENTS.md before landing.

[MA2153]

Addressed in 3ee2029.

Comment cleanup — removed all seven flagged comments, plus the dir/sort rationale in parseContentListSearch and the "changing a filter restarts the cursor chain" line above statusFilter that the review body pointed at.

Two comments in the test file I kept, since they document a footgun rather than the change:

• The one above the first page-indicator assertion, trimmed to two lines. The mocked ContentEditor in this file renders the entry title, so getByText("Post 21") passes while sitting on the edit screen — a future test author in this file gets this wrong by default, and my first draft did.
• The one explaining the 800ms wait in "restores a page and a search term together". Without it the sleep looks arbitrary.

The q staleness bug is real, and reachable by the back button — not just deep links. ContentList stays mounted across search-param changes, so: page 2 → type a query → back lands on the ?page=2 entry with no q, while the search box still shows the term over unfiltered results.

Failing test first (follows a search term that changes in the URL while the list stays mounted), then the fix.

I didn't move the debounce up to ContentListPage. A URL-controlled input means every keystroke round-trips through navigate before it renders, which is exactly the lag the internal state exists to avoid. Instead the list now tracks the term it last reported and adopts the caller's value only when the two differ — an external change syncs the input, the caller echoing back our own query doesn't reset it mid-word. initialSearchQuery is renamed to searchQuery, since it's no longer mount-only; the prop is new in this PR, so nothing released depends on the old name.

pnpm lint:json 0 diagnostics, pnpm typecheck clean, packages/admin suite 1252 passed.

[emdashbot]

This is still the right fix: moving the content-list view state into TanStack Router search params makes back-navigation restore the editor’s view, keeps the component backward-compatible through additive controlled props, validates params before forwarding them, and resets page on filter/sort changes. I re-read the changed files and the diff; the prior open item about the search input not reacting to external q changes while mounted is now addressed by a sync effect in ContentList and a regression test.

The only remaining issue from the prior review is the AGENTS.md comment-discipline cleanup that was requested before landing. A few comments introduced or left by this PR still justify design decisions, narrate the original test pitfall, or duplicate PR-description notes. I’ve line-anchored the representative instances; removing them is a straightforward cleanup pass with no behavior change. No logic bugs, regressions, security issues, or missing tests were found. The changeset, exports, prop typings, and test coverage look good.

[MA2153]

Ok, at this point the bot is just nitpicking

[khoinguyenpham04]

Thank you for tackling this. Could you please address these comments.

[khoinguyenpham04]

Could we avoid replaying the full cursor chain for deep page URLs? On a fresh load, ?page=500 fetches every preceding API page and retains all rows. Could we bound the automatic loading or use cursor-backed URL state?

[MA2153]

Agreed that this is worth solving, but I don't think it should be solved in this PR — and I don't think bounding the loading is the right shape either. Opened #2255 for it.

Short version of what I found while looking into it. The replay is a symptom of the URL storing an ordinal when we already have an anchor: encodeCursor produces { orderValue, id } and findMany applies it as a comparison, never a row lookup, so it stays valid even if the anchored row is deleted. ?after=<cursor> reads limit rows at page 2 and page 500 alike, restores in one request on back-navigation, and holds its window steady while other editors save — which the current design and offset pagination both fail to do.

It also collapses the two-tier pagination that causes the replay: the API pages at 100 while the table slices at 20, which is the only reason the auto-chaining effect exists. Match those and one URL page is one API page.

Two things it needs that aren't free — a before cursor in findMany for the ← button on a cold-loaded deep link, and a decision about the page-number label, since an anchor doesn't know it's the twelfth page. Both are in the Discussion.

Cursor-backed URL state is your other suggestion, so I think we agree on the direction; I'd rather get a maintainer read on the label trade-off before rewriting the list's pagination model.

On this PR specifically: the cost is bounded even today. totalPages comes from the server's total and clampedPage pins the page to it, so ?page=999999 settles onto the last real page after one fetch. What's left is a genuinely large collection reached by a deep bookmark, which is unchanged from what clicking there already costs.

[MA2153]

Moved this to draft pending a maintainer call on #2255.

Both of the local findings from the review are fixed in c6cca1a, each with a failing test first: readCalendarDate now rejects dates that fit YYYY-MM-DD but never happened, and a search term still inside the debounce no longer overwrites the page a Back press just restored.

What's holding it is the third comment. Chasing it turned up something bigger than the fix it asked for: the deep-page replay is a symptom of this PR storing an ordinal in the URL when the API already hands out a stable anchor. If #2255 lands the anchor direction, ?page=2 stops being the URL contract for this route and becomes ?after=<cursor> — so merging this as-is would ship a query-param shape that a follow-up immediately changes, and admin URLs people have started bookmarking would break twice instead of once.

The alternatives I weighed and rejected: capping the restorable page in the parser clamps whether or not the rows are already cached, so a legitimate back-navigation from a deep page lands on the wrong one — the same class of bug this PR exists to fix. Bounding the auto-chaining by loaded data avoids that, but it's a real change to the list's pagination model, which is the thing #2255 is asking about.

So the choice is which URL shape this route gets, and that's a maintainer's call rather than something to settle inside a bug-fix PR. Happy to mark it ready again unchanged if the answer is "ship the page number, revisit later" — the reported back-button bug is fixed either way, and the deep-page cost is bounded today by the total-based clamp.