Ported the posts and pages lists to React behind a flag - #29785
Open
peterzimon wants to merge 80 commits into
Open
Ported the posts and pages lists to React behind a flag#29785peterzimon wants to merge 80 commits into
peterzimon wants to merge 80 commits into
Conversation
no ref
First phase of porting the Ember posts and pages list screens to React. Both
implementations live on the same URLs and the flag picks one, so we can compare
them side by side and fall back instantly if the React version has problems.
The flag is private and defaults to off, so nothing changes for users yet -
hence no emoji.
Gating a route needs both halves, or both implementations end up in the DOM:
React renders its screen instead of EmberFallback, and the Ember route aborts
its own transition so its subtree never renders and its three infinity models
never fire.
Aborting is only half the job for a route other code navigates *to*, which is
where this differs from the memberDetailsReact precedent - nothing transitions
into the member route, but the publish flow calls transitionTo('posts') four
times and two breadcrumbs link to it. Ember and React share the location hash,
and an aborted transition never reaches updateURL, so those navigations would
be silent no-ops. PostsRoute now supplies the URL itself for named transitions,
and leaves URL-initiated ones alone so query params - which is how saved views
are addressed - survive.
Reused FlagGatedRoute rather than copying member-detail-gate: it already owns
the loading/error/flag semantics, and only needed an optional fallback, since
these routes must keep the gift-link modal host mounted next to Ember.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
no ref Verifies the postsListReact swap in the real admin app, booted in a real browser against mocked APIs: the React screen serves /posts and /pages only when the flag is on, each route gets its own resource, and the Ember list's testids are absent so the two trees can never both be live. Written because verifying this by hand needed a working dev server and a logged-in session, which made it slow and unrepeatable - and it is exactly the kind of thing that should keep being checked in CI rather than once. The Ember half of the handshake is covered separately in the ember-admin acceptance suite, since there is no Ember app in this harness. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
no ref Ports the filter-building half of apps/ember-admin/app/routes/posts.js: type to status mapping, the NQL builder, the three query buckets and their per-bucket default sorts. Two things here are load-bearing beyond "it fetches posts". The list is three queries, not one, drained scheduled then drafts then published - drafts sort by updated_at because they have no publish date. And buildAllFilter also feeds the inverted "select all" filter that bulk delete runs server-side against posts that were never loaded, so its exact output matters. The chip codec is deliberately separate from @/shared/filters' NQL engine. Members round-trips one ?filter= string; posts round-trips five discrete params because that is what sidebar saved views persist and what the Ember screen reads. Unknown values are carried through verbatim rather than dropped - a saved view can point at a since-renamed tag, and rewriting the URL under the user would corrupt their view. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
no ref The React list now fetches the right posts for any URL. Still bare titles - this is about the data being right, not the design. The list is three queries, not one: scheduled, drafts, published+sent, each with its own default sort because drafts have no published_at and sort by when they were last touched. All three fire at once; what is sequenced is rendering, so a bucket only appears once every earlier one has loaded every page. That rule is a pure function so it can be tested with plain data instead of mocked queries - including the case that actually matters day to day, where a site has nothing scheduled and drafts must appear immediately rather than waiting. usePostsList calls both resource hooks per bucket and gates them with `enabled` rather than picking one, since a hook chosen at runtime breaks the rules of hooks. The URL is never rewritten on hydration, which is a deliberate difference from the members equivalent: a posts URL *is* a saved view's identity, so canonicalising one we merely parsed would corrupt the view and desync from the Ember screen reading the same params. Unrecognised values are carried through for the same reason. Dropped formats=mobiledoc,lexical from the list request. Ember sends it, which pulls whole post bodies into a 30-row list; the server's default relations already attach what the list renders. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
no ref I claimed omitting formats=mobiledoc,lexical avoids pulling whole post bodies into a 30-row list. It does not: defaultFormat in the posts input serializer fills the same value in server-side when the client leaves it out, so the request is byte-identical to Ember's either way. Omitting `include` is still right, and now says why: defaultRelations attaches exactly what the list renders, but only while neither include nor columns is set - so reaching for `columns` to trim the payload would silently drop tags, authors, email and click counts. Trimming the bodies out is still worth doing, but it needs an explicit narrower formats, which is a separate change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
no ref Five issues a review pass found, all in the phase just committed: The composer held the entire list until every bucket had answered, so one slow query hid everything and the list could sit in a loading state indefinitely. Only *later* buckets depend on earlier ones, so it now walks in order and shows each bucket as soon as it has answered, stopping at the first that is still loading or still has pages. A bucket that errored no longer counts as drained either - treating it as exhausted let the next bucket silently take its place. Authors and contributors were not scoped to their own posts: the plumbing existed but the screen never passed the current user, so the author URL param won instead. Ember forces the signed-in user's slug for those roles. Nothing drove pagination, so a site with more than 30 scheduled posts showed one page and never reached its drafts. Wired up a plain load-more button; the virtualised scroll replaces it next phase. Blank handling diverged from Ember's isBlank on whitespace-only values, so ?tag=%20%20 produced `tag: ` here and no clause there. These strings are compared against saved views and run server-side by bulk delete, so they have to agree exactly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
no ref The previous commit made the composer release as soon as the first bucket answered. That was wrong, and wrong in a way that bites almost everywhere: most sites have zero scheduled posts, so that bucket answers first and instantly, and the list was declared loaded and complete with two queries still in flight. Today that renders an empty list; once the real empty state lands it would flash "Start creating content" on nearly every page load, and any infinite scroll driven off hasNextPage would be told there is nothing more to fetch. Ember waits for all three: the route returns RSVP.hash of the three models and shows a skeleton until they resolve. Restored that, and wrote down why, since releasing earlier is a tempting change to make twice. The sequential-drain rule and the fix for errored buckets are unaffected - they now apply once everything has answered. Also corrected a comment that claimed the shared testids already make the e2e page object work against both implementations. They do not yet: getPostByTitle matches an h3 the React row has not got, and waitForPageToFullyLoad expects the list element in the empty state. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
no ref Every string a post row renders, ported from the Ember row template and its helpers: the byline and primary tag, which date field applies, the status label, and the detail Ember reveals on hover. Kept out of the component deliberately. This is where parity actually lives - the wording and the conditions behind it are far easier to get subtly wrong than the layout, and far harder to eyeball. A few that would not survive a visual check: an email-only post that failed says "Failed to send newsletter" rather than "Sent"; a scheduled email-only post is never described as "published"; drafts and scheduled posts show when they were last touched because they have no publish date. formatPostTime is its own module because the branches are order-dependent and the order is load-bearing - anything within twelve hours either way reads as relative and beats every absolute format, and "yesterday" is checked before "tomorrow" because published posts vastly outnumber scheduled ones. `now` is injectable so those branches can be tested without freezing the clock; writing the tests immediately caught that from midday, nothing later the same day is more than twelve hours away. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
no ref The list now looks like a list: feature image or placeholder, featured star, title, "By <authors> in <tag> - <date>", and the status line, with the detail Ember reveals on hover done as a group-hover so it needs no JS state. Contributors get a link out to the live post for published posts they can no longer edit, matching Ember. Both empty states are ported: the cold start invites you to write, the filtered one offers a way back and clears every param. Sorting deliberately does not count as filtering - Ember excludes `order` from that check, so re-sorting an empty list still offers "write your first post" rather than "clear your filters". Writing the tests turned up two locator traps worth noting: role-name matching is substring-based, so "New post" also matched the empty state's "Write a new post"; and because the fakes do not implement NQL, an unfiltered render serves the same posts to all three status queries, so any count assertion has to scope itself to one bucket. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
no ref text-green and text-red were raw palette aliases; Shade registers --color-state-success and --color-state-danger for exactly this, and they resolve to the same green-500 and red-500 Ember's CSS uses. Draft keeps the pink alias: it is not success, warning or danger, so there is no semantic token for it, and pink is what Ember shows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
no ref Published and sent rows were green. Ember colours exactly three states - draft pink, scheduled green, error red - and published/sent have no rule at all, so they inherit the muted grey of .gh-content-entry-status. That is most rows on a real site, so painting them green changed the whole feel of the screen. I had cited that stylesheet as saying the opposite. Returning from the editor left you with no sidebar. Aborting a transition means the route you came FROM never deactivates, so the editor's teardown - which clears full-screen mode, the thing the React shell reads to decide whether to show the sidebar - never ran. The hover detail was rendered always and merely faded with opacity, so it stayed in the accessibility tree: a screen reader read every scheduled row's full dispatch details aloud on every row. Now mounted only while hovered, as Ember does. "Show all posts" cleared the sort as well as the filters; Ember's link deliberately leaves `order` alone. The acceptance test asserted the wrong behaviour, so it was locking the bug in. Also: a failed email no longer reddens a draft (un-publishing leaves the email record attached, and Ember gates on status); pages can no longer be described as sent; empty-state copy is verbatim from the Ember templates, terminal full stops included, with the invented descriptions removed; the date tooltip regained its "Updated"/"Published" prefix; the meta separator is a hyphen, not an en dash; and the featured star uses a semantic token. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
no ref Filtering now uses the same Shade Filters pattern as the Members list, driving the same five URL params the Ember screen reads and sidebar saved views persist. The awkward part is that the URL carries slugs but a chip has to read as a name. Author and tag use createGhostBrowseValueSource, whose hydrate step exists for exactly this: opening a saved view whose tag is not in the first page of results, where the chip would otherwise show a bare slug. A slug that resolves to nothing keeps showing the slug rather than being dropped - a saved view can point at a since-renamed tag, and quietly rewriting the URL would corrupt it. allowMultiple is off because each field maps to one URL param, so a second chip on the same field would be unrepresentable. Sort is its own dropdown, not a chip. It has no operator, so "Sort is Newest first" would read as nonsense, and order also feeds each status bucket's default ordering, which is data plumbing rather than filtering. "Newest first" is the absence of the param, not a value. Contributors get only the type filter and authors lose the author filter, since both are already scoped to their own posts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
no ref The comment said allowMultiple was off; the prop was never passed, and Shade defaults it to true. Each field here maps to one URL param holding one value, and the serializer keeps the last - so a user could sit looking at two "Post type" chips while only one of them was in the URL, and a saved view built from that would silently lose the other. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
no ref A filter value that resolves to nothing rendered as "Select…" - the value disappeared from the UI while staying in the URL, so a saved view pointing at a deleted tag showed an empty list with no indication why. Now shows "Unknown tag" / "Unknown author" / "Unknown type" as Ember does. The test that was meant to cover this only asserted the URL survived, never that anything was shown, so it passed against the broken behaviour; a failure screenshot from the review is what caught it. The Author dropdown had a permanent "Load more" that refetched page one forever: useBrowseUsers returned a next-page param unconditionally, so hasNextPage was always true. Every other resource guards on pagination.next - this one didn't, and the author filter is its first consumer to route through the value-source machinery. The sort menu used a bare aria-label of "Sort", which overrides the button text so assistive tech never heard which sort was active, and its items carried no selected state. Now radio items with the value in the label. "Show all posts" pushed a history entry. Ember forces replace for any posts-to-posts transition, so back should leave the screen rather than return to the filtered view - and the FilterBar's own Clear button was already replacing, so the two were inconsistent. Also four TypeScript errors that `tsc --noEmit -p tsconfig.json` missed but `tsc -b` catches, which is the command the build actually runs; and the author hydration test passed with hydration removed, because the passthrough fake served the same user from the plain browse. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
no ref
Turning the flag on killed every active state in the Posts nav - the item
itself, Drafts, Scheduled, Published, and all saved views. Nothing errored;
they just stopped highlighting. The sidebar derived active state from the Ember
routing bridge, which reads Ember's current route name, and the Ember route
aborts when React owns the URL, so that name is never "posts" any more.
Active state now comes from React's own location when the flag is on, and from
the bridge when it is off, so both implementations keep working. Both are
computed every render and only the result is selected - hooks cannot be called
conditionally and the flag can flip at runtime.
A view matches only when all five params agree, as Ember's activeView does
after dropping nulls: a view of {type: draft} is not active on
?type=draft&tag=news, that is a different view. Sort counts too. The parent
item highlights only when no view underneath does.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
no ref Clicking Posts now returns you to the filters you last had, as it does in Ember. Three rules, ported from state-bridge's getRouteUrl: already on the route gives a bare URL, so a second click goes home; otherwise the last params for that route come back; unless those params are exactly a saved or default view, because then clicking Posts would silently drop you into whichever view you had open last. Ember reads this off its live controller query params. React has no equivalent long-lived controller, so the list screen reports its params as they change. Kept in module scope rather than sessionStorage - Ember's is in-memory and per-tab, so persisting it would be a behaviour change rather than a port. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
no ref
The bookmark button in the filter bar creates and edits saved views, gated as
Ember gates it: admins only, posts only (never pages), not while a default view
is active, and only with something actually filtered - where a sort on its own
counts, unlike the empty state's "showing all" check.
Records are written in exactly Ember's shape, so a view saved here appears in
the Ember sidebar and vice versa while both implementations exist. Verified
against a real save: {"name":"Blog posts","route":"posts","color":"orange",
"filter":{"tag":"blog"}}.
Views for every screen share one shared_views setting, so a save round-trips
the whole list rather than just the posts entries - otherwise saving a post
view would delete the member ones.
Caught by hand while testing: the button never appeared for the site owner.
Ember's isAdmin is or(isOwnerOnly, isAdminOnly), but the framework's
isAdminUser is Administrator only. hasAdminAccess is the equivalent, and the
owner is the most likely person to be saving views in the first place.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
no ref Rows now carry the right-hand metrics, each linking into its analytics tab. Which columns appear is a pure function with an exhaustive test table, because the rules are fiddly and invisible unless a site happens to be configured a particular way: Sent is a fallback, shown only when neither Opens nor Clicks is - so turning off both tracking settings makes a column appear rather than disappear. The email columns need tracking enabled both site-wide and on the individual email, since a post sent before the setting changed keeps the flags it was sent with. Visitors is strictly `published`, so an email-only post has none. Members is hidden on invite-only sites and for email-only posts, but not for pages, which are never email-only. Contributors see no metrics at all. Opens, Clicks and Sent come from the list payload and work now. Visitors and Members need two POST-with-body stats endpoints, which the framework's query factory doesn't support - that is a framework addition rather than wiring, so the counts are passed in as props and land separately. Ember fills these in reactively too, deliberately not awaiting them so a slow analytics service can't hold up the list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
no ref Two reviews found the same critical bug: saving a post view could permanently delete other screens' saved views. Members, posts and pages share one shared_views setting, and the parser silently drops entries it can't validate while still reporting success - so the save wrote back only the valid subset. A non-array value was worse: it parsed as an empty list, so the next save replaced everything with just the new view. Saving before settings finished loading did the same, since undefined was coerced to "[]". Writes now work on the raw array and re-serialize only the entry they change, so an entry this build cannot understand - including one written by a future version - survives untouched. An unreadable value now refuses to write rather than treating it as empty, and a save before settings load throws instead. Sticky filters were broken on Pages for the three commonest filters: the posts default views were passed in as the "is this just a view?" list regardless of route, and every default view is route: posts. Pages also had its saved views computed but never rendered, so a view nobody could see suppressed the Pages highlight. The colour picker was missing entirely - Ember offers all nine on create and edit, and the colour shows as a dot in the sidebar, so a random assignment with no way to change it is a removed control, not a cosmetic gap. Deleting a view left you stranded on its URL watching the sidebar entry vanish, with the button flipping back to "Save as view" where saving would re-create it. Now navigates back to the clean route, as Ember does. Also: Enter bypassed the in-flight guard; save errors rendered as muted text with no alert role; two views sharing a filter collided on their React key; and a test named for the Owner-vs-Administrator distinction asserted the framework helper directly rather than the call the screen makes, so swapping the helper back would have kept it green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
no ref The Visitors and Members columns now show real numbers. Both endpoints are POST-with-body reads - the id lists are too long for a query string - so they could not go through the framework's query factory, which only builds GETs; they are written against useQuery directly. The id list is part of the query key, so changing the filter starts a new query rather than writing a stale response over the new one. That is what the Ember service's manual generation counter exists to prevent, and TanStack gets it for free. Two bugs found by checking the real responses rather than trusting the shape: The member endpoint returns the map directly in stats[0], not nested under a `data` key like the visitor one - I had assumed both matched. More subtly, passing `Content-Type` alongside the header fetchApi already adds for string bodies produced two differently-cased keys in one header object. The request dropped the body, and the endpoint answered 200 with an empty map rather than an error - so the columns rendered a plausible-looking 0 for every post. Verified against a post with 69 free and 19 paid members, which now reads 88. Also fixes pages not appearing in the list until a manual refresh: the Ember bridge's model-to-query-type map had no entry for `page`, so saving one in the editor never invalidated the React pages list. Harmless while Ember owned /pages. Widening Post to carry authors, tags and tiers collided with narrower local declarations around the analytics screens; those are now supertypes, which also let two casts in an existing test go away. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ref DES-1343 Completes Phase 5 of the Ember→React posts/pages list port, behind the `postsListReact` flag. Two pieces were outstanding: the hover breakdowns Ember shows on each metric, and the trailing action button at the row's end. - the panels use Shade's `HoverCard`, not `Tooltip`: the tooltip is the small dark chip, so a labelled table inside it renders dark-on-dark. Ember's is a white elevated card, which is what `HoverCard` already is - one panel per *group*, matching Ember — the three email columns share a single trigger, so crossing from Opens to Clicks doesn't flash it - the panel shows raw counts where the column shows a rate, as Ember does. The test fixture uses numbers where the two differ so reusing the rate helper can't pass by coincidence again - `hasPostAnalyticsPage` is expressed from Ember's three `show*Analytics` predicates rather than from the column list. The two deliberately diverge: the Members column renders on an invite-only site and a failed send still shows its Sent count, while neither earns an analytics page. Deriving one from the other lost both columns - the row is no longer a single anchor. The metric links are anchors, and nesting them inside a row-wide one is invalid HTML; it is now a container with three anchor regions, as Ember's own markup is
ref DES-1343 Phase 6 of the Ember→React posts/pages list port, behind the `postsListReact` flag. No checkboxes, as in Ember: cmd-click, shift-click, Cmd+A, Escape, click-away, and a "select mode" while a modifier is held. The idea the whole phase turns on: after Cmd+A the selection is *inverted*, and `selectedIds` stops meaning "the rows chosen" and starts meaning "the rows taken out". That is what lets a bulk action cover posts that were never loaded — it sends a filter rather than 2,000 ids — and nearly every bug available here is a branch that only considered one of the two readings. - the selection is cleared whenever the list's filter changes, as Ember does on every model refresh. Without it, selecting all drafts and then clearing the type filter leaves an inverted selection re-bounded to *every post on the site*, which is what Phase 8 would hand to a bulk delete - the shift range reproduces Ember's asymmetry: forward excludes the anchor, backward includes it. It looks like an accident of Ember's loop, but the range is remembered so the next shift-click can undo it, and an anchor left out survives an undo that should have cleared it - deselecting a row only moves the shift anchor when that row *is* the anchor - click-away ignores clicks inside `role="alertdialog"` as well as `dialog`: Radix renders destructive confirmations as the former, so matching only the latter would clear the selection as the user clicked Delete Two deliberate divergences, both documented at their call sites: shift-clicking the anchor row selects nothing rather than running away to the end of the list, and Cmd+A does nothing for authors and contributors rather than building a phantom selection they have no actions for.
ref DES-1343 Two problems on the React posts list, both found in manual testing. The row hover used `bg-surface-elevated`, which resolves to white in light mode — invisible against a white page, so there was effectively no hover at all. Worse, it sat on a child of the element carrying the selected background, so it painted over it: a selected row looked unselected for as long as the pointer was over it, which made a shift-range look like it had missed its last row. Hover and selection now paint the same element, using the two tokens meant for the pair — `table-row-hover` and `muted`, one step apart — so selection stays visible while hovered. Selecting was also doing far more work than it needed to. Selection state and the modifier "select mode" both live above the list, and the row wasn't memoised, so every cmd-click and every press of the Cmd key re-rendered every row along with its Radix hover cards — by far the most expensive thing on the screen. `metricsSettings` was rebuilt each render too, which would have defeated the memo regardless. Measured on a 102-row list, a cmd-click goes from 22-39ms at 35 rows to 3-11ms.
ref DES-1343 Phase 7 of the Ember→React posts/pages list port, behind the `postsListReact` flag: the context menu, and the four actions that need no confirmation — copy link, copy preview link, duplicate, and share as a gift. The idea the menu turns on: it describes the **whole selection**, not the row under the cursor, and Ember's status predicates are "any" rather than "every". One published post among five drafts still offers Unpublish. Getting that backwards would quietly hide actions from mixed selections, which is most of them. - the bulk actions land in Phase 8, so they render disabled rather than enabled-but-inert. A menu that closes and does nothing when you pick Delete is worse than one that says the item isn't ready - gift-link eligibility now lives in `@/shared/gift-link`, read by both implementations, so the entry point can't appear on one side of the flag and not the other - right-clicking an unselected row selects it for as long as the menu is open; right-clicking a row already in a selection leaves the selection alone. That is Ember's freeze/clearOnNextUnfreeze pair, collapsed to one boolean because Radix owns the menu's open state - `memo(PostListRow)` was silently dead: Radix's trigger clones fresh handlers and a fresh `style` onto its child every render. Memoising the menu restores it, along with a stable per-row open handler One deliberate fix rather than a port: Ember's "Copy preview link" copies `post.url`, the public permalink, which for a draft points at a page that does not exist yet — and is the identical string its "Copy link to post" produces, making the two items indistinguishable. This copies the real preview link.
ref DES-1343 Phase 8 (partial) of the Ember→React posts/pages list port, behind the `postsListReact` flag: delete, unpublish, unschedule, feature and unfeature, plus the client-side NQL pruning they all depend on. Add a tag and Change access still need their pickers and stay disabled. The pruner re-runs NQL in the browser against the list's own filter and drops the rows an edit has pushed out of it — that is what makes unfeaturing a post while viewing `?type=featured` remove it immediately, with no refetch and no lost scroll position. Two rules it exists to hold: - a post the action did not edit is never removed, whatever the filter says about it. The alternative is a bulk edit silently emptying rows it had nothing to do with - the expansions are not optional. Without them `tag:news` is looked up as a literal `tag` property, which a post does not have, so every edited post fails to match and the whole selection vanishes from the list The acceptance tests assert the outgoing request rather than the screen, because the selection filter is appended straight to `DELETE /posts/?filter=`: one post sends `id:['…']`, Cmd+A sends the list filter with no ids at all, and Cmd+A minus a few sends `(status:published)+id:-['…']` — parenthesised, so the subtraction applies to the whole filter and not to its last term. The selection is snapshotted when a menu item is picked rather than read when the modal confirms. Radix closes the menu on select, which clears a transient selection, so a modal reading the live selection would find it empty by the time the user pressed Delete. Also: the cache patch is scoped to the buckets on screen, so a list cached from a different filter is never pruned against this one; after an edit the rows still visible stay selected, matching Ember's clearUnavailableItems rather than a full clear; and feature and unfeature show no toast, as in Ember.
ref DES-1343 Completes Phase 8 of the Ember→React posts/pages list port: the two remaining bulk actions, Add a tag and Change access, plus fixes for what the review of the previous commit turned up. The serious one was mine. `isSingle` exists in the modal copy precisely because the selection count is not sufficient to decide singular from plural — an inverted selection of one is still "these posts" — and then the call site passed `isSinglePostSelected(...) || snapshot.count === 1`, reinstating the derivation it was added to prevent. Since `getPostSelectionCount` floors at 1, a stale total could produce a confirmation reading "delete this post", naming one post, while the request carried a filter matching every post on the site. `isSingle` is now captured in the snapshot alongside the filter and the count, and the toast branches on it too, as Ember's does. Also from the review: the cache guard tested for `cached.pages` to detect an infinite query, but a non-infinite *pages* response is literally `{pages: Page[]}` — such an entry would have been treated as infinite and rewritten to nothing. It now discriminates on `pageParams`. And the bulk-edit request shape had no test at all, so a typo in an action verb would have passed every test in the file; every verb and payload is now asserted. Add a tag and Change access refetch rather than prune. Ember re-fetches each edited post in batches of 50 first, because the new tag has to be in the store before the filter can be evaluated against it; applying the edit locally is not an option here, since a newly created tag's slug is decided server-side.
ref DES-1343 Peter reported that after changing a post's access, clicking a post no longer opened the editor — Ember logged `transitioning to…` but never completed. Reproduced as something simpler: the modal never closed. An open Radix modal sets `body { pointer-events: none }`, so the whole admin was frozen and any click that did land started a transition into a screen that could not render. The cause was ordering. `runWithPayload` awaited `invalidateQueries` before calling the callback that closes the modal, and `invalidateQueries` settles only once every active query has refetched. Nothing downstream needed that to have finished, so the modal now closes first and the refetch follows. The duplicate action had the same shape and got the same fix. Also from the same round of testing: - tags already on the post are shown ticked and disabled in Add a tag. This action can only add, so offering to untick one would promise a removal it cannot perform — but omitting them reads as "this post has no tags" - Change access seeds the tier picker from the post, as Ember does. It opened empty even for a post that already had tiers, leaving Save disabled until you re-ticked what was already true - bulk edits now notify the Ember store through `onInvalidate`. Every other mutation gets this via `invalidateQueries: {dataType}`, which these cannot declare without also refetching the lists and undoing the client-side prune — so Ember was left holding post records it believed were current, while the editor is still Ember - pinned `featured` in a test fixture: the builder randomises it, and it decides whether the menu offers Feature or Unfeature, so the test was flaky by construction
ref DES-1343 I added `useFramework().onInvalidate(dataType)` to the bulk actions on a hypothesis about Ember holding stale post records. It was wrong twice over: `PostsResponseType` has no entry in the bridge's `emberDataTypeMapping`, so the call throws — Peter hit it as an error toast on every add-tag — and the mapping it would have needed resolves to `store.unloadAll('post')`, which would drop the record out from under an editor the user may have open. The freeze it was meant to fix was the await-ordering bug fixed in the previous commit, not staleness. Removing it rather than adding the mapping: making a speculative call work is how you end up with `unloadAll` firing mid-edit.
ref DES-1343 Phase 9, partially. The editor writes a localStorage key on publish or schedule and navigates to the list, which reads it on mount. The editor stays Ember on both sides of the flag, so only the reader moves. Both keys are cleared as they are read, before anything is fetched. Ember clears after the modal opens, so a failed request leaves the key in place and the celebration re-fires on every visit to the list until one happens to succeed. Clearing first costs at most one missed celebration and cannot loop. The headings are ported from the modal's `<h1>`: a scheduled post gets "All set!" and nothing else, while a published one gets a second line that depends on whether it is a page, whether it was email-only, and whether the published count has arrived. The count is fetched but not awaited — Ember blocks the celebration behind that second request, and the fallback wording is one Ember already uses. The modal itself is not wired up yet. Dropping `PostShareModal` into the screen produces no dialog: the key is read and cleared, the list renders, nothing 404s, and no dialog appears. Not committing that half until I understand why — the plan called this component out as never having been used in an app, which now looks like the right warning.
ref DES-1343 The celebration reader is single-shot — it clears the localStorage key as it reads it, so a failed request cannot leave the key behind and re-fire the modal on every visit. StrictMode invokes effects twice on mount, so the first invocation consumed the key and set the state and the second read nothing and clobbered it straight back to null. The celebration could never appear. Guarded with a ref, which survives the double-invoke on the same instance while still letting a genuinely new mount — publishing a second post — read a fresh key. Confirmed by instrumenting the acceptance run: before the fix the only outgoing request was the list itself; after it, both the post fetch and the published-count fetch fire as they should. The modal still does not render, and its acceptance test is not committed with this. Two dead ends ruled out along the way, both worth recording: a `data-testid` passed to `PostShareModal` lands on Radix's `Dialog.Root`, which renders no DOM node at all, so it can never be located that way; and a bare `document.querySelector` inside an acceptance test queries the test's own document rather than the app's iframe, so those probes were meaningless.
…t flag - FlagGatedRoute's Ember-authority branch returned a bare EmberFallback instead of the route's `fallback` element, so in the integrated admin with `postsListReact` off — the default — the gift-link modal host never mounted and the Ember list's share-as-gift context menu item silently did nothing - the config-path branches already honoured `fallback`, and the gate tests only covered that path (standalone React, no Ember bridge), which is how this passed green; both suites now cover the Ember-present branch
patchCaches was the only cache maintenance the run() path did, and it had four gaps: - the predicate matched bucket filters by substring, so a cached list whose filter this one merely prefixes (status:draft vs status:draft+featured:true) was patched and pruned against a filter that was not its own — now an exact match on the key's decoded filter param - edited rows were pruned against the list-wide filter, so on the unfiltered list an unpublished post still matched every-status and sat in the published bucket labelled Draft — now pruned per bucket filter - meta.pagination.total was never adjusted, so after Cmd+A + delete the next inverted selection computed its count from the old total — now decremented by the rows removed per query - nothing marked the resource's other cached lists (saved views, search index, analytics screens) stale, and with staleTime at five minutes they kept serving deleted posts — now invalidated with refetchType 'none' so the just-patched active queries are not refetched and scroll survives, matching what runWithPayload already did The snapshot's allFilter existed only for the list-wide prune, so its plumbing through usePostActions and the screen goes with it.
- applyFilter inferred the implementation from an isVisible() probe, which does not retry: an Ember re-render between the waitFor and the probe sent Ember runs down the React path to wait on a Filter button that never exists there, timing out custom-views in CI on all three attempts - the suites already know the flag state, so the page object now takes the implementation in its constructor and never infers it from the DOM; single-lane suites run flag-off and keep the Ember default
…ct-posts-pages-list
- the visitor and member count queries key on the full id list, so loading more rows starts a brand-new query with no cached entry and every visible count rendered as zero while the POST was in flight; keepPreviousData holds the last counts until the new response lands, as the bucket queries already do
- both dialogs had a title but no description, so Radix warned on every open and screen readers announced nothing about what the dialog does; every other admin modal includes one
- use-custom-sidebar-views lost its only production consumer when
nav-custom-views.tsx was replaced by nav-saved-views; the hook and the test
file named after the deleted component go together
- usePostSelection returned selectAll and onContextMenuOpenChange with no
consumer — Cmd+A dispatches internally and rows go through
getContextMenuOpenHandler
- the screen's header docblock still described the phase-one placeholder
("renders bare titles"), and the selection hook now records the measured
~85ms-per-selection-change regression where the follow-up will land
- the screen object was the only one in apps/admin hardcoding its testid strings, and the e2e page object re-hardcoded the same three; the selectors package exists to keep the admin helpers, e2e page objects, and both list implementations on one vocabulary
- the same six-line block of chrome fakes was pasted into eleven beforeEach hooks across eight files; fakePostsListScreen() declares it once beside fakeSettingsScreens, and per-spec fakes registered after it still win
- two tests replaced navigator.clipboard with a recorder and never restored it, so the stub survived into every later test in the file; the recorder is now shared and afterEach puts the original property back
- the tag route this was modelled on checks tagDetailsReact === true and has a unit test pinning that a non-boolean labs value does not hand the route to React; the posts route had the truthy check and no test — a labs value of the string 'true' would have aborted every posts transition with no React screen claiming the URL
- restoring the URL after parking called replaceState(null, ...), and
react-router keeps {usr, key, idx} in window.history.state — nulling it
breaks its back/forward index and useBlocker on the member, tag and
automation editors, on every visit to /posts or /pages
- the state is captured alongside the URL before parking and restored
unconditionally: the hash-unchanged case still loses its state to the
parking navigation itself, so it needs the restore too
- an edited row that left its bucket still exists, so decrementing the bucket total shrank the list-wide count the selection reads; totals now move only on delete - an inverted delete covers rows that were never loaded, so subtracting the removed cached rows left the old total behind; the selection now carries its inverted flag and the total becomes the surviving exclusions - the meta deref inside the setQueriesData updater gains a full optional chain — a throw there surfaces as a false error toast after a successful mutation - prune-non-matching-posts' docs described the list-wide filter its caller no longer passes, and claimed to be a straight port of Ember's updateFilteredPosts, which prunes the other way
- the separator above Unpublish was a flag on the item, set whenever the selection could carry a gift link — but the gift link is filtered per row after the list is built, so a public post (no gift link) drew a rule between Copy link and Unpublish; the menu now draws it from adjacency to the gift link it actually rendered - the regression test guards its arrange step with a selection-count assertion — a broken selection made it pass vacuously — and both separator shapes are pinned - metaMouseDown moves to the screen helper so the two files stop carrying private copies
…ywhere - selectAuthor and selectTag still clicked the Ember per-field triggers unconditionally, so the first React-lane test to call them would hang; they now branch like selectType, and selectOrder says plainly that the React sort lives in its own menu - the editor back link belongs to PostEditorPage beside the rest of the editor chrome; the breadcrumb test now waits on the analytics screen rendering rather than the URL, which matches before Ember has parked - the change-access description singularises with its title
- the note blamed the per-row menu wrapper for ~85ms per selection change, but that wrapper was already removed on this branch (35ms to 4ms at 100 rows, measured — see 044d957); the note would send the next engineer to re-do finished work
- filter-in-the-URL is subsumed by the saved-view byte-identity test in the same suite plus the acceptance currentRoute assertions, and the empty state is pinned in posts-list-rows acceptance; each ran twice per flag state under per-test isolation, so this drops four Ghost environments from every CI run while the parity signal stays
- React prunes the row against its bucket's filter so it leaves the published section at once; Ember prunes against the list-wide filter and leaves it there labelled Draft — the divergences file exists to hold that difference with its reason, and it was only recorded in a pruner docblock - confirmDelete generalises to confirmAction so the new test can confirm an unpublish through the page object
- compressed the comment blocks that argued layout choices, carried phase-plan rhetoric, or cited a commit SHA down to the constraints they contain; rationale that long belongs in commit messages - PostsContextMenu's memo wrapper never held — its children prop is a fresh element on every row render — and its docblock described the pre-inlining architecture; the row's own memo is the boundary that matters
- the totals arithmetic is not observable through the acceptance suite but feeds the selection count, so the four behaviours get pinned directly: delete decrements, inverted delete keeps only the exclusions, an edit leaves the total alone, and a filter that merely extends the bucket's is not patched
ref https://linear.app/ghost/issue/PLA-334 Icons on each row and figures in mono, matching what Ember draws in the same panel: a paper plane for Sent, an envelope for Opens, a cursor for Clicks. The colours and typography are left alone - Peter wanted those as they were. The icon is named on the row (`icon: 'sent'`) rather than attached as a component, so `post-metric-tooltips.ts` stays plain TypeScript with no React in it - it is the module the unit tests lean on. The component owns the name to icon mapping, and because that mapping is a `Record` over a union, a new row without an icon will not compile. Mono for the values follows the analytics tables, which already do this: the figures line up on their digits when several rows stack. The card also opens above the metric now, as Ember's does - its `.above` positioning is the default and it drops below only when there is no room. Radix flips on collision by itself, so setting the side is a preference and the fallback comes free. Confirmed in the browser against the bottom row of the viewport, where a card that could not fit above would have flipped. The card widened from `w-44` to `w-48`: the icons take horizontal room and "Unique visitors" was close to wrapping without it.
ref https://linear.app/ghost/issue/PLA-332 The columns showed a figure stacked over a text label. They now show an icon beside the figure, as Ember's do. The icons are exact rather than approximations: `app/assets/icons/analytics-*` turned out to be Lucide icons exported to SVG, each still carrying its `lucide-…-icon` class, so the names were read out of the files. That caught a guess in the previous commit - paid members is `wallet-cards`, not `user-plus`, and `user-plus` is the *members* column, so the same icon would have meant two different things. The row and the hover panel now read from one shared map, because Ember draws the same icon in both places and two maps would drift. Each column keeps an `aria-label` ("Opens: 78%") and a `title`. Without the text label a column is an unnamed picture beside a bare number - "0%" with no subject. Ember titles its icons for the same reason, and the acceptance tests find columns by accessible name. Figures are smaller and unbolded, matching Ember's `.gh-post-list-analytics-metric` - midgrey at normal weight, secondary to the title rather than competing with it. Two spacing changes came out of looking at it on a narrow window: - The meta line now truncates like the title above it. Left to wrap, a long author-and-tag line ran to three lines and drove the row's height, which read as the metrics squashing the title. The metrics were not the cause. - The metrics hide below 1200px, where Ember hides the same container. A literal 1200 rather than a named breakpoint: Shade's nearest is `sidebarlg` at 1240px, which describes the sidebar and would tie this rule to something unrelated. Gaps are 12px throughout the right-hand side, with the button 20px off the metrics - it is an action rather than a figure, and reads as the last item in the run when spaced the same. The extra space is a margin on the button rather than a wider row gap, which would have pushed the title away from the metrics too. The columns keep `min-w-16` so the figures line up in scannable columns. That costs even spacing - a short value sits right-aligned in its box and leaves slack on its left, so the space between two blocks reads wider than the space to the button - and the trade is deliberate, so it is written down at the call site.
ref DES-1343 A checkbox list became a field of chips with a floating list beneath it, following the members label picker - minus the editing, since nothing here renames or deletes a tag. Tags already on the selected posts are no longer shown. They were listed ticked and disabled, which read as a set you could edit while offering no way to untick one: the action can only add. Ember shows only what you are adding, and now so does this. Internal tags render as a solid dark chip against the outline of a public one, as Ember styles them. Adding in bulk is where the distinction earns its keep - an internal tag changes nothing a reader sees, so mistaking one for the other means quietly publishing a label meant to stay private. A typed name counts as internal too when it begins with `#`, which is Ghost's own rule, so the chip tells the truth before the save rather than changing appearance after it. A separate picker rather than generalising the members one, which was Peter's call: it keeps a shipped feature out of this branch's review. The duplication is real and worth revisiting if the two ever want different behaviour. Two details are load-bearing, and both are written down where they live: - Escape closes the list and is stopped from bubbling. Left alone, Radix takes the same key as a request to dismiss the dialog, throwing away every tag picked so far. - The list is dismissed by a plain `pointerdown` listener that does not prevent the default. It floats over the dialog's own footer, so this is what lets a single click on Add both close the list and press the button - a Radix popover would swallow that first click. The tests cannot lean on that last one: the browser driver checks a button is unobscured before clicking it, and would wait forever. They dismiss the list by clicking the dialog heading first, which is the same gesture with its timing made explicit. Two new tests: that the dialog offers a tag the post already has rather than pre-ticking it, and that a typed tag is sent with no id for the server to create.
ref DES-1343 Typing a tag name the server does not recognise creates a tag as a side effect of saving the post. The bulk action announced that the *posts* had changed and nothing else, so the cached tag list never heard about the new tag - it was missing from the filter and from the add-tag dialog itself until the browser was refreshed. Invalidated for every add rather than only the creating kind: adding an existing tag moves its post count, so that list is stale either way. This is the third time the same shape has caught this feature - a tag created as a by-product of a post operation, with nothing watching for it. The first was the Ember bridge reporting a `post` change when the editor created a tag; the second was that fix's own blind spot. Worth naming, because it will recur wherever tags are created indirectly. The test was written first and watched fail - the tags endpoint fetched once and never again - so it catches the regression rather than merely passing beside it.
ref DES-1343 Six things Peter found by using it. The list no longer opens focused on load, and the field carries a chevron so it reads as a dropdown rather than a text input. Selecting a tag clears the search term, which used to sit in the field so the next thing typed appended to a search already acted on. Each row shows its slug in grey mono on the right. Arrow keys move through the filtered list and onto the "Create" row, and Enter takes whatever is highlighted. This needed the list rebuilt rather than patched: it was `cmdk`, which only drives the keyboard for an input inside its own tree, and this input sits in the chip field above the list - so the keys had nowhere to go. Putting the input inside `cmdk` was not open either, since it is a dependency of shade and not of admin. The list is now plain markup with the highlight handled directly, which also lets the highlight reach the create row. Selection keys on the tag id rather than its name. Names are not unique - a site can carry two tags called "broaf" differing only by slug - and comparing names ticked both at once, sending a tag the user had not chosen. That one has a regression test. Escape closes the list, and reaches the dialog only when nothing is typed and nothing is picked. Radix reads the key as "dismiss", so backing out of the open list was also the gesture that discarded the work. Two attempts at that last one failed before the third worked, which is worth recording because both look reasonable: - Guarding it on the input's key handler does nothing once a row is clicked with the mouse, because that moves focus off the input. - Guarding it with a capture-phase listener on `document` does not work either. Radix listens in capture on the same node, and `stopPropagation` does not stop a sibling listener there - only `stopImmediatePropagation` would, and that depends on registration order, which shifts every time the effect re-runs. The working version uses Radix's own `onEscapeKeyDown`, where `preventDefault` is the supported way to refuse. The picker closes its list; the dialog decides its own dismissal; no listeners race. Note the trade: with tags picked, Escape will never close this dialog. Cancel and the close button remain.
ref DES-1343 Two tags called "broaf" highlighted as one row in the tag filter, because Shade built each row's `cmdk` value from the option's *label* — so `cmdk` saw one row, not two. Identity now comes from the option's `value`, which is unique by definition, with `keywords` carrying the label so the filters that do filter client-side still match what the row shows. That is the fix on its own. Getting there went the long way round first: the slug was passed as `detail`, which made the composed label-and-detail value unique as a side effect. It worked, but it paid for a rare case in every row — stacked, the slug read as a second entry in the list; beside the label, it crowded out the name being chosen. It is now carried in the row's `title` and not drawn, which is what Peter asked for and what the value fix makes possible. Also dropped the invisible checkmark from unselected rows. Selected options render in their own group above with a real check, so the `opacity-0` one was a spacer that only narrowed the names. The tag filter's popover is widened to 320px through the per-field `className` Shade already supports, so the 200px default is untouched for everything else. Blast radius checked before changing shared code: nothing else in the codebase passes `detail` to a filter option, so no other filter changes appearance. The Author filter has the same exposure with duplicate display names and is fixed by the same change, since it no longer depends on anything being passed.
Contributor
E2E Tests FailedTo view the Playwright test report locally, run: REPORT_DIR=$(mktemp -d) && gh run download 31380279939 -n playwright-report -D "$REPORT_DIR" && npx playwright show-report "$REPORT_DIR" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rebuilds Ghost Admin's posts (
/ghost/#/posts) and pages (/ghost/#/pages) lists in React with full feature parity, behind the privatepostsListReactLabs flag. The flag defaults off — Ember still serves both routes unless you turn it on in Developer Experiments.What's here
Everything the Ember lists do: three sequenced status queries, the five URL params, filtering and sorting, sidebar saved views and sticky filters, analytics columns with hover breakdowns, modifier-click selection, the right-click menu, every bulk action, and the post-publish celebration.
Both implementations coexist on the same URLs. The flag decides which renders, and the Ember route aborts its transition so only one is ever in the DOM.
The two ideas the port turns on
The URL is a saved view's identity. The sidebar stores those five query params verbatim, and both implementations read the same ones. A screen that rewrote or dropped one would corrupt every view the other created — so the URL is never rewritten on hydration, only by an explicit user action, and an e2e test asserts it survives a load byte-for-byte on both sides.
Selection is inverted, not enumerated. After Cmd+A,
selectedIdsstops meaning "the rows chosen" and starts meaning "the rows taken out". That is what lets a bulk action cover posts that were never loaded — it sends a filter rather than 2,000 ids. Nearly every bug available in that area is a branch that considered only one of the two readings.Testing
Known gaps
The selection-speed regression from the per-row context menu was fixed on this branch (35ms → 4ms per selection change at 100 rows — see
044d9573a6e/ DES-1343). Remaining perf candidates — narrowing the select-mode[&_li_*]descendant selectors, bailing out of no-opcleardispatches, and the Phase 10 virtualisation — are follow-ups, gated on a fresh production-build measurement.One deliberate divergence is documented in
posts-list-divergences.test.ts(the draft preview link — an Ember bug fixed rather than ported). The Ember-side bugs it and the context-menu tests reference still need issues filed.The two screens deliberately do not look alike yet — a design pass is separate.
🤖 Generated with Claude Code