Conversation
Puts the posts list behind a new "Posts list redesign" experimental feature: featured-image cards with serif titles, a lead card for the newest post, date-group headers and filter chips in place of the tab row. Rows carry all-time view and comment counts so the list answers "how is the blog doing?" rather than only "what have I written?". The row components live in ui/rs/contentlist and take no post-specific types, so the pages list can adopt them rather than growing a second copy. Comment counts read the site's own REST API and so work on self-hosted sites; view counts need WordPress.com stats and are omitted where those are unavailable.
nbradbury
added
Posts + Pages
Do Not Merge
Collaborator
Generated by 🚫 Danger |
Contributor
|
|
Contributor
|
|
Contributor
🤖 Build Failure AnalysisThis build has failures. Claude has analyzed them - check the build annotations for details. |
…code PostRsListViewModel gained two constructor parameters without its test being updated, which broke the unit test compile. Detekt objected to return counts, nesting and a file whose name did not match its single class declaration, none of which change behaviour to resolve. Adds tests for RsCommentCountFetcher and ContentDateGrouper. The fetcher's batching is worth pinning: the interesting cases are a post with no comments counting zero rather than being omitted, and a batch too large to count falling back to per-post totals instead of silently undercounting. The grouper's tests pin "now" rather than reading the clock so the bucket boundaries do not drift with the run date.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Five issues from review of the metrics path, all in how fetches are triggered and bounded rather than in what they display. A refresh clears the cached "nothing to show" entries, which puts those rows back into the pending state - but the only thing that asks for metrics is the visible-row set changing, which a refresh in place does not do, so the skeletons had nothing left to resolve them. The refresh now re-requests for the rows already on screen. The view-count semaphore was allocated per call while the caller fires on every visible-set change, so the cap applied per emission rather than across the screen and a fling could still put a request in flight for every row it passed. It is now one ViewModel-scoped gate, and the visible-row stream is debounced so a fling settles before fetching. A batched comment count returned the same "no answer" for a request that failed and one too large to fit in a page, so an offline batch fanned out into per-post requests that each failed in turn. The two cases are now distinct, and only the second falls back. Metrics are skipped entirely while searching: results there mix statuses and only published posts are fetched, so a skeleton raised over a search result could never resolve. Also removes a stale doc block that described batch cancellation the implementation deliberately does not do.
Six spots where the same logic was written twice, each of which could have been edited in one copy and not the other. The compact and hero rows repeated the whole badges/title/excerpt/meta stack, differing only in title size, and the views and comments metrics repeated their separator-plus-plural rendering. Both are now written once. The sync-failed label restated the metadata text style just to change its colour, so that style now takes a colour parameter. The comment-count and view-count fetches each wrote their cache and pushed the result to the rows in both the success and failure branches. That was the riskiest of these: updating one branch and not the other would leave a row waiting on a skeleton that never resolves. Each now has a single write and a single apply, with the fetch itself reduced to an expression that yields null when it cannot answer. Note this is not a net reduction in lines - the extracted helpers cost about what the duplication did. The point is that these can no longer drift apart.
Adds a user toggle in the top app bar switching the redesigned list between two densities. Condensed drops the excerpt, the metrics and the lead row's hero image, taking a row from roughly 166dp to 110dp. Dropping the excerpt is what makes the toggle worth having. Removing the hero and the metrics alone saves nothing after the first row: the date still occupies the metadata line and the overflow button sets its height. The excerpt is 46dp of a 166dp row. Condensed also stops metrics being fetched rather than merely hiding them, so a condensed list issues no per-post view requests and no comment-count request. Returning to comfortable has to ask for them explicitly - the rows on screen have not changed, and the visible-row set is what normally triggers a fetch. Comfortable remains the default, so this is additive: nothing changes for anyone who does not use the toggle. The preference is stored under a content_list key rather than a post_list one so the pages list can share it when it adopts these components.
The redesigned list set its top app bar title in a serif face, following the design doc. No other top bar in the app does, so on device it read as a rendering fault rather than a choice - and it was not even the specified typeface, since the serif here is the system fallback rather than the Source Serif 4 the design asked for. Row titles keep the serif. That is where it earns its place: it is the content, and it matches how blogging prompts and the Reader's serif reading preference already use it.
Switching density changed every row's height at once, and the list snapped to the new layout. Cards now animate their own size, so the excerpt and metrics going away and the thumbnail shrinking read as the row contracting rather than being replaced. Rows below were already moved by the animateItem() the list applies. The lead row is a different layout rather than the same one resized, so its swap between the hero and compact shapes is crossfaded instead. That is a dissolve, not a move: the image does not travel from the top of the card to the trailing edge. Making it travel needs a shared element transition, which is worth doing only if the dissolve reads as a teleport in practice.
The button sat at the trailing edge of the text column, which is a weighted sibling of the thumbnail. A post with a featured image therefore had its button pushed left by the thumbnail's width, while a post without one had it against the card edge - so the button moved horizontally from row to row. It is now a child of the row itself, after the thumbnail, and lands in the same place either way. Rows also get shorter as a result. The button's 48dp touch target was inflating the text column, which is what the trimmed bottom inset was compensating for; with it moved out, the metadata line is just text and the card pads itself normally. Roughly 166dp to 146dp comfortable, and 110dp to 96dp condensed. The hero row keeps its button on the metadata line. It has no trailing thumbnail, so that line already runs to the card edge and the button is already where it should be.
Moving the compact row's button to the card edge left the hero's behind: its button stayed inside a column padded by 14dp, so its icon sat 26dp from the edge against 12dp everywhere else. The hero now uses the same shape as the compact row - a weighted text column with the button beside it - so both end the same distance in. Three things became dead once the button left the text column in both shapes, and are removed: the menu parameter on RowBody and on RowMetaLine, the nested weighted Row that existed only to push the button right, and the trimmed bottom inset that compensated for the button inflating the hero's column. The metadata line is now a flat row of text and the hero pads its bottom normally.
WpRequestResult.RequestExecutionFailed is a data class, so mocking it trips the project's DoNotMockDataClass lint rule and failed both flavour lint jobs. Built for real instead, following the pattern already used in ApplicationPasswordValidatorTest.
Adding a modifier for the overflow button move placed it after size, which also has a default, so modifier was not the first optional parameter and Compose's ModifierParameter lint rule failed both flavour lint jobs. The call site passes named arguments, so only the signature changes.
The toggle paired ViewAgenda with ViewList: two unrelated glyphs, so the two states did not read as one control. DensitySmall and DensityLarge are the same glyph family at different spacing, and are named for this axis rather than borrowed from layout icons. It also settles an RTL inconsistency. ViewList is auto-mirrored and ViewAgenda is not, so one state flipped under a right-to-left locale and the other did not. Both density glyphs are symmetric. The icon still shows the density a tap switches to, matching what the content descriptions say.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## trunk #23316 +/- ##
==========================================
- Coverage 38.01% 37.88% -0.14%
==========================================
Files 2264 2270 +6
Lines 126820 127496 +676
Branches 17811 17922 +111
==========================================
+ Hits 48212 48301 +89
- Misses 74629 75208 +579
- Partials 3979 3987 +8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Both states appeared instantly the moment the list resolved, which read as a pop rather than a transition. They now fade in over 300ms. The fade is driven by a MutableTransitionState rather than a plain `visible = true`. These states only enter the composition once the list has turned out to be empty or failed, so a plain flag would already sit at its target on first composition and Compose would skip the animation entirely - the same pop, with more code. FadeInOnAppear wraps both branches of the state switch, so the two content composables stay pure layout and the two states cannot drift apart.
Tested on a device with and without it: the density switch looks the same. animateItem() was already doing the visible work - sliding rows into their new positions - and a card's height change is small enough, and brief enough, that the sliding masked it. That leaves one animation per row during a switch rather than two. animateItem() animates placement and add/remove fades but not size, so the two were not redundant in principle; they just turned out to be in practice, at this size of change. The crossfade on the hero-to-compact swap stays. That is a different layout rather than the same one resized, which animateItem() does not cover.
Six places where the same thing was expressed twice. The featured image was built twice - once full width for the lead row, once as a square for the rest - with identical request builders and shimmer fallbacks. Both row shapes also repeated the same outer row: padding that drops its end inset when a menu is present, a weighted text column, and the button beside it. Both are now written once, the latter with a trailing slot for the compact row's thumbnail. That second one is worth the extraction on its own: those two shapes drifting apart is exactly how the hero's overflow button ended up sitting 14dp further in than every other row. ContentListCard no longer takes a nullable onClick. Only the placeholder passed null, and it now builds its own card. The two non-content entry types held identical data and differed only in what rendered them, so they are one type that switches on the post's display state. Note this needed an isRedesignEnabled guard: with the flag off every row goes through the pre-redesign item, which draws its own placeholder, and without the guard flag-off placeholders would have started using the redesigned one. Also folds loadItemsForTab's local search predicate into the isSearching property - keeping it a local, since the function suspends between its three reads - and corrects expectsMetrics's comment, which predated the density gate it now applies. This is not a net reduction in lines; helpers used at two call sites cost about what the duplication did. The point is that these can no longer drift.
The full-width image was reserved for the newest post. It now applies to any post that has a featured image, so the list reads as a column of image-led cards rather than one lead card above a column of thumbnails. Dropping the first-row condition made the tracking variable behind it dead, so it is gone; the rule is now just comfortable density, a list that groups by date, and a post that has a featured image. This also widens the gap between the two densities, which is arguably the bigger win: comfortable becomes genuinely magazine-like while condensed stays tight, rather than the two differing by an excerpt.
Cards sit 8dp from the screen edge rather than 12dp, and 4dp apart rather than 8dp. The extra width matters most on the image-led rows, where the full-width image gains it directly. The filter chips move with them. They were also at 12dp, so leaving them would have left the chip row indented further than the cards beneath it. The group header inset is now derived from the card padding rather than being its own 16dp. The mockup sets headers 4dp further in than cards, and a hardcoded value meant that relationship broke silently the moment the card padding moved - which is what this change does. Padding inside the cards is unchanged. Reducing that would crowd the text against the card edge rather than make the list denser.
The density toggle was added between the author filter and search. Top app bar actions are end-aligned, so that pushed the author filter one slot left of where it has always been. Moving the toggle ahead of the author filter leaves both pre-existing icons in their original positions and adds the new one to their left. On sites without author filtering there is nothing to preserve, and the toggle simply sits left of search.
fetchMediaUrls logs its failure and returns an empty map, so rows with a featured image kept isImagePending set and shimmered indefinitely. There was no failure path at all: no retry, no fallback, nothing to end the wait. Media ids the lookup does not answer for are now recorded, and those rows stop waiting and render without an image. A row in that state is also no longer given the image-led shape. An image-led card with no image is a compact card with the wrong padding and a gap where the photo belongs. A refresh clears the record so a failed lookup is tried again. That matters because the lookup is a batch: a single unreadable item leaves every id in the request unanswered, so without a retry one bad attachment would cost the whole list its images until the app restarted. This does not address why the lookup fails - on at least one site it returns "Server could not read response", which is a parse failure somewhere in rs or the payload, and is being tracked separately.
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
4 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.
TL;DR
Puts the wordpress-rs posts list behind a new Posts list redesign experimental feature: featured-image cards, date-group headers, filter chips instead of tabs, per-row view and comment counts, and a Comfortable / Condensed density toggle. The row components are post/page-agnostic so the pages list can adopt them rather than growing a second copy.
untitled.webm
Description
New, shared —
ui/rs/contentlist/, deliberately free of post-specific types (the overflow menu is a composable slot):ContentListRow/ContentListHeroRow— cards with serif titles and a two-line excerpt; posts with a featured image get a 130dp full-width one, the rest a 72dp trailing thumbnailContentDateGroup— buckets rows into "This week" / "Earlier in July" / "March 2025"ContentListFilterChips— scrolling chips; the pager stays, so swiping between tabs still worksMetrics. Rows show all-time views and comment counts, from two sources with different reach:
RsCommentCountFetcher, one batchedcomments?post[]=…request per visible set, counted client-side. Reads the site's own REST API, so it works on self-hosted. Falls back to per-postX-WP-Totalwhen one page can't cover the batch, which would otherwise undercount.StatsDataSource.fetchPostViews, one request per post, capped at 4 concurrent. WordPress.com/Jetpack only; self-hosted rows simply show no views.Both are fetched by scroll position rather than page load, cached per post id, and show a shimmer until they land. Neither can take the screen down — a failure clears the skeleton and the row falls back to date-only.
Density. A top-bar toggle switches between Comfortable (the default, unchanged) and Condensed, which drops the excerpt, the metrics and the full-width image — roughly 146dp → 96dp for a row without a featured image, and a good deal more for one with. Condensed also stops metrics being fetched rather than merely hiding them, so it issues no per-post requests; switching back re-requests for the visible rows. Stored under a
content_list_*key so the pages list can share it.Known limits: the metrics gate requires
ORIGIN_WPCOM_RESTandhasCapabilityViewStats, so Jetpack sites added via application password get no views despite having working stats — left as-is for now, related to CMM-2304. Comment-count semantics differ slightly between the REST path (approved only) and WP.com stats.Testing instructions
Enable the feature:
android_wp_rs_wpcomon.Metrics on a WordPress.com site:
views · commentsSelf-hosted site:
Density toggle:
Regression — flag off:
Also worth checking: dark mode, landscape, larger font sizes, and the Scheduled tab (no date groups by design).