[Notifications Refresh] Improve comment moderation views transition smoothness

[salimbraksa]

Description

As the black “Approve Comment” button fades out, it creates an unusual visual effect against the lighter background, potentially detracting from the overall smoothness of the transition. This PR addresses this issue to ensure a smoother animation.

Old Transition	New Transition
Screen.Recording.2024-04-30.at.18.27.02.mov	CleanShot.2024-05-05.at.20.13.28.mp4

CleanShot.2024-05-05.at.19.05.09.mp4

Test Instructions

1. Go to CommentModerationView.swift.
2. Enable Previews.
3. Tap the buttons at the centers to change the moderation state.
4. Verify the transition is smooth.

Regression Notes

1. Potential unintended areas of impact
   N/A

2. What I did to test those areas of impact (or what existing automated tests I relied on)
   N/A

3. What automated tests I added (or what prevented me from doing so)
   N/A

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:

• WordPress.com sites and self-hosted Jetpack sites.
• Portrait and landscape orientations.
• Light and dark modes.
• Fonts: Larger, smaller and bold text.
• High contrast.
• VoiceOver.
• Languages with large words or with letters/accents not frequently used in English.
• Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
• iPhone and iPad.
• Multi-tasking: Split view and Slide over. (iPad)

[salimbraksa]

1. I removed the liked case as it seems more like a substate of the approved state rather than a distinct state within the moderation view.
2. I added the spam state.

[wpmobilebot]

📲 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	pr23157-fc2c176
	Version	24.7
	Bundle ID	org.wordpress.alpha
	Commit	fc2c176
	App Center Build	WPiOS - One-Offs #9843

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

[wpmobilebot]

📲 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	pr23157-fc2c176
	Version	24.7
	Bundle ID	com.jetpack.alpha
	Commit	fc2c176
	App Center Build	jetpack-installable-builds #8891

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

[salimbraksa]

My understanding is that the previous implementation featured the moderation view as a single entity with three components:

• The title
• The main action view
• The secondary view

These subviews would change depending on the viewModel.state. I tried to fix the transition glitch without altering the overall structure too much, but it was challenging. Consequently, I've shifted to a new approach where:

• Each viewModel.state is now represented as a distinct view.
• Switching states involves simply transitioning between these views.

[salimbraksa]

When transitioning between two views, such as from Pending to Approved, the removal animation for Pending starts immediately, while the insertion animation for Approved is delayed. This approach helps prevent the Approve Comment black button from clashing visually with the Reply view.

[alpavanoglu]

Would it not be possible to apply this logic without altering the structure as much? I feel like we had to add substantially more code to achieve an improvement on animation (the +- doesn't fully represent it as we removed bunch of #Preview code here).

I personally found the changes more complicated than the older version.

[salimbraksa]

Would it not be possible to apply this logic without altering the structure as much?

My initial instinct was to enhance the animation without changing the structure too much, but I wasn't able to get the results I wanted.

For instance, I tried grouping the titleHStack, mainActionView, and secondaryActionView within a Group view and then applied the .transition to the entire group. It almost worked:

var body: some View {
    VStack(spacing: .DS.Padding.double) {
        Divider()
            .foregroundStyle(Color.DS.Background.secondary)
        VStack(spacing: .DS.Padding.double) {
            Group {
                titleHStack
                mainActionView
                secondaryActionView
            }
            .transition(
                .asymmetric(
                    insertion: .opacity.animation(animation.delay(animationDuration * 0.8)),
                    removal: .opacity.animation(animation)
                )
            )
        }
        .padding(.horizontal, .DS.Padding.double)
    }
}

And here's the result. As you can see, we almost achieved what we were aiming for, but if you look closely, you'll notice that the title doesn’t animate for some reason.

CleanShot.2024-05-09.at.00.42.40.mp4

I've tried another solution of applying the .transition modifier on each individual view, but then the code started to get complicated because of the duplicated .transition calls.

[salimbraksa]

I'm pretty sure there's a way to enhance the animation using the previous structure, but it seemed easier to represent each state as a single unit and then apply the transition to the entire view.

If you feel strongly about keeping the previous structure, I can spend more time to look into this. Let me know what you think. 🙇

[salimbraksa]

I personally found the changes more complicated than the older version.

Could you please elaborate on this a bit more?

For example, if you look at this code, I think it's pretty clear that each viewModel.state is represented by a distinct view. And the implementation of Pending, Approved, and TrashSpam should be clear too. Let me know if you think otherwise. 🙇

    var body: some View {
        VStack(spacing: .DS.Padding.double) {
            Divider()
                .foregroundStyle(Color.DS.Background.secondary)
            VStack(spacing: .DS.Padding.double) {
                switch viewModel.state {
                case .pending:
                    Pending(viewModel: viewModel)
                case .approved(let liked):
                    Approved(viewModel: viewModel, liked: liked)
                case .trash, .spam:
                    TrashSpam(viewModel: viewModel)
                }
            }
            .padding(.horizontal, .DS.Padding.double)
        }
        .animation(.smooth, value: viewModel.state)
    }

[alpavanoglu]

I personally found the changes more complicated than the older version.

Before there was a View with 3 optional components. Now there's a container view, a Generic View to reduce code duplication, and 3 different views for each state. The call-site is clear, though I think the implementation below is overly complicated imho due to the extra layering, Generic View with dynamic arguments and onChange modifier to update the title.

However the animation looks smoother. I'm pretty sure we can achieve this in either implementation but this View has to be resolved for us to proceed with other tickets. I would prefer to merge this than to spend time on the old version's animation. Otherwise we'll stepping on each others' toes.

Please go ahead and merge if the PR is ready and I'll take the changes on my branch.

[salimbraksa]

though I think the implementation below is overly complicated imho due to the extra layering, Generic View with dynamic arguments and onChange modifier to update the title.

Thank you for the feedback. I'll create a separate PR to address these points. 🙇

[salimbraksa]

This modifier animates the entire view's size when the state changes.

[justtwago]

Thanks for the enhancement, Salim! The animations look really cool!
I do like that each case is separated into its own view. In case we need to redesign a particular state - we would have less chances to screw other states :)

[alpavanoglu]

Would it not be possible to apply this logic without altering the structure as much? I feel like we had to add substantially more code to achieve an improvement on animation (the +- doesn't fully represent it as we removed bunch of #Preview code here).

I personally found the changes more complicated than the older version.

[alpavanoglu]

Nitpick: Should we not define a separate constant and specify that it is for insertion duration rather than multiplying it here?

[salimbraksa]

Resolved in fc2c176