Skip to content

Conversation

@backwardstruck
Copy link
Contributor

@backwardstruck backwardstruck commented Jul 2, 2024

Closes:

#11861

Description

Adds a compose card underneath both totals and cart screens in the POS feature. Currently, totals and cart are shown as two separate cards when in the checkout state. This enhancement ensures a unified card appearance that adheres to the Figma design.

Changes include modifications to WooPosHomeScreen to integrate the compose card that envelops both the totals and cart sections.

New Design:

Screenshot 2024-07-05 at 15 54 14

Old Design:

Screenshot 2024-07-04 at 16 58 05

Steps to reproduce

To test the changes made in this PR, follow the steps outlined below:

  1. Navigate to the POS screen
  2. Add items to the cart and proceed to checkout.
  3. Ensure the design specifications are met, focusing on layout and style consistency.

Testing information

  • Devices Used: Tested on tablet emulator.
  • Alternate Workflows:
    • Verify edge cases where the cart has numerous items or is empty.
  • Affected Areas:
    • Cart screen
    • Totals screen
  • Critical Flows:
    • Ensure no functional regressions occur in the checkout process.

Release Notes

  • 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.

@backwardstruck backwardstruck changed the base branch from trunk to 11714-woo-pos-show-only-small-part-of-the-cart-screen-when-there-are-not-products-selected July 2, 2024 19:39
@dangermattic
Copy link
Collaborator

dangermattic commented Jul 2, 2024

1 Warning
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@backwardstruck backwardstruck changed the base branch from 11714-woo-pos-show-only-small-part-of-the-cart-screen-when-there-are-not-products-selected to 11867-woo-pos-hide-cart-when-payment-succeed July 2, 2024 19:41
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jul 2, 2024

📲 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
FlavorJalapeno
Build TypeDebug
Commitcc8dc7c
Direct Downloadwoocommerce-wear-prototype-build-pr11892-cc8dc7c.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jul 2, 2024

📲 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
FlavorJalapeno
Build TypeDebug
Commitcc8dc7c
Direct Downloadwoocommerce-prototype-build-pr11892-cc8dc7c.apk

Base automatically changed from 11867-woo-pos-hide-cart-when-payment-succeed to trunk July 4, 2024 14:00
@backwardstruck backwardstruck marked this pull request as ready for review July 4, 2024 20:55
@backwardstruck backwardstruck requested a review from kidinov July 4, 2024 20:55
@backwardstruck
Copy link
Contributor Author

@kidinov I think it's pretty close to the design now (although not quite). Should I try to make further changes in this PR or ticket out the remaining modifications?

@kidinov
Copy link
Contributor

kidinov commented Jul 5, 2024

@backwardstruck 👋

I noticed that the margin is not correct now (weirdly that on your screenshot it's ok 🤔 ). The loading state also has an incorrect margin. The success screen lost it's margin too

Having said that, now that we are getting m2 designs, which do not contain this at all, I am wondering if it makes sense to make these changes at all, wdyt?

image image image

@backwardstruck
Copy link
Contributor Author

@backwardstruck 👋

I noticed that the margin is not correct now (weirdly that on your screenshot it's ok 🤔 ). The loading state also has an incorrect margin. The success screen lost it's margin too

Having said that, now that we are getting m2 designs, which do not contain this at all, I am wondering if it makes sense to make these changes at all, wdyt?

image image image

Ah, that screenshot is from the design, not the app. So that's why it's 100% correct.

You're right, about the margins.

I didn't realize the M2 designs were so different. Let me check and potentially close this PR then.

@backwardstruck backwardstruck added the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Jul 5, 2024
@backwardstruck
Copy link
Contributor Author

Final decision on this pending Slack discussion:

https://a8c.slack.com/archives/C070SJRA8DP/p1720190008512919?thread_ts=1720104580.663059&cid=C070SJRA8DP

@backwardstruck backwardstruck changed the title [Woo POS] Add Compose Card Under Cart and Totals as per Design [Woo POS] Update checkout screen per M2 Design Jul 5, 2024
@backwardstruck backwardstruck removed the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Jul 5, 2024
@backwardstruck
Copy link
Contributor Author

@kidinov this is updated to simply remove the cards from cart and totals screens. I made a separate ticket to finish updating both cart and totals screens for the M2 design (e.g. replacing the Collect Card Payment button):
#11933

@backwardstruck backwardstruck linked an issue Jul 10, 2024 that may be closed by this pull request
@backwardstruck backwardstruck enabled auto-merge July 10, 2024 22:36
@kidinov kidinov self-assigned this Jul 12, 2024
@kidinov
Copy link
Contributor

kidinov commented Jul 12, 2024

@backwardstruck 👋

Looks good! I just noticed the margins may be a bit too big on the cart. And the margin between products and the cart a bit too small. Also, maybe changes from that PR can be used to avoid merge conflicts and reuse the method that adapts margins for smaller screens. It should be merged to the trunk soon)

Screenshot 2024-07-12 at 10 08 31 image

…nd-totals-as-per-design

# Conflicts:
#	WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/WooPosHomeScreen.kt
#	WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartScreen.kt
@backwardstruck backwardstruck force-pushed the 11861-woo-pos-add-compose-card-under-cart-and-totals-as-per-design branch from 7ed6ec7 to ec526b3 Compare July 12, 2024 19:21
…nd-totals-as-per-design

# Conflicts:
#	WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/WooPosHomeScreen.kt
#	WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartScreen.kt
@backwardstruck
Copy link
Contributor Author

Thanks for the additional review @kidinov It should be closer to the design now:

  • Fixed the margins may be a bit too big on the cart.

  • Fixed the margin between products and the cart a bit too small.

  • Reused the new toAdaptivePadding method that adapts margins for smaller screens.

@backwardstruck backwardstruck changed the title [Woo POS] Update checkout screen per M2 Design [Woo POS] Small updates to checkout screen per M2 Design Jul 12, 2024
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 40.01%. Comparing base (9f2048d) to head (cbf4362).
Report is 8 commits behind head on trunk.

Additional details and impacted files
@@             Coverage Diff              @@
##              trunk   #11892      +/-   ##
============================================
- Coverage     40.01%   40.01%   -0.01%     
  Complexity     5432     5432              
============================================
  Files          1184     1184              
  Lines         67012    67012              
  Branches       9335     9335              
============================================
- Hits          26815    26814       -1     
- Misses        37707    37708       +1     
  Partials       2490     2490              

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

Copy link
Contributor

@kidinov kidinov left a comment

Choose a reason for hiding this comment

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

I think start/end padding are different now on the totals and on the cart screens, but I think it's fine to move on with this, as we anyway will have to redo those screens as they changed in m2

image

@backwardstruck backwardstruck merged commit 56453a6 into trunk Jul 15, 2024
@backwardstruck backwardstruck deleted the 11861-woo-pos-add-compose-card-under-cart-and-totals-as-per-design branch July 15, 2024 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Woo POS] Add Compose card under cart and totals as per design

6 participants