[Notifications Refresh] Resolve an issue where the comment moderation sheet not resizing properly when comment status changes #23272

salimbraksa · 2024-05-29T11:52:48Z

Description

This PR resolves an issue where the moderation view doesn't resize properly when the comment status changes.

Before	After
	CleanShot.2024-05-29.at.13.10.58.mp4

Test Instructions

Setup a comment in either the Trash or Spam state.
Navigate to that notification comment.
Change its status to Approved.
Verify the moderation view height looks good and not squashed
Go back to the previous screen.
Navigate to the same comment again.
Verify the moderation view height is the same as before

Regression Notes

Potential unintended areas of impact
Smoke test the comment moderation screen:

Verify the content preview at the top works as expected
Post a long comment and verify scrolling works
Tap the comment menu button

What I did to test those areas of impact (or what existing automated tests I relied on)
Manual testing
What automated tests I added (or what prevented me from doing so)
None.

PR submission checklist:

I have completed the Regression Notes.
I have considered adding unit tests for my changes.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing checklist:

…laid out properly

wpmobilebot · 2024-05-29T12:01:52Z

📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.

	App Name	WordPress Alpha
	Configuration	Release-Alpha
	Build Number	pr23272-304dfc9
	Version	24.8
	Bundle ID	org.wordpress.alpha
	Commit	`304dfc9`
	App Center Build	WPiOS - One-Offs #10056

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

wpmobilebot · 2024-05-29T12:02:05Z

📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.

	App Name	Jetpack Alpha
	Configuration	Release-Alpha
	Build Number	pr23272-304dfc9
	Version	24.8
	Bundle ID	com.jetpack.alpha
	Commit	`304dfc9`
	App Center Build	jetpack-installable-builds #9105

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

salimbraksa · 2024-05-29T15:42:46Z

...Classes/ViewRelated/Notifications/Comment Moderation/CommentModerationSheetHostingView.swift

+import UIKit
+import SwiftUI
+
+final class CommentModerationSheetHostingView: UIView {


This is the view that fixes this bug.

For context, the bug was occurring because the moderation hosting view was not resizing automatically when its content changed. This is a known issue in UIKit and SwiftUI integration, where the hosting view doesn't resize when the SwiftUI view's size changes.

While it's possible to resize the hosting view by calling hostingView.invalidateIntrinsicContentSize, the resize will occur without animation which might break the moderation view animation.

There is a workaround to resolve this issue and it works as following:

Constrain the moderation hosting view to the edges of viewController.view so that it matches the size of its parent view.

By default, the Hosting View will prevent touches from passing through. To allow touches outside the moderation view to pass through, we are overriding the hosting view hitTest method.

Make the Hosting View transparent.

Now the Bottom Sheet can resize freely, and the user doesn't see the “Hosting View”.

salimbraksa · 2024-05-29T15:50:23Z

...Classes/ViewRelated/Notifications/Comment Moderation/CommentModerationSheetHostingView.swift

+              hitView === hostingView
+        else {
+            return super.hitTest(point, with: event)
+        }


If the touch happens inside the moderation view (for example, if the user taps the Approve button or any other button), then everything will proceed as normal.

salimbraksa · 2024-05-29T15:53:18Z

...Classes/ViewRelated/Notifications/Comment Moderation/CommentModerationSheetHostingView.swift

+            if let respondingView = subview.hitTest(point, with: event) {
+                return respondingView
+            }
+        }


If the hosting view is tapped, the touch is forwarded to one of the hosting view's siblings. In the case of the CommentDetailViewController, the viewController.view has two subviews: the tableView and the hostingView. Therefore, the touch is forward to the tableView.

salimbraksa · 2024-05-29T15:56:28Z

WordPress/Classes/ViewRelated/Comments/Content/CommentDetailContentTableViewCell.swift

@@ -67,6 +67,7 @@ private extension CommentDetailContentTableViewCell {
            hostingController.rootView = content
        } else {
            let hostingController = UIHostingController<CommentContentHeaderView>(rootView: content)
+            hostingController.view.setContentCompressionResistancePriority(.required, for: .vertical)


Sometimes the comment cell header is compressed ( height is 0 ) and a constraints ambiguity warning is thrown in the console. Setting the compression resistance fixed this issue.

This change is not part of the fix, but I thought of including in this PR because it's just a 1 line.

salimbraksa · 2024-05-29T16:08:57Z

...Classes/ViewRelated/Notifications/Comment Moderation/CommentModerationSheetHostingView.swift

+        self.hostingController = controller
+    }
+
+    override func hitTest(_ point: CGPoint, with event: UIEvent?) -> UIView? {


ℹ️ I'm planning to unit test this method.

Resolved in 7832ce2

WordPress/Classes/Extensions/SwiftUI/View+ReadSize.swift

alpavanoglu · 2024-05-30T09:03:50Z

WordPress/Classes/ViewRelated/Comments/CommentDetailViewController.swift

@@ -187,9 +187,9 @@ class CommentDetailViewController: UIViewController, NoResultsViewHost {
    override func viewDidLoad() {
        super.viewDidLoad()
        configureView()
-        configureReplyView()
+//        configureReplyView()


Should we delete these?

I can delete the method call in this PR, but I want to keep the implementation until the "reply" logic is refactored.

Resolved in e1e6826

alpavanoglu · 2024-05-30T09:15:30Z

...Classes/ViewRelated/Notifications/Comment Moderation/CommentModerationSheetHostingView.swift

+        self.hostingController = controller
+    }
+
+    override func hitTest(_ point: CGPoint, with event: UIEvent?) -> UIView? {


Adding a short documentation to this would make sense I think as it isn't very obvious why it is needed.

Resolved in 304dfc9

alpavanoglu · 2024-05-30T13:06:19Z

🧪 Tested and it works well! Thanks for the fix @salimbraksa !

justtwago

Tested it and it worked as expected 🎸
Thanks for the fix, Salim!

salimbraksa added 4 commits May 29, 2024 12:39

Tweak Moderation Sheet UI

7a88eaa

Add a utility to read the size of a SwiftUI view

5fc2145

Resolve an issue in the comment content cell where the header wasn't …

89afc01

…laid out properly

Add a view hosting the comment moderation sheet

91d9a2b

salimbraksa added the Notifications label May 29, 2024

salimbraksa self-assigned this May 29, 2024

salimbraksa changed the base branch from trunk to feature/notifications_refresh_p2 May 29, 2024 11:53

Minor code tweaks

62f7923

salimbraksa requested a review from alpavanoglu May 29, 2024 15:24

salimbraksa assigned justtwago May 29, 2024

salimbraksa marked this pull request as ready for review May 29, 2024 15:24

salimbraksa commented May 29, 2024

View reviewed changes

salimbraksa unassigned justtwago May 29, 2024

salimbraksa requested a review from justtwago May 29, 2024 16:01

salimbraksa commented May 29, 2024

View reviewed changes

alpavanoglu reviewed May 30, 2024

View reviewed changes

justtwago approved these changes May 30, 2024

View reviewed changes

Add unit tests for CommentModerationSheetHostingView

7832ce2

salimbraksa force-pushed the task/fix-comment-moderation-height-animation branch from 9af6496 to 7832ce2 Compare May 31, 2024 15:39

salimbraksa added 2 commits May 31, 2024 16:41

Remove commented method calls

e1e6826

Add documentation clarifying the need for overriding hitTest method

304dfc9

salimbraksa requested a review from alpavanoglu May 31, 2024 17:01

alpavanoglu approved these changes May 31, 2024

View reviewed changes

salimbraksa merged commit 02d452c into feature/notifications_refresh_p2 May 31, 2024
24 checks passed

salimbraksa deleted the task/fix-comment-moderation-height-animation branch May 31, 2024 17:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Notifications Refresh] Resolve an issue where the comment moderation sheet not resizing properly when comment status changes #23272

[Notifications Refresh] Resolve an issue where the comment moderation sheet not resizing properly when comment status changes #23272

salimbraksa commented May 29, 2024 •

edited

Loading

wpmobilebot commented May 29, 2024 •

edited

Loading

wpmobilebot commented May 29, 2024 •

edited

Loading

salimbraksa May 29, 2024 •

edited

Loading

salimbraksa May 29, 2024 •

edited

Loading

salimbraksa May 29, 2024 •

edited

Loading

salimbraksa May 29, 2024 •

edited

Loading

salimbraksa May 29, 2024

salimbraksa May 31, 2024 •

edited

Loading

alpavanoglu May 30, 2024

salimbraksa May 30, 2024

salimbraksa May 31, 2024

alpavanoglu May 30, 2024

salimbraksa May 31, 2024

alpavanoglu commented May 30, 2024

justtwago left a comment

[Notifications Refresh] Resolve an issue where the comment moderation sheet not resizing properly when comment status changes #23272

[Notifications Refresh] Resolve an issue where the comment moderation sheet not resizing properly when comment status changes #23272

Conversation

salimbraksa commented May 29, 2024 • edited Loading

Description

Test Instructions

Regression Notes

PR submission checklist:

wpmobilebot commented May 29, 2024 • edited Loading

wpmobilebot commented May 29, 2024 • edited Loading

salimbraksa May 29, 2024 • edited Loading

Choose a reason for hiding this comment

salimbraksa May 29, 2024 • edited Loading

Choose a reason for hiding this comment

salimbraksa May 29, 2024 • edited Loading

Choose a reason for hiding this comment

salimbraksa May 29, 2024 • edited Loading

Choose a reason for hiding this comment

salimbraksa May 29, 2024

Choose a reason for hiding this comment

salimbraksa May 31, 2024 • edited Loading

Choose a reason for hiding this comment

alpavanoglu May 30, 2024

Choose a reason for hiding this comment

salimbraksa May 30, 2024

Choose a reason for hiding this comment

salimbraksa May 31, 2024

Choose a reason for hiding this comment

alpavanoglu May 30, 2024

Choose a reason for hiding this comment

salimbraksa May 31, 2024

Choose a reason for hiding this comment

alpavanoglu commented May 30, 2024

justtwago left a comment

Choose a reason for hiding this comment

salimbraksa commented May 29, 2024 •

edited

Loading

wpmobilebot commented May 29, 2024 •

edited

Loading

wpmobilebot commented May 29, 2024 •

edited

Loading

salimbraksa May 29, 2024 •

edited

Loading

salimbraksa May 29, 2024 •

edited

Loading

salimbraksa May 29, 2024 •

edited

Loading

salimbraksa May 29, 2024 •

edited

Loading

salimbraksa May 31, 2024 •

edited

Loading