Add quick-win UX polish to the rs comments list#23071
Closed
nbradbury wants to merge 26 commits into
Closed
Conversation
Add a Compose comments list backed by wordpress-rs, following the
postsrs/pagesrs pattern. There's no rs mobile-cache support for comments
yet, so each tab pages directly against /wp/v2/comments using the
response's nextPageParams as the cursor.
- 5 filter tabs (All/Pending/Approved/Spam/Trashed). The All and
Approved tabs use CommentStatus.Custom("all"/"approve") because
WP_Comment_Query doesn't recognise the "approved" value the rs enum
serialises to
- Rows show avatar, bold "author on post" title (post titles resolved
via a batched sparse-field request), snippet, date, pending indicator
- Pull-to-refresh, load-more, retry snackbars, shimmer/empty/error states
- Tapping a row opens the rs comment detail; the list refreshes on return
- Gated in ActivityLauncher.viewUnifiedComments by RS_UNIFIED_COMMENTS +
site capability (WP.com REST or application password), falling back to
the legacy list; the now-dead per-tap rs gate inside the legacy list
fragment is removed
Batch moderation and search follow in stacked PRs.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rience Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…g and SnackbarMessage Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Danger requires immutable string keys: changed copy gets a new key rather than editing the old one in place. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Key the post-title cache by (site, post id) so titles can't leak across sites, and negative-cache unresolvable ids so they aren't re-requested on every page load - Set perPage explicitly on title batches (server default of 10 silently truncated larger batches) and fall back to the pages endpoint for comments left unresolved by the posts endpoint - Discard load-more results whose fetch predates the latest applied first page, preventing stale pages/cursors after a silent refresh - Offer a retry action on load-more failures and auto-advance past pages that dedupe away entirely, so pagination can't stall at the list bottom Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Chunk post-title batches to the server's per_page maximum of 100; larger values are rejected outright with rest_invalid_param - Always attempt the pages endpoint for ids the posts endpoint didn't resolve, and only negative-cache when both endpoints succeeded - Clear negative-cached titles on user-initiated refresh so posts published after first sight can resolve - Cap unattended load-more auto-advance at 3 pages and let it handle empty pages, not just fully-deduplicated ones - Don't cancel/re-fire an identical in-flight title-resolve batch - Add CommentsRsDataSourceTest covering the cache, fallback, and chunking behavior, plus ViewModel tests for the new paging rules Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Cancel a tab's in-flight title-resolve job on user refresh so a job holding pre-clear 'not found' results can't re-apply them via the identical-ids guard - Generation-stamp the negative cache so a fetch already in flight when the user cleared can't re-poison entries with pre-clear results - Make the data source tests execute each request's builder lambda against a mocked uniffi client, asserting the endpoint, include count, and perPage actually sent — the chunking test now fails if chunking is reverted, and the posts-before-pages order is asserted, not assumed Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolves the Android Lint code-scanning alerts on CommentsRsDataSourceTest: SparseAnyPostWithViewContext and PostsRequestFilterListWithViewContextResponse are pure data classes, so the tests now build real instances. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Detail opened from the rs list backfills the FluxC cache (WP.com) and resolves the post title via rs, so title and like state are correct - Detail reports RESULT_OK only when the comment changed; the list only refreshes then, keeping paged lists and scroll position intact - Guard against duplicate first-page fetches; skip the retry snackbar when a silent refresh fails on a tab that already shows comments - Clear isLoadingMore when a refresh replaces the list so paging can't stall behind the busy guard - Stop the load-more trigger firing on the empty pre-layout list and re-arm it when the list size changes - Evict all cached post titles for the site on user refresh so renamed posts pick up their new title - Track COMMENT_FILTER_CHANGED on tab changes like the legacy list - Restore the singleTop launch mode the legacy comments activity has - Drop the dead initializingTabs bookkeeping - Merge RsCommentListItem into RsComment (one shared type and mapper) - Move the new unit tests to a follow-up PR to reduce this PR's size Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Delete the now-unused ActivityLauncher.viewUnifiedCommentsDetails - Track title-resolve jobs as a plain per-tab Job: cancel-and-relaunch is cheap because the data source caches resolved titles - Drop the dead tab parameter from onCommentClick and the unused snackbarMessages default argument Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Long-press a comment to enter selection mode, then tap rows to build the selection. The top bar swaps to a contextual action bar offering the same per-tab actions as the legacy list's action mode (approve/unapprove/spam/ not-spam/trash/restore/delete), with confirmation dialogs for trash and delete-permanently and COMMENT_BATCH_* analytics. Moderation runs the rs writes in parallel, aggregates any failures into a single snackbar, then refreshes all initialized tabs, since moderated comments move between them. Selection clears on tab change and after each batch action. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…list-batch # Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListActivity.kt # WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListUiState.kt # WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListViewModel.kt # WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt # WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListScreen.kt # WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt
The confirm button is wired directly to onConfirmPendingAction in the screen, so this field was never set or read. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Drop the single-use ConfirmationDialogState wrapper; pass pending confirmation and dismiss callback directly to the screen - Extract the confirmation dialogs into a private BatchConfirmationDialogs composable, removing the LongMethod suppression - Remove a redundant trailing semicolon in CommentsRsBatchAction Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Text labels ("Approve", etc.) crowded the contextual action bar and
wrapped the "N selected" title. Render each batch action as an icon
button reusing the legacy comments list's action-mode drawables
(thumbs up/down, spam, trash), keeping the string label as the icon's
content description for accessibility.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Match the legacy comment list's action mode: keep approve/unapprove as always-visible icon buttons and move the remaining actions (spam, not-spam, trash, restore, delete) into an overflow menu, so the "N selected" title no longer wraps. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collapse the PendingConfirmation sealed interface into a single data class that carries the CommentsRsBatchAction, so onBatchAction routes on isDestructive and onConfirmPendingAction reuses action.targetStatus instead of re-mapping the confirmation type back to a CommentStatus. Also use MutableStateFlow.update in toggleSelection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Approve and unapprove stay visible in the batch action bar but are disabled (dimmed) unless the selection actually contains a comment they can act on: approve needs an unapproved comment, unapprove needs an approved one. The active tab's selected statuses drive the enablement. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Gate the contextual action bar on the active tab actually containing selected comments, so it no longer briefly flashes the next tab's actions (with approve/unapprove dimmed) mid-swipe before the selection clears. Drive the confirmation dialog off ConfirmationCopy carried by each destructive action instead of a non-exhaustive when: onBatchAction now routes to confirmation only when the action has copy, so a destructive action can no longer strand the selection with no dialog. This also replaces the isDestructive flag and drops the duplicated trash/delete dialog branches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Animate the top-bar swap between the normal bar and the selection CAB - Make the avatar tappable to toggle selection, the discoverable path into selection mode (long-press still works) - Add Select All to the CAB overflow menu - Re-tapping the active tab scrolls its list back to the top (per-tab LazyListStates hoisted to the screen) Long-press haptics were considered but need no code: Compose Foundation 1.11.4's combinedClickable already performs them by default. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Generated by 🚫 Danger |
On a paginated list "Select all" only selects the comments loaded so far, which reads as selecting everything on the tab. Drop it rather than ship a misleading affordance. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Back now dismisses the selection instead of leaving the screen, matching the legacy action mode - The avatar crossfades to the checkmark when a row is selected - The selection bar uses surfaceContainerHigh so entering selection mode visibly reads as a mode change Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Closing - I've decided to add these changes to #23062. |
Contributor
|
|
Contributor
|
|
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
3 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.
Description
Stacked on #23062: targets trunk per the plan to rebase once the batch PR merges, so until
then the diff also shows the batch-moderation commits — this branch's own changes are just the
top commits.
Small UX wins for the rs comments list, in two rounds. Round 1: the top bar animates between
the normal and selection states, tapping a comment's avatar toggles selection (a discoverable
alternative to long-press), and re-tapping the active tab scrolls its list back to the top.
Round 2: back now dismisses the selection instead of leaving the screen (matching the legacy
action mode), the avatar crossfades to the checkmark on selection, and the selection bar uses
a distinct container color so the mode change is visible.
Notes: long-press haptics needed no code — Compose Foundation 1.11.4's
combinedClickablealready performs them by default. A "Select all" overflow action was added and then removed:
on a paginated list it only selects the loaded comments, which reads as selecting everything
on the tab.
Testing instructions
On #23062's setup (flag on, WP.com or app-password site):
Selection polish:
Scroll to top: