Skip to content

[Dynamic Dashboard] Make order items clickable #11554

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 21, 2024
Merged

Conversation

0nko
Copy link
Contributor

@0nko 0nko commented May 20, 2024

Part of #11461. The PR makes order items of the orders card clickable and tapping on them opens an order detail. The navigation first opens the order list, which then opens the order details. The order list is required because it handles order trashing.

Screen_recording_20240520_214206.webm

To test:

  1. Make sure you have the Most recent orders card selected
  2. Tap on one of the orders on the card
  3. Verify the correct card detail is opened
  4. Go back
  5. Notice the order list is visible
  6. Go back
  7. Notice the dashboard is visible
  8. Tap on an order and change the status
  9. Save the order and go back
  10. Leave the order list and notice the orders card reflects the order change

@0nko 0nko added the feature: dashboard Related to home screen project label May 20, 2024
@0nko 0nko added this to the 18.8 milestone May 20, 2024
@wpmobilebot
Copy link
Collaborator

📲 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
FlavorJalapeno
Build TypeDebug
Commitadb175b
Direct Downloadwoocommerce-prototype-build-pr11554-adb175b.apk

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 0% with 30 lines in your changes are missing coverage. Please review.

Project coverage is 40.46%. Comparing base (75b99e8) to head (adb175b).

Files Patch % Lines
...android/ui/dashboard/orders/DashboardOrdersCard.kt 0.00% 24 Missing ⚠️
...id/ui/dashboard/orders/DashboardOrdersViewModel.kt 0.00% 5 Missing ⚠️
...erce/android/ui/orders/list/OrderListRepository.kt 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              trunk   #11554      +/-   ##
============================================
- Coverage     40.47%   40.46%   -0.02%     
  Complexity     5200     5200              
============================================
  Files          1083     1083              
  Lines         62996    63015      +19     
  Branches       8623     8625       +2     
============================================
  Hits          25497    25497              
- Misses        35204    35223      +19     
  Partials       2295     2295              

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

@JorgeMucientes JorgeMucientes self-assigned this May 21, 2024
navigateSafely(
resId = R.id.orders,
navOptions = navOptions {
popUpTo(graph.findStartDestination().id) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why do you need these navOptions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It fixes an issue when you try to tap on the bottom navigation, which doesn't work without the above. Hicham found this solution here.

@JorgeMucientes
Copy link
Contributor

Nice work @0nko! Code looks good and everything works as expected. I just left a minor question about the need of popUpTo to the startDestination of the graph.
Feel free to merge :)

@0nko 0nko merged commit 0200a2c into trunk May 21, 2024
14 checks passed
@0nko 0nko deleted the issue/11461-navigate-orders branch May 21, 2024 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: dashboard Related to home screen project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants