Dynamic Dashboard: Reviews card states and unit tests #12806

hafizrahman · 2024-05-21T23:53:21Z

Please do not merge until target is trunk

Closes: #12742

Description

This PR completes the Reviews card task by adding the following:

Finalizing UI for loading state,
UI for error state,
UI for empty state (both for when there's no reviews at all in the site, and when there's no reviews after filtering). The wording here follows Android's implementation.
Dismiss and onError retry trackings
Unit tests

Testing instructions

Please check and compare with screenshots posted below:

Start with no internet, ensure "error state" is shown.
Retry, ensure this is tracked: Tracked dynamic_dashboard_card_retry_tapped, ... with type: reviews included.
Restart test, now with internet on a site with no reviews. ensure "Empty state case: absolutely no reviews" case shown.
Add one review on the site, but do not approve it. Go back to site, filter by "Approved". Ensure "Empty state case: no filtered reviews" case is shown.
Try reloading and ensure loading state is shown correctly.
HIde the card, ensure this is tracked: Tracked dynamic_dashboard_hide_card_tapped, ... with type: reviews included.

Screenshots

Error case:

Empty state case: absolutely no reviews	Empty state case: no filtered reviews

Loading state case: no reviews ever loaded	Loading state case: some reviews were shown before

wpmobilebot · 2024-05-22T00:01:32Z

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

	App Name	WooCommerce iOS
	Build Number	pr12806-0724980
	Version	18.8
	Bundle ID	com.automattic.alpha.woocommerce
	Commit	`0724980`
	App Center Build	WooCommerce - Prototype Builds #9330

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

itsmeichigo · 2024-05-22T05:56:48Z

WooCommerce/Classes/ViewRelated/Dashboard/Reviews/ReviewsDashboardCard.swift

+
+    func emptyView(isFiltered: Bool) -> some View {
+        VStack(spacing: 0) {
+            ReviewsDashboardEmptyView(isFiltered: isFiltered)


Nit: this view is simple so can be part of this file, not necessary to be separate unless it's to be reused.

good point, updated in 811f143

itsmeichigo · 2024-05-22T05:58:47Z

WooCommerce/Classes/ViewRelated/Dashboard/Reviews/ReviewsDashboardEmptyView.swift

+
+        static let noFilteredReviewsText = NSLocalizedString(
+            "mostActiveCouponsEmptyView.noFilteredReviewsText",
+            value: "No reviews match the selected filter, please try changing the filter.",


Nit: if this view is kept in ReviewDashboardCard, you can use the current filter to display in the message. Something like: "No reviews matching Hold status". This would be more explicit and concise.

Yup, that's more useful. Added in 811f143

itsmeichigo · 2024-05-22T06:05:51Z

...erce/WooCommerceTests/ViewRelated/Dashboard/Reviews/ReviewsDashboardCardViewModelTests.swift

+        await viewModel.reloadData()
+
+        // Then
+        XCTAssertEqual(viewModel.data.count, self.sampleReviews.count)


We should compare the data rather than just the count, to make sure that the details are correct.

Thanks, updated in 8739e01

Conflicts: WooCommerce/Classes/ViewRelated/Dashboard/Reviews/ReviewsDashboardCard.swift

Conflicts: WooCommerce/WooCommerce.xcodeproj/project.pbxproj

… message.

itsmeichigo

Hey @hafizrahman 👋 As discussed in the previous PR, I think we should not always show the review list when there is existing data. From my testing, when refreshing the card, we show the list without the shimmering animation, making it confusing to understand if data is being reloaded.

hafizrahman · 2024-05-29T05:54:39Z

@itsmeichigo I think something might have gone wrong / unaccounted for after merging from the latest branch, I was just responding to the previous feedbacks without adding any logic change. Let me check, thanks!

dangermattic · 2024-05-30T03:27:53Z

	1 Warning
⚠️	This PR is assigned to the milestone 18.9. This milestone is due in less than 2 days. Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

hafizrahman · 2024-05-30T03:44:34Z

@itsmeichigo I decided to revamp the loading state to match similar behavior in InboxDashboardCard and have placeholder data displayed always when loading. I also simplified the syncing to only show items when there's full text with product names and notifications loaded, just makes things easier to follow that way.

Added in b758898 and 8d87f7f

Video of latest loading state:

Simulator.Screen.Recording.-.iPhone.15.iOS.17.4.-.2024-05-30.at.10.37.49.mp4

itsmeichigo

Thanks Hafiz, LGTM

hafizrahman added 3 commits May 21, 2024 22:14

Add empty state.

818994c

Add error handling

eba77d8

Add dismiss tracking

0c517ec

hafizrahman changed the base branch from trunk to feat/12742-review-card-functionality-ii May 21, 2024 23:53

hafizrahman added 2 commits May 22, 2024 09:18

Merge branch 'feat/12742-review-card-functionality-ii'

a768992

Add unit tests.

037849d

hafizrahman force-pushed the feat/12742-review-card-states-tests branch from ee015b2 to 037849d Compare May 22, 2024 02:19

hafizrahman mentioned this pull request May 22, 2024

Dynamic Dashboard: Various states and tracking for Reviews card #12787

Closed

1 task

hafizrahman added 2 commits May 22, 2024 09:26

Hide filter divider when loading to reduce extra line showing.

6d7f514

Update empty state wordings matching Android.

74b5358

hafizrahman marked this pull request as ready for review May 22, 2024 02:44

hafizrahman added the type: task An internally driven task. label May 22, 2024

hafizrahman added this to the 18.8 milestone May 22, 2024

hafizrahman requested review from selanthiraiyan and itsmeichigo May 22, 2024 02:44

hafizrahman added the status: do not merge Dependent on another PR, ready for review but not ready for merge. label May 22, 2024

itsmeichigo self-assigned this May 22, 2024

itsmeichigo reviewed May 22, 2024

View reviewed changes

hafizrahman mentioned this pull request May 22, 2024

Dynamic Dashboard: Reviews Card Reload Functionality Part 2 #12769

Merged

itsmeichigo modified the milestones: 18.8, 18.9 May 24, 2024

Base automatically changed from feat/12742-review-card-functionality-ii to trunk May 28, 2024 05:00

hafizrahman added 5 commits May 28, 2024 16:40

Merge branch 'trunk'

ac6f9ab

Conflicts: WooCommerce/Classes/ViewRelated/Dashboard/Reviews/ReviewsDashboardCard.swift

Fix post-merge issue.

4da0e36

Fix some broken unit tests.

8739e01

Refactor to update data using fetchedReviews as it becomes available.

35f5113

Merge branch 'trunk'

f25f27d

Conflicts: WooCommerce/WooCommerce.xcodeproj/project.pbxproj

hafizrahman removed the status: do not merge Dependent on another PR, ready for review but not ready for merge. label May 29, 2024

Refactor empty view to simplify and add filter name in the empty view…

811f143

… message.

hafizrahman requested a review from itsmeichigo May 29, 2024 03:22

Add unit test for switchingStatus

6f8c7a6

itsmeichigo reviewed May 29, 2024

View reviewed changes

Revamp loading state to show shimmering placeholder data always.

b758898

Simplify loading to just use syncingData

8d87f7f

hafizrahman requested a review from itsmeichigo May 30, 2024 03:45

Update unit tests

0724980

itsmeichigo approved these changes May 30, 2024

View reviewed changes

hafizrahman merged commit 34b8bca into trunk May 31, 2024
22 checks passed

hafizrahman deleted the feat/12742-review-card-states-tests branch May 31, 2024 02:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dynamic Dashboard: Reviews card states and unit tests #12806

Dynamic Dashboard: Reviews card states and unit tests #12806

hafizrahman commented May 21, 2024 •

edited

Loading

wpmobilebot commented May 22, 2024 •

edited

Loading

itsmeichigo May 22, 2024

hafizrahman May 29, 2024

itsmeichigo May 22, 2024

hafizrahman May 29, 2024

itsmeichigo May 22, 2024

hafizrahman May 29, 2024

itsmeichigo left a comment

hafizrahman commented May 29, 2024

dangermattic commented May 30, 2024

hafizrahman commented May 30, 2024 •

edited

Loading

itsmeichigo left a comment

Dynamic Dashboard: Reviews card states and unit tests #12806

Dynamic Dashboard: Reviews card states and unit tests #12806

Conversation

hafizrahman commented May 21, 2024 • edited Loading

Description

Testing instructions

Screenshots

Error case:

wpmobilebot commented May 22, 2024 • edited Loading

itsmeichigo May 22, 2024

Choose a reason for hiding this comment

hafizrahman May 29, 2024

Choose a reason for hiding this comment

itsmeichigo May 22, 2024

Choose a reason for hiding this comment

hafizrahman May 29, 2024

Choose a reason for hiding this comment

itsmeichigo May 22, 2024

Choose a reason for hiding this comment

hafizrahman May 29, 2024

Choose a reason for hiding this comment

itsmeichigo left a comment

Choose a reason for hiding this comment

hafizrahman commented May 29, 2024

dangermattic commented May 30, 2024

hafizrahman commented May 30, 2024 • edited Loading

itsmeichigo left a comment

Choose a reason for hiding this comment

hafizrahman commented May 21, 2024 •

edited

Loading

wpmobilebot commented May 22, 2024 •

edited

Loading

hafizrahman commented May 30, 2024 •

edited

Loading