[Woo POS][Design System] Reimplement 0.5 width for 4 payment error views #15249
Conversation
…text and buttons content are 0.5 width.
…nd string to enable 0.5 width with external padding removed.
…ard coded max width to enable 0.5 width for CTAs.
…out to enable 0.5 width for the button.
|
|
…ageViewModel` has one less property.
|
@coderabbitai review |
|
@jaclync in your screenshots, the |
WalkthroughThe update removes the redundant Changes
Sequence Diagram(s)sequenceDiagram
participant PaymentProcess
participant ErrorVM as Error ViewModel
participant ErrorView as Error Message View
participant TotalsHelper as TotalsViewHelper
participant TotalsView as Totals View
PaymentProcess->>ErrorVM: Capture payment error
ErrorVM-->>ErrorView: Provide updated error message (no nextStep)
ErrorView->>UI: Render error layout (fixed width, spacers, max dimensions)
TotalsView->>TotalsHelper: shouldApplyPadding(.card(.paymentError))
TotalsHelper-->>TotalsView: Return false (no extra padding)
TotalsView->>UI: Render Payment Error Layout with PaymentViewLayout
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (9)
💤 Files with no reviewable changes (1)
🔇 Additional comments (20)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
In the largest font size in shorter tablets like iPad mini, the error message text views can get tall with 0.5 width:
However, after some thinking this is an edge case and the CTA is still tappable it might be better going with UI/UX consistency. I updated the text container in the non-retryable error view to be 0.5 width in 2bf7c2a. Feel free to share other thoughts! |
For payment intent creation:
Note that when you do this, the Terminal SDK has some heuristic that means it starts saying you're not connected to the internet when you retry. If you wait a couple of minutes, it'll work again. I don't think this will happen so consistently for real reproductions. For the capture error, see the instructions in #13642 |
There was a problem hiding this comment.
Works well, thanks for the changes.
Re: larger dynamic text sizes... as they increase, the design starts to look very unbalanced when it's all constrained to 50% width, and it doesn't make the best use of space.
I think we should increase the percentage width we use as the dynamic type size increases. This could just be "go direct to 100% when we hit accessibility2" or something like that – use your best judgement 😊
| @@ -323,7 +323,10 @@ private extension TotalsView { | |||
| case .validatingOrderError: | |||
| return .outlined | |||
| case .paymentError: | |||
| return .topAligned | |||
There was a problem hiding this comment.
I think we can remove the topAligned definition if we're not using it here. Or update it to match this new one.
Part of #15235
Description
This pull request updates the layout of 4 payment error message views to match the latest design so that CTAs have 0.5 of the container width. It was attempted before, but reverted because the container was often capped to a width that is not filling the available space.
Localization and Message Updates:
nextStepproperty and combined its message into themessageproperty inPointOfSaleCardPresentPaymentCaptureErrorMessageViewModel(WooCommerce/Classes/POS/Presentation/Card Present Payments/Reader Messages/PointOfSaleCardPresentPaymentCaptureErrorMessageViewModel.swift). [1] [2] [3]UI Layout Adjustments:
PointOfSaleCardPresentPaymentCaptureErrorMessageView(WooCommerce/Classes/POS/Presentation/CardReaderConnection/UI States/Reader Messages/PointOfSaleCardPresentPaymentCaptureErrorMessageView.swift). [1] [2]PointOfSaleCardPresentPaymentErrorMessageView(WooCommerce/Classes/POS/Presentation/CardReaderConnection/UI States/Reader Messages/PointOfSaleCardPresentPaymentErrorMessageView.swift). [1] [2] [3]PointOfSaleCardPresentPaymentIntentCreationErrorMessageView(WooCommerce/Classes/POS/Presentation/CardReaderConnection/UI States/Reader Messages/PointOfSaleCardPresentPaymentIntentCreationErrorMessageView.swift).PointOfSaleCardPresentPaymentNonRetryableErrorMessageView(WooCommerce/Classes/POS/Presentation/CardReaderConnection/UI States/Reader Messages/PointOfSaleCardPresentPaymentNonRetryableErrorMessageView.swift). [1] [2]Payment Layout Adjustments:
paymentErrorstate inTotalsViewto remove additional padding (WooCommerce/Classes/POS/Presentation/TotalsView.swift).TotalsViewHelperto remove additional padding for thepaymentErrorstate (WooCommerce/Classes/POS/ViewHelpers/TotalsViewHelper.swift).Steps to reproduce
Not all errors are easily reproducible. Retryable and non-retryable payment errors are the easiest to repro, please feel free to share repro steps for the "capture error" and "payment intent creation" error.
Connect your readerin the totals viewTesting information
Feel free to also test other error cases by replacing the implementation of
woocommerce-ios/WooCommerce/Classes/POS/Card Present Payments/CardPresentPaymentsAlertPresenterAdaptor.swift
Line 22 in 430e2a5
to send the specific error, and test it by connecting the reader in the totals view after adding item(s) to cart and checking out. For example, to repro the "capture error" state:
Screenshots
PointOfSaleCardPresentPaymentIntentCreationErrorMessageView
PointOfSaleCardPresentPaymentCaptureErrorMessageView
PointOfSaleCardPresentPaymentNonRetryableErrorMessageView
PointOfSaleCardPresentPaymentErrorMessageView
RELEASE-NOTES.txtif necessary.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:
Summary by CodeRabbit
Refactor
Style