Comment details trash moderation redesign #20952
Conversation
Generated by 🚫 Danger |
|
 |
App Name |  WordPress |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr20952-d0fa51a | |
| Commit | d0fa51a | |
| Direct Download | wordpress-prototype-build-pr20952-d0fa51a.apk |
|
 |
App Name |  Jetpack |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr20952-d0fa51a | |
| Commit | d0fa51a | |
| Direct Download | jetpack-prototype-build-pr20952-d0fa51a.apk |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/notifications_refresh_p2 #20952 +/- ##
====================================================================
- Coverage 40.98% 40.98% -0.01%
====================================================================
Files 1523 1523
Lines 69626 69628 +2
Branches 11515 11516 +1
====================================================================
Hits 28534 28534
- Misses 38504 38505 +1
- Partials 2588 2589 +1 ☔ View full report in Codecov by Sentry. |
|
| @@ -687,33 +690,14 @@ private void moderateComment( | |||
| // Fire the appropriate listener if we have one | |||
| if (note != null && mOnNoteCommentActionListener != null) { | |||
| mOnNoteCommentActionListener.onModerateCommentForNote(note, newStatus); | |||
| dispatchModerationAction(site, comment, newStatus); | |||
| mViewModel.dispatchModerationAction(site, comment, newStatus); | |||
| } else if (mOnCommentActionListener != null) { | |||
| mOnCommentActionListener.onModerateComment(comment, newStatus); | |||
| // Sad, but onModerateComment does the moderation itself (due to the undo bar), this should be refactored, | |||
There was a problem hiding this comment.
According to this comment, we cannot dispatchModerationAction when the comment is from comment list. So I keep the current behavior, every time we moderate the comment, it will finish the screen (when it is from comment list).
| } else if (mOnCommentActionListener != null) { | ||
| mOnCommentActionListener.onModerateComment(comment, newStatus); | ||
| // Sad, but onModerateComment does the moderation itself (due to the undo bar), this should be refactored, | ||
| // That's why we don't call dispatchModerationAction() here. | ||
| } | ||
| } | ||
|
|
||
| private void dispatchModerationAction( |
There was a problem hiding this comment.
Moved to view model
| @@ -687,33 +690,14 @@ private void moderateComment( | |||
| // Fire the appropriate listener if we have one | |||
| if (note != null && mOnNoteCommentActionListener != null) { | |||
| mOnNoteCommentActionListener.onModerateCommentForNote(note, newStatus); | |||
| dispatchModerationAction(site, comment, newStatus); | |||
| mViewModel.dispatchModerationAction(site, comment, newStatus); | |||
There was a problem hiding this comment.
When the comment is from a notification, we can dispatchModerationAction so it can update the comment status without leaving the screen.
P.S. When DELETE a comment, it will finish the screen after the deletion.
There was a problem hiding this comment.
Awesome work @jarvislin ⭐
The code looks great and the app worked as expected in various configurations on a Pixel 8 / Android 14 🎉
|
@jarvislin Just noticed a minor detail we may handle separately. When spamming a comment the UI still reads |
Implemented
https://github.com/Automattic/wordpress-mobile/issues/41
https://github.com/Automattic/wordpress-mobile/issues/40
This PR contains features related to comment moderation. The changes mostly are in the moderation bottom sheet. Every action button is implemented on the bottom sheet in this PR.
To Test:
NotificationsComment...iconP.S. It will update the moderation status UI directly if the comment is from a notification (not including deletion action). But it will finish the screen if the comment is from the comment list.
Regression Notes
Potential unintended areas of impact
What I did to test those areas of impact (or what existing automated tests I relied on)
What automated tests I added (or what prevented me from doing so)
PR Submission Checklist:
skipped
Testing Checklist (strike-out the not-applying and unnecessary ones):
skipped