feat(messages): trending tags rail (#30) - #129
Merged
Merged
Conversation
Surfaces GET /api/tags/trending as a horizontally scrolling rail of tag chips, and every chip opens that tag's feed through the single entry point #29 left behind, MessagesDestinations.tagFeedRoute(tag) — with the tag handed over byte-for-byte, because real tags contain spaces and commas ("life is short, o brave girl"). Placement: the rail is the first row *inside* the feed list, not a fixed band above it. The top of that screen is already spoken for by the view switcher (#19) and the composer, so the rail is walked past on every visit and then scrolls away. It is also pinned into the feed's empty state, where having somewhere to go matters most — including the tag feed's own "nothing tagged X yet". The window is a *request* parameter, never response metadata: the live payload is { tags: [ { tag, count, lastUsedAt } ] } and reports nothing about the period it covers, while the server silently falls back to `week` for any value it does not recognise. So the app sends window=week explicitly from a typed TrendingWindow and labels the rail from the window it asked for — "Trending this week" is true because the request made it true. lastUsedAt, the one recency the payload does report, is spoken in each chip's accessibility label. Parsing is defensive: rows may lack count or lastUsedAt, may null them, and may carry keys the API adds later; a row with no usable tag is dropped rather than rendered as a blank chip. Loading, tags, empty and error are four explicit states decided in one place, so a quiet instance ("no trending tags yet") can never be mistaken for a failed lookup (which gets its own message and a Retry), and neither can ever be a blank strip. Tests: the live payload shape and its odd rows (unit), the window and limit on the wire plus the failure path (MockWebServer), the four states and the tag surviving intact into the route (ViewModel), and the rendered states, the tap, and the rail's two homes in the feed (Compose). Closes #30
Kept both deep-link resolutions in MainActivity: #89's blog subscription route from the queue and the tag-feed route this branch carries.
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
1 participant
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.
Closes #30. Part of epic #27 — the last of its three sub-issues.
Zero
:appchanges were needed: #29 already wiresonOpenTagat bothMessagesRoutecallsites, so taps flow through the existing
MessagesDestinations.tagFeedRoute(tag)entry point.The issue's premise about the window is wrong, and no label was invented
helpapi/utility-endpoints.txtplus live probing establish thatwindowis a requestparameter (
day|week|month, defaultweek) and that the response reports nothing aboutthe period — it is a bare
{"tags":[{"tag","count","lastUsedAt"}]}.Worse: the server silently falls back to
weekfor an unrecognised value.?window=bogusreturned byte-identical results to
?window=week, with no error — the sameaccept-and-quietly-rewrite behaviour found on saved-view
config.mode(#51) andtheme(#36).So the app sends
window=weekexplicitly from a typedTrendingWindowenum and derives the headingfrom the window it requested — "Trending this week" is true because the request makes it true,
not because the API said so.
lastUsedAt, the one recency the payload does carry, goes into eachchip's accessibility label.
Placement
The rail is the first row inside the feed's
LazyColumn, not a fixed band. The top of thatscreen is already contested by #19's view-preference switcher and the composer FAB, so a permanent
band would cost ~78dp forever; as a list row it is walked past on every visit and then scrolls away.
It is also rendered in the feed's empty state, where "somewhere to go" matters most — covering
both a fresh account and the tag feed's own "Nothing tagged X yet". It renders on tag feeds too
(lateral hopping between tags), and is omitted entirely where the host wires no
onOpenTag.Verification
./gradlew :app:assembleDebug testDebugUnitTest→ BUILD SUCCESSFUL, 1484 tests, 0 failures. TDD— tests written before each layer. 20 new JVM tests: the live payload parses with spaces, commas and
mixed case intact; rows missing or nulling
count/lastUsedAtparse; a row with no usable tag isdropped; unknown keys ignored;
window/limitverified on the wire; a 500 is an error, not anempty list; the four UI states are decided in one
statusproperty so EMPTY and ERROR are provablydistinct; a tapped tag becomes
messages/tag/life%20is%20short%2C%20o%20brave%20girland decodesback byte-for-byte. 10 Compose tests compile, not executed (no emulator).
Reviewer notes
surface, and a stale cached rail would be worse than the honest empty/error states. A failed
refresh keeps previously loaded chips rather than replacing them with a banner.
TrendingTagsViewModel, so opening a tag feed makes one extra smalltrending request. Sharing it across the graph was not worth the scoping complexity.
GET /api/messages?tag=) #29 landed on the queue, so it carries an explicit merge of Tags: tag-filtered feed (GET /api/messages?tag=) #29 plus amerge bringing it up to the current queue tip. The PR diff is exactly this issue's 14 files.