Convert the comment editor to Compose and save via wordpress-rs - #23084
Merged
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>
Search icon in the top bar opens an inline query field (like the rs posts list): queries are debounced 250ms with a 3-character minimum, keep the active tab's status filter, and clear back to the normal tab content when search closes. Selection mode is cleared when search opens or closes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…list-search # 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 # WordPress/src/main/res/values/strings.xml
- Track and cancel in-flight load-more jobs in clearTabs(): a survivor could land on a reset page-generation counter and append its stale (unfiltered) page into fresh search results, corrupting the cursor chain - Don't resurrect a cleared tab from the stale-load-more discard branch; the ghost entry blocked initTab and left the tab shimmering forever - Keep the tab row visible while searching: the comments endpoint accepts a single status per request (unlike posts), so a search is always scoped to one tab — visible tabs make that scope explicit and switchable - Reset each tab's scroll position when search opens, changes query, or closes, so replaced content renders at the top instead of clamping to the old offset and tripping the load-more trigger - Use trimmed query lengths for the debounce filter and the idle/searching thresholds, so whitespace-only queries can't fetch the unfiltered list - Clear results on any below-minimum query (not just blank) so the previous query's rows can't flash as matches for the next one - Guard initTab against fetching during search-idle (Activity recreation or a late pager settle would bypass the debounce and minimum length) - Only clear the selection on a real tab change, not the settled-page re-emission after recreation, and request search focus once per open so a selection round-trip doesn't pop the keyboard Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Re-init the tab the user is looking at (lastTrackedTab, which follows every pager settle) instead of the tab captured at keystroke time; a tab switch inside the debounce window previously left the visible tab as a permanent shimmer with no fetch in flight. Removes the activeSearchTab field and the tab parameter from onSearchQueryChanged - Trim and dedupe the query pipeline so whitespace-only edits neither restart the debounce nor refetch, and send the trimmed query to the API - Clear displayed rows on any trimmed-query change, covering whole-string replacements (select-all + paste, keyboard suggestions, voice input) that never dip below the minimum length - Rework the scroll reset: skip the initial emission so Activity recreation keeps the restored scroll positions, and skip lists already at the top to avoid pointless forced remeasures and fling cancellation - Don't re-select failed batch-moderation ids into a search session opened while the batch ran; the untracked job outlives clearTabs and an invisible selection would swallow back presses and hijack row taps - Consolidate the minimum-length policy into one isSearchable predicate exposed as isQuerySearchable, so the screen no longer re-derives it from the ViewModel's companion constant Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Guard the failed-batch re-selection with a clear-generation counter instead of the instantaneous search flag: a batch run inside an unchanged search keeps the retry re-selection (its failed rows are still displayed), while any clearTabs() during the batch — search opened, closed, or query changed — suppresses it, closing the invisible-selection hole when search was opened and closed mid-batch - Unify the search-query policy in one flow: a shared trim+dedupe normalization feeds an immediate on-change clear and the debounced fetch, so the two halves can't drift; onSearchQueryChanged shrinks to a plain setter. The collector no longer re-clears, so a pager settle that already fetched the same query inside the debounce window keeps its results instead of being cancelled and refetched - Track the pager's current tab in a dedicated field instead of reusing the analytics dedupe key, so tracking changes can't silently retarget the debounced search - Render the blank search-idle screen (not the animated shimmer) for tabs cleared while awaiting the debounce, removing the per-keystroke results/shimmer flicker; shimmer now means an actual fetch in flight - Pass isSearchActive/isQuerySearchable to the tab screen and derive the idle/searching split inside it, removing the representable-but- impossible flag combination - Hoist the once-per-open focus request out of SearchTopBar, dropping its two ferry parameters Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Left behind when SearchTopBar's focus effect was hoisted to the caller. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
When started with a noteId, the ViewModel refreshes the note DB after every successful write, fires commentModerated for the notifications result contract, edits via NotificationCommentIdentifier, and falls back to Note.hasLikedComment() when the FluxC cache row is missing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
New fragment factory takes noteId, reply prefill and focus flags; the result intent accumulates the NOTE_MODERATE_* extras the notifications list uses to update the moderated note's row. Not yet reachable from the notifications flow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
NotificationsDetailActivity's pager now hosts the rs detail fragment for comment notes when the site resolves and the RS_UNIFIED_COMMENTS gate (renamed to shouldUseRsComments) is on; the legacy fragment stays as the fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The notifications pager paints its ViewPager2 with a placeholder gray; the rs detail fragment's root was transparent, so that gray showed through inside a notification. Match the legacy comment detail and set colorSurface on the root. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replaces the XML/View Binding UI of UnifiedCommentDetailsFragment with a pure-Compose screen: content, action footer with dropdown more-menu, confirm dialogs, snackbars, and a reply box with @-mention suggestions and a full-screen editor replacing CollapseFullScreenDialogFragment. The fragment is now a thin ComposeView host; the ViewModel is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follows the legacy displayHtmlComment pipeline: emoticon smilies become unicode emoji first, then remaining <img> tags render as width-capped block images via Coil, with link-aware text segments between them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Moves CommentDetailsActions to its own file, rewrites findMentionToken without loop jumps, and drops an unused import. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…etail-compose # Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/comments/unified/UnifiedCommentDetailsFragment.kt # WordPress/src/main/res/layout/unified_comment_details_fragment.xml
Makes the fragment's action callbacks a single lazy instance, hoists the full-screen editor's duplicated send-enabled expression, drops a wrapper Column from the mention panel, and collapses the moderate button's three near-identical branches into one call. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The More button's weight sat on the Box that anchors its dropdown, so the button itself wrapped content and hugged the slot's start edge. Fill the Box so the icon centres and the ripple covers the full slot, matching the sibling action buttons. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Fire the snackbar's onDismissAction even when the view lifecycle cancels the coroutine (rotation), so the load-error snackbar still closes the screen like the legacy Snackbar callback did on view detach. - Drop the Send IME action from the reply field so the enter key inserts a newline again (legacy textMultiLine parity); sending stays on the send button. - Only show the mention suggestion panel while the reply field is focused, so a restored draft ending in an @-token doesn't pop the panel on open. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Replace the XML unified comment editor with a Compose screen (Scaffold + TopAppBar, four fields, discard-confirm dialog), hosted in a ComposeView. - Save edits through wordpress-rs when the site supports it (works on WP.com and self-hosted application-password sites, which the FluxC updateEditComment path can't edit), mirroring the result into the FluxC cache; fall back to FluxC otherwise. - Enable the name/URL/email fields for every comment, matching the legacy editor's POST behavior. - Remove the now-unused edit XML layout, menu, and EditCancelDialogFragment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Generated by 🚫 Danger |
Contributor
|
|
Contributor
|
|
Bundle the author name/email/url into a CommentAuthor value object so CommentsRsDataSource.updateComment stays within the parameter limit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Compose editor drives its fields from editedComment, so the old shouldInitComment/shouldInitWatchers flags (which told the XML fragment when to setText and attach TextWatchers) are dead. Remove them, give EditCommentUiState sensible defaults so the loading placeholder is a one-liner, and drop the now-redundant mapInputSettings() and the recreation test that only covered the removed flag. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The comment param was only used for its id, which equals updatedComment.id (updatedComment is comment.copy(...)); use that instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ditor-compose # Conflicts: # WordPress/src/main/res/values/ids.xml
- Auto-capitalize sentences in the comment body field (parity with the legacy field and the reply box). - Remove the now-dead InputSettings/inputSettings and write-only isFromRegisteredUser state, and the tests that only asserted the hardcoded all-enabled flags. - Extract the duplicated SnackbarMessageHolder->SnackbarHostState bridge into a shared showMessage() extension, reused by the editor and detail fragments. - Disable the Save button while saving/loading to prevent a double-submit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The style was only referenced by the deleted unified_comments_edit_fragment toolbar, so it's now unused and tripped lint's UnusedResources check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## trunk #23084 +/- ##
==========================================
- Coverage 37.37% 37.36% -0.01%
==========================================
Files 2340 2340
Lines 127002 127023 +21
Branches 17510 17517 +7
==========================================
- Hits 47469 47465 -4
- Misses 75650 75671 +21
- Partials 3883 3887 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- Mirror the server's post-save comment state (author fields, KSES-filtered content) into the FluxC cache instead of the values that were sent, so server-rejected edits can't linger locally. - Keep the discard-changes dialog across rotation by driving it from ViewModel ui state instead of a one-shot event and a fragment field. - Disable IME autocorrect on the author name/URL/email fields, matching the legacy layout which only opted the comment body into textAutoCorrect. - Preserve the form's scroll position across a failed save by hoisting the scroll state above the progress swap. - Guard onActionMenuClicked against double-tap re-entrancy with an isSaving flag set before the save coroutine launches. - Render the progress label via the shared uiStringText() helper. - Move the SnackbarMessageHolder bridge to ui/compose/utils, where other Compose screens can reuse it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
nbradbury
marked this pull request as ready for review
July 10, 2026 13:51
Contributor
|
Are we ok with these two points?
|
The endpoint ignores author name/email/url for comments from registered users, so offering to edit them silently no-ops. Reinstate the legacy affordance: those fields render disabled unless the comment is from a guest, driven by a canEditAuthorFields flag on the ui state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Thanks for raising both — good catches. 1. Registered-user author fields — agreed, the silent no-op wasn't a good trade. Restored the legacy affordance in bc8969a: the name/web address/email fields now render disabled for comments from registered authors (driven by a 2. Flag scope — you're right that the editor (and rs save on capable sites) now serves all three entry points regardless of |
Contributor
Author
|
@adalpari Claude posted that previous comment, not me. I'm looking into whether all comment fields should be editable. In the "legacy" comment editor they were all editable, but this may have been a mistake. I'll let you know when this is ready for a re-review. |
Reverts the registered-user field disabling: WordPress core's REST comments controller applies author_name/author_email/author_url unconditionally on update (the user-account override exists only on create), so the edits are genuinely saved for any comment. Also corrects the code comments that claimed the endpoint ignores author identity for registered users. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Ahhh, didn't see this and already tested and approved the PR :S |
Contributor
Author
|
@adalpari After looking into this further, it appears we should make all comment fields editable. This is what the legacy editor did, and Claude did a deep dive and came to the same conclusion. review-comment-editor-author-fields-2026-07-10.pdf Let me know if you'd like to give it another spin, but otherwise I'm good to merge. |
Contributor
Cool, I'm good with merging it too. :) |
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.
Description
Converts the unified comment editor from XML/Views to Jetpack Compose and moves its save path onto wordpress-rs, continuing the comments-unification migration.
What changed
UnifiedCommentEditScreen(Compose):CommentsRsDataSource.updateComment) when the site supports it, then mirrors the result into the FluxC cache so the list/notifications stay in syncNote: the editor is shared by every comment-edit entry point, so these changes apply regardless of the RS comments experimental flag. The rs save path is deliberately not flag-gated — FluxC's
updateEditCommentcannot edit comments on self-hosted application-password sites, so rs is required there; XML-RPC self-hosted sites fall back to FluxC.Testing instructions
Test on both a WordPress.com site and a self-hosted application-password site.
Edit from the rs comment detail (requires the RS comments experimental flag, Me → Experimental Features):
Edit from the non-experimental entry points (flag OFF — these flows also use the new editor):