Issue/11750 dashboard big screen support #13488

JorgeMucientes · 2025-02-07T07:33:58Z

Description

Based on the changes I worked on here: pe5sF9-3si-p2 and resulting in this PR work, I've refactored the prior implementation to avoid using LazyStaggeredGrid to support big screen in the dashboard tab. Instead, this relies on a custom implementation that will manually add 2 or 3 columns to the dashboard based on the screen's currentWindowAdaptiveInfo().windowSizeClass.

Changes added:

Removed usage of xml NestedScrollView. This was needed in order to add Composable colums with vertical scroll that work well/coordinate with the collapsing toolbar.
Refactored pull to refresh implementation to use Composable approach instead of relying in our custom view.
Adjusted columns and manually created grid to coordinate with the collapsing toolbar.

There's one thing I'm not fully convinced. To determine the number of columns, I'm using windowSizeClass.windowWidthSizeClass.

This results in bigger phones showing 3 columns in landscape (see screen recordings below). I don't think it is a major issue, but I'd say two columns would look better. Let me know what you think, although I suspect it won't be straightforward to force two columns for mobile phones on landscape mode.

Testing information

Just run the app on the phone screen and tablet screen and check that the dashboard works as expected.

Rotate the device and test the number of columns adjusts accordingly
Check pull to refresh works as expected
Check scrolling up and down works well with the collapsing toolbar
Add and remove cards at will and check that they are displayed in the right order.

The tests that have been performed

The above ☝🏼

Images/gif

Tablet Experience

TablerLayout.mp4

Large phone Eexperience (Pixel 8 pro)

LargePhone.mp4

Smaller phone experience

smallerPhone.mp4

I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on big (tablet) and small (phone) in case of UI changes, and no regressions are added.

…fresh

…agment

- Replace custom `WindowSizeClass` with `WindowWidthSizeClass` from `androidx.window.core.layout`. - Use `currentWindowAdaptiveInfo` instead custom extension that ignores screen width to calculate SizeClass - Update the number of columns in the Dashboard based on the Window Size Class. - Use a `Column` with vertical scrolling instead of `LazyColumn` when `numberOfColumns` is 1. - When multiple columns in a row instead of LazyVerticalStaggeredGrid that doesn't work well with nested scrolling interactions - Adds dependency to adaptiveAndroid

dangermattic · 2025-02-07T07:34:50Z

	1 Warning
⚠️	This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

wpmobilebot · 2025-02-07T07:38:37Z

Project dependencies changes

The following changes in project dependencies were detected (configuration vanillaReleaseRuntimeClasspath):

list

New Dependencies
androidx.compose.material3.adaptive:adaptive-android:1.0.0
androidx.window.extensions.core:core:1.0.0
androidx.window:window-core:1.3.0
androidx.window:window-core-android:1.3.0

Upgraded Dependencies
androidx.window:window:1.3.0, (changed from 1.0.0)

tree

 \--- androidx.preference:preference:1.2.1
      \--- androidx.slidingpanelayout:slidingpanelayout:1.2.0
-          \--- androidx.window:window:1.0.0
-               +--- org.jetbrains.kotlin:kotlin-stdlib:1.6.0 -> 2.1.10 (*)
-               +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.5.2 -> 1.8.1 (*)
-               +--- androidx.annotation:annotation:1.2.0 -> 1.8.1 (*)
-               +--- androidx.collection:collection:1.1.0 -> 1.4.0 (*)
-               \--- androidx.core:core:1.3.2 -> 1.13.1 (*)
+          \--- androidx.window:window:1.0.0 -> 1.3.0
+               +--- androidx.annotation:annotation:1.3.0 -> 1.8.1 (*)
+               +--- androidx.collection:collection:1.1.0 -> 1.4.0 (*)
+               +--- androidx.core:core:1.8.0 -> 1.13.1 (*)
+               +--- androidx.window.extensions.core:core:1.0.0
+               |    +--- androidx.annotation:annotation:1.6.0 -> 1.8.1 (*)
+               |    \--- org.jetbrains.kotlin:kotlin-stdlib:1.8.20 -> 2.1.10 (*)
+               +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.22 -> 2.1.10 (*)
+               +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.7.3 -> 1.8.1 (*)
+               \--- androidx.window:window-core:1.3.0 (c)
++--- androidx.compose:compose-bom:2024.09.01
+|    \--- androidx.compose.material3.adaptive:adaptive-android:1.0.0 (c)
+\--- androidx.compose.material3.adaptive:adaptive-android:1.0.0
+     +--- androidx.annotation:annotation:1.1.0 -> 1.8.1 (*)
+     +--- androidx.annotation:annotation-experimental:1.4.0 -> 1.4.1 (*)
+     +--- androidx.compose.foundation:foundation:1.6.5 -> 1.7.1 (*)
+     +--- androidx.compose.ui:ui-geometry:1.6.5 -> 1.7.1 (*)
+     +--- androidx.window:window:1.3.0 (*)
+     +--- androidx.window:window-core:1.3.0
+     |    \--- androidx.window:window-core-android:1.3.0
+     |         +--- androidx.annotation:annotation:1.7.0 -> 1.8.1 (*)
+     |         +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.22 -> 2.1.10 (*)
+     |         \--- androidx.window:window:1.3.0 (c)
+     +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.22 -> 2.1.10 (*)
+     \--- org.jetbrains.kotlin:kotlin-stdlib-common:1.8.22 -> 2.1.10 (*)

wpmobilebot · 2025-02-07T07:38:58Z

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

	App Name	WooCommerce-Wear Android
	Platform	⌚️ Wear OS
	Flavor	Jalapeno
	Build Type	Debug
	Commit	`3259fb8`
	Direct Download	`woocommerce-wear-prototype-build-pr13488-3259fb8.apk`

wpmobilebot · 2025-02-07T07:40:28Z

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

	App Name	WooCommerce Android
	Platform	📱 Mobile
	Flavor	Jalapeno
	Build Type	Debug
	Commit	`3259fb8`
	Direct Download	`woocommerce-prototype-build-pr13488-3259fb8.apk`

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/DashboardContainer.kt

+    mainActivityViewModel: MainActivityViewModel,
+    dashboardViewModel: DashboardViewModel,
+    blazeCampaignCreationDispatcher: BlazeCampaignCreationDispatcher,
+    widgetModifier: Modifier


JorgeMucientes · 2025-02-07T08:27:46Z

WooCommerce/build.gradle

@@ -411,6 +411,7 @@ dependencies {
    implementation(libs.androidx.compose.material.icons.extended)
    implementation(libs.androidx.compose.ui.text.google.fonts)
    implementation(libs.androidx.navigation.compose)
+    implementation(libs.androidx.adaptive.android)


Added this dependency to rely on official library to categorize screen size. We currently have a custom implmentation of WindowSizeClass but I think we should be using the official one. Also the custom implementation of WindowSizeClass currently only exposes windowHeightSizeClass but not windowWidthSizeClass. So instead of manually adding windowWidthSizeClass, I figured we could use the official size class library for it.

JorgeMucientes · 2025-02-07T10:02:06Z

Setting the milestone to 21.8. I noticed the code freeze hasn't happened yet. I don't want to rush this into trunk and rather leave more time for testing, as there are multiple changes in one of the main screens of the app.

pachlava

@JorgeMucientes 👋

I noticed that this fails:

> Task :WooCommerce:compileWasabiDebugAndroidTestKotlin FAILED

e: file:///Users/.../woocommerce-android/WooCommerce/src/androidTest/kotlin/com/woocommerce/android/e2e/screens/mystore/DashboardScreen.kt:17:37 Unresolved reference 'my_store_refresh_layout'.

Making tests execution impossible. This is caused by the removal of my_store_refresh_layout from fragment_dashboard.xml and probably can be fixed by replacing R.id.my_store_refresh_layout in DashboardScreen.kt:17:37 by another component (ideally, unique), the presence of which can inform that we're on the Dashboard Screen.

codecov-commenter · 2025-02-07T12:44:39Z

Codecov Report

Attention: Patch coverage is 10.37736% with 95 lines in your changes missing coverage. Please review.

Project coverage is 37.87%. Comparing base (c9748f1) to head (3259fb8).
Report is 13 commits behind head on trunk.

Files with missing lines	Patch %	Lines
...ommerce/android/ui/dashboard/DashboardContainer.kt	0.00%	89 Missing ⚠️
...ommerce/android/ui/dashboard/DashboardViewModel.kt	64.70%	4 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk   #13488      +/-   ##
============================================
- Coverage     37.89%   37.87%   -0.02%     
  Complexity     8957     8957              
============================================
  Files          2050     2050              
  Lines        112100   112166      +66     
  Branches      14173    14187      +14     
============================================
+ Hits          42476    42484       +8     
- Misses        65745    65801      +56     
- Partials       3879     3881       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pachlava

@JorgeMucientes I took a liberty of updating the missing reference in UI tests with 581d2e9, so tests-wise it's good to go now

JorgeMucientes · 2025-02-07T15:10:38Z

@JorgeMucientes I took a liberty of updating the missing reference in UI tests with 581d2e9, so tests-wise it's good to go now

Awesome @pachlava. Thanks so much for proactively taking a look into this 🥇

JorgeMucientes added 8 commits December 5, 2024 15:35

Adds grid layout for bigger screens

9939969

Extract dashboard container out of the xml NestedScroll and SwipeToRe…

83227a1

…fresh

Add a composable implementation of swipe to refresh

0eca83c

Clean xml layout and ensure we keep the same logic in the DashboardFr…

2b7fbfc

…agment

Merge branch 'trunk' into issue/11750-dashboard-big-screen-support

8d9bedf

Merge branch 'trunk' into issue/11750-dashboard-big-screen-support

86bb062

Fix unit tests after adding composable pull to refresh implementation

c72ece0

JorgeMucientes added the feature: dashboard Related to home screen project label Feb 7, 2025

JorgeMucientes added this to the 21.8 milestone Feb 7, 2025

Remove animation when cards are displayed in multiple columns

3189ad8

JorgeMucientes modified the milestones: 21.8, 21.7 Feb 7, 2025

Update release notes

5244644

github-advanced-security bot found potential problems Feb 7, 2025

View reviewed changes

Adjust dashboard card paddings

8c52d0c

JorgeMucientes requested review from hichamboushaba and irfano February 7, 2025 08:22

JorgeMucientes marked this pull request as ready for review February 7, 2025 08:23

JorgeMucientes requested a review from a team as a code owner February 7, 2025 08:23

JorgeMucientes commented Feb 7, 2025

View reviewed changes

hichamboushaba self-assigned this Feb 7, 2025

JorgeMucientes modified the milestones: 21.7, 21.8 Feb 7, 2025

pachlava requested changes Feb 7, 2025

View reviewed changes

Fix the unexisting UI component reference.

581d2e9

pachlava self-requested a review February 7, 2025 12:28

pachlava approved these changes Feb 7, 2025

View reviewed changes

JorgeMucientes added 2 commits February 7, 2025 16:13

Merge branch 'trunk' into issue/11750-dashboard-big-screen-support

42aa734

Update release notes

3259fb8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue/11750 dashboard big screen support #13488

Issue/11750 dashboard big screen support #13488

JorgeMucientes commented Feb 7, 2025 •

edited

Loading

dangermattic commented Feb 7, 2025 •

edited

Loading

wpmobilebot commented Feb 7, 2025

wpmobilebot commented Feb 7, 2025 •

edited

Loading

wpmobilebot commented Feb 7, 2025 •

edited

Loading

JorgeMucientes Feb 7, 2025 •

edited

Loading

JorgeMucientes commented Feb 7, 2025

pachlava left a comment

codecov-commenter commented Feb 7, 2025 •

edited

Loading

pachlava left a comment •

edited

Loading

JorgeMucientes commented Feb 7, 2025

Issue/11750 dashboard big screen support #13488

Are you sure you want to change the base?

Issue/11750 dashboard big screen support #13488

Conversation

JorgeMucientes commented Feb 7, 2025 • edited Loading

Description

Testing information

The tests that have been performed

Images/gif

Reviewer (or Author, in the case of optional code reviews):

dangermattic commented Feb 7, 2025 • edited Loading

wpmobilebot commented Feb 7, 2025

Project dependencies changes

wpmobilebot commented Feb 7, 2025 • edited Loading

wpmobilebot commented Feb 7, 2025 • edited Loading

JorgeMucientes Feb 7, 2025 • edited Loading

Choose a reason for hiding this comment

JorgeMucientes commented Feb 7, 2025

pachlava left a comment

Choose a reason for hiding this comment

codecov-commenter commented Feb 7, 2025 • edited Loading

Codecov Report

pachlava left a comment • edited Loading

Choose a reason for hiding this comment

JorgeMucientes commented Feb 7, 2025

JorgeMucientes commented Feb 7, 2025 •

edited

Loading

dangermattic commented Feb 7, 2025 •

edited

Loading

wpmobilebot commented Feb 7, 2025 •

edited

Loading

wpmobilebot commented Feb 7, 2025 •

edited

Loading

JorgeMucientes Feb 7, 2025 •

edited

Loading

codecov-commenter commented Feb 7, 2025 •

edited

Loading

pachlava left a comment •

edited

Loading