[Woo POS] [Cash & Receipts] UI updates from testing #14919

iamgabrielma · 2025-01-20T13:52:41Z

Description

This PR addresses some of the UI updates brought up from testing ( pdfdoF-6dp-p2 ) in order to bring the iOS UI/UX closer to what Android already has implemented. I'm intentionally leaving out some of the items brought up in testing, as we still have no clear consistent designs, and some of the items raised are platform-specific preferences.

If any, these can be handled outside of the project as needed.

Changes

Updates the "change due" text colour from "success green" to "gray":

Adjust the "mark payment as complete" button height dynamically when keyboard is presented/hidden:

Screen.Recording.2025-01-20.at.14.41.35.mov

Update the green colour used on success checkmark when dark mode, and moved it to POS constants file.

Light	Dark

Testing information

Enable acceptCashForPointOfSale flag
Go through a POS transaction, select payment with cash.
Observe that:
- When adding a higher amount than the order total, the "change due" text colour is no longer green
- Showing/hiding the keyboard makes the "mark as complete" button move along, rather than being partially hidden or being static
- Upon successful payment, in dark mode, the success checkmark circle is dark green.

I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if 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:

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 all devices (phone/tablet) and no regressions are added.

dangermattic · 2025-01-20T13:53:20Z

	1 Warning
⚠️	This PR is assigned to the milestone 21.5. 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

wpmobilebot · 2025-01-20T14:03:08Z

📲 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	pr14919-0cddff1
	Version	21.4
	Bundle ID	com.automattic.alpha.woocommerce
	Commit	`0cddff1`
	App Center Build	WooCommerce - Prototype Builds #12684

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

success text color happened to be different, but we no longer use it. Let’s move the circle success color to POS constants so we do not have different tones all around. We still keep the same values for light-dark modes

…ates

iamgabrielma · 2025-01-21T11:08:19Z

WooCommerce/Classes/POS/ViewHelpers/KeyboardObserver.swift

+import SwiftUI
+import Combine
+
+final class KeyboardObserver: ObservableObject {


Happy to get some feedback on this KeyboardObserver. I wasn't quite sure how to handle the dynamic height for the buttons when keyboard is on/off or when we present messages like error/change due. Right now it seems to do the job but I'm unaware if there are better solutions 🤔

I don't know... but you might not need any of it. I've certainly never had to worry about it in SwiftUI before.

This article suggests that we should just be able to rely on SwiftUI to handle it for us. We can wrap it in a scroll view and it might just work...

At larger dynamic type sizes we'll need one anyway; the button gets hidden by the keyboard at the moment.

…ates

joshheald

Some suggestions to try inline. For summary:

Try removing all the observation and just wrapping it in a ScrollView.
If you still need the observation, try encapsulating it in a view modifier rather than duplicating across the two views.
If that doesn't work, just tidy up the keyboard height naming and logic a bit...

I found that it's broken in various ways with larger dynamic text sizes:

Even at 90%, the disconnected card reader view has truncated text now

When you increase the text size, the cash button gets pushed off screen – here's 135%

When you get up to the very biggest sizes, it's not accessible or visible at all:

On the cash and email entry screen, it gets pushed behind the keyboard, if the keyboard is full width not floating. You can still usually get to it by pressing enter, but it would be nicer in a scrollview, I think.

joshheald · 2025-01-23T10:08:42Z

WooCommerce/Classes/POS/Presentation/Reusable Views/POSSendReceiptView.swift

+    private var keyboardHeight: CGFloat {
+        if keyboardObserver.keyboardHeight < Constants.keyboardShownButtonSpacing {
+            return Constants.keyboardShownButtonSpacing
+        } else {
+            return Constants.keyboardHiddenButtonSpacing
+        }
+    }


This doesn't return the height of the keyboard, so it's a confusing name...

joshheald · 2025-01-23T10:10:55Z

WooCommerce/Classes/POS/Presentation/Reusable Views/POSSendReceiptView.swift

@@ -15,6 +16,14 @@ struct POSSendReceiptView: View {
        EmailFormatValidator.validate(string: textFieldInput)
    }

+    private var keyboardHeight: CGFloat {
+        if keyboardObserver.keyboardHeight < Constants.keyboardShownButtonSpacing {


I don't really understand the intent of this comparison... could you explain a little?

Why would the keyboard being taller than the shown button spacing mean that we should return the shown button spacing? Surely there's some other threshold to decide that?

At the moment, there are some unexpected changes of this spacing at times, and it might be because of this comparison...

joshheald · 2025-01-23T10:13:35Z

WooCommerce/Classes/POS/Presentation/Reusable Views/POSSendReceiptView.swift

            }

+            Spacer().frame(height: keyboardHeight)


Perhaps you could make a viewModifier which you use on this spacer, so we don't declare all this keyboard observation twice?

Suggested change

Spacer().frame(height: keyboardHeight)

Spacer()

.keyboardResponsiveHeight(

withLargeKeyboard: Constants.keyboardShownButtonSpacing,

withoutKeyboard: Constants.keyboardHiddenButtonSpacing)

joshheald · 2025-01-23T10:14:38Z

WooCommerce/Classes/POS/Presentation/Reusable Views/POSSendReceiptView.swift

@@ -86,6 +98,7 @@ struct POSSendReceiptView: View {
        }
        .padding([.horizontal, .bottom])
        .animation(.easeInOut, value: errorMessage)
+        .animation(.easeInOut, value: keyboardObserver.keyboardHeight)


I'm not sure how nicely this animation will play if you do the separate viewmodifier though. It may still be possible, but check before you go too deep.

joshheald · 2025-01-23T10:18:57Z

WooCommerce/Classes/POS/ViewHelpers/KeyboardObserver.swift

+import SwiftUI
+import Combine
+
+final class KeyboardObserver: ObservableObject {


I don't know... but you might not need any of it. I've certainly never had to worry about it in SwiftUI before.

This article suggests that we should just be able to rely on SwiftUI to handle it for us. We can wrap it in a scroll view and it might just work...

At larger dynamic type sizes we'll need one anyway; the button gets hidden by the keyboard at the moment.

iamgabrielma · 2025-01-23T12:02:45Z

Thanks for testing Josh!

I found that it's broken in various ways with larger dynamic text sizes

While I address the keyboard observer feedback and other items, can you check if the screenshots/testing were made before or after merging trunk? Initially I thought differences you see could be from iOS17 vs 18 discrepancies, but I'm seeing the views behaving "looking fine" both using iOS17.6.1 (physical) and iOS18 (simulator) and very different from your screenshots.

Eg iOS 17.6.1 recording on physical device with 135% max font size:

RPReplay_Final1737632946.MP4

And this is with massive sizes also on iOS17.6.1 in physical device.

RPReplay_Final1737633663.MP4

…ates

joshheald · 2025-01-23T13:09:04Z

While I address the keyboard observer feedback and other items, can you check if the screenshots/testing were made before or after merging trunk? Initially I thought differences you see could be from iOS17 vs 18 discrepancies, but I'm seeing the views behaving "looking fine" both using iOS17.6.1 (physical) and iOS18 (simulator) and very different from your screenshots.

Ah, I didn't realise this was fixed on trunk. I've merged trunk to this PR, and it doesn't have nearly so many issues with dynamic type after that.

The only remaining one I see is that the CTA button for Mark payment as complete gets smaller in the largest accessibility text sizes, which isn't great.

…ps://github.com/woocommerce/woocommerce-ios into task/14910-pos-cash-testing-feedback-ui-updates

…dding

iamgabrielma · 2025-01-23T14:23:44Z

Thanks for double-checking! I'm glad wasn't something off with iOS versions 😅

Since the changes I've also seen that we don't need the keyboard observer, as you mentioned in the comments, and a simple ScrollView for the cash view is enough, just to be sure that all the vertical components will be accessible if they happen to be partially hidden due to sizing. I've updated this here: 5f9b8b9 and removed entirely the keyboard observer on bc427fb

The only remaining one I see is that the CTA button for Mark payment as complete gets smaller in the largest accessibility text sizes, which isn't great.

Right, applying conditionalPadding works fine when we want to remove padding entirely based on font size, but doesn't work that well for smooth size transitions, specially between bigger-but-non-accessible and accessible sizes. I've followed your idea of making a view modifier here 3c8757c and adapt the size of the button based on dynamicTypeSize , which is also limited to ~60% of its default size as we set its upper bound via .dynamicTypeSize(...DynamicTypeSize.accessibility1) so it cannot go smaller.

Here's for both cash and receipts views high the highest font size and error messaging:

Email receipts	Cash payments

joshheald

Looking better, thanks for the changes. Some further suggestions to improve the accessibility

I poked around with removing the limitation on the largest dynamic text size. By default, the scrollview doesn't allow you to scroll the content covered by the keyboard in to view, i.e. the button.

One way I found to resolve it was to add the KeyboardObserver back, then add the following to the scroll view.

                .safeAreaPadding(.bottom, keyboardObserver.keyboardHeight)
                .scrollDismissesKeyboard(.interactively)

safeAreaPadding is iOS 17 only, but with some careful use of availability checks you could fall back to the current solution (limited to accessibility1) for iOS 16. It might be best to do this in a view modifier... but not necessarily.

Note that I also moved our navigation header out of the scrollview.

Here's how it looks:

accessibility.scrolling.mp4

The iOS 16 support may go away if I implement Observation as I intend to. There are backport packages we could use but I'd rather not, for a small percentage of devices that we intend to drop support soon.

joshheald · 2025-01-23T15:13:07Z

WooCommerce/Classes/POS/Presentation/PointOfSaleCollectCashView.swift

+
+                FormattableAmountTextField(viewModel: textFieldViewModel, style: .pos)
+                    .focused($isTextFieldFocused)
+                    .dynamicTypeSize(...DynamicTypeSize.accessibility1)


Is limiting the size the only option here?

I'd expect people to only select the giant sizes if they really need them; limiting the range available means we're not fully supporting those users.

With the keyboard on the way, is the best I found to make it work both for iPads and iPad mini in large accessibility sizes. I think there are alternatives like we do with the header (composing it horizontally rather than vertically) and we could do the same for textfield + button and put them side by side on larger sizes rather than force them to stack vertically. This could be done with ViewThatFits for example 🤔

I think we haven't (or at least personally I have not) thought about those screens with accessibility in mind when were designed, and we could have a different version that adapts nicely with larger sizes. I've raised this with design as something we can do going forward, and I'll definitely add it to my list as well when we explore features and UI.

I like the solution you propose for iOS17+, I've spent some time today testing it and seems to work nicely. I've decided to log it separately thought and not make the changes in this PR, as we need to add several conditional modifiers to make it work with iOS16 and iOS17+ and I'm not confident of or having the time to test this properly before the code freeze in a couple of hours.

joshheald · 2025-01-23T15:15:45Z

WooCommerce/Classes/POS/Utils/AdaptiveButtonPaddingModifier.swift

+        case .xxxLarge:       return defaultButtonPadding * 0.7
+        case .xxLarge:        return defaultButtonPadding * 0.8
+        case .xLarge:         return defaultButtonPadding * 0.9


This spacing's a bit odd... you'd still have the multipliers aligned if you put each return on a new line

True that, updated on 0cddff1

iamgabrielma · 2025-01-24T06:57:29Z

Thanks for the review and the comments @joshheald . The .safeAreaPadding recommendation looks like a great way to go, and really appreciate it, but as I mention on #14919 (comment) I'll hold it for now just because of testing time before the freeze. I'll circle back to it as soon as possible since we may not even need to add the conditional handling depending on how the conversation about supporting iOS16 goes.

I'll go ahead and merge this one for now 👍

iamgabrielma added 3 commits January 20, 2025 14:22

update change due text color

2e41a17

button dynamic spacing when button shown

61d90b9

Extract KeyboardObserver

a4217eb

iamgabrielma added type: task An internally driven task. feature: POS labels Jan 20, 2025

iamgabrielma added this to the 21.5 milestone Jan 20, 2025

iamgabrielma added 6 commits January 20, 2025 17:37

add preview

ef0296f

Add keyboard observer to receipts view

a1ad3a0

Move circle bg color to POS constants

d0c8f85

success text color happened to be different, but we no longer use it. Let’s move the circle success color to POS constants so we do not have different tones all around. We still keep the same values for light-dark modes

update dark mode color for circle success

b251a6d

Merge branch 'trunk' into task/14910-pos-cash-testing-feedback-ui-upd…

a7e634c

…ates

clean up keyboard observer

8f90fae

iamgabrielma commented Jan 21, 2025

View reviewed changes

iamgabrielma requested a review from joshheald January 21, 2025 12:26

iamgabrielma marked this pull request as ready for review January 21, 2025 12:26

iamgabrielma mentioned this pull request Jan 22, 2025

[Woo POS] [Cash & Receipts] Enable acceptCashForPointOfSale #14884

Merged

4 tasks

joshheald self-assigned this Jan 23, 2025

Merge branch 'trunk' into task/14910-pos-cash-testing-feedback-ui-upd…

3ead569

…ates

joshheald reviewed Jan 23, 2025

View reviewed changes

Merge branch 'trunk' into task/14910-pos-cash-testing-feedback-ui-upd…

203bdf7

…ates

iamgabrielma added 5 commits January 23, 2025 14:26

Wrap cash view in ScrollView. Remove keyboard observer

5f9b8b9

Merge branch 'task/14910-pos-cash-testing-feedback-ui-updates' of htt…

0be5aa6

…ps://github.com/woocommerce/woocommerce-ios into task/14910-pos-cash-testing-feedback-ui-updates

remove keyboard observer from receipts view and adjust conditional pa…

bc427fb

…dding

Use AdaptiveButtonPaddingModifier for button padding

3c8757c

linting

f7cb29f

iamgabrielma requested a review from joshheald January 23, 2025 14:24

joshheald approved these changes Jan 23, 2025

View reviewed changes

styling

0cddff1

iamgabrielma mentioned this pull request Jan 24, 2025

[Woo POS] Accessibility improvements iOS17+ #14970

Open

iamgabrielma merged commit e6f6894 into trunk Jan 24, 2025
12 checks passed

iamgabrielma deleted the task/14910-pos-cash-testing-feedback-ui-updates branch January 24, 2025 06:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Woo POS] [Cash & Receipts] UI updates from testing #14919

[Woo POS] [Cash & Receipts] UI updates from testing #14919

iamgabrielma commented Jan 20, 2025 •

edited

Loading

dangermattic commented Jan 20, 2025 •

edited

Loading

wpmobilebot commented Jan 20, 2025 •

edited

Loading

iamgabrielma Jan 21, 2025

joshheald Jan 23, 2025

joshheald left a comment

joshheald Jan 23, 2025

joshheald Jan 23, 2025

joshheald Jan 23, 2025

joshheald Jan 23, 2025

joshheald Jan 23, 2025

iamgabrielma commented Jan 23, 2025 •

edited

Loading

joshheald commented Jan 23, 2025

iamgabrielma commented Jan 23, 2025

joshheald left a comment

joshheald Jan 23, 2025

iamgabrielma Jan 24, 2025

joshheald Jan 23, 2025

iamgabrielma Jan 24, 2025

iamgabrielma commented Jan 24, 2025

-            Spacer().frame(height: keyboardHeight)
+            Spacer()
+                .keyboardResponsiveHeight(
+                    withLargeKeyboard: Constants.keyboardShownButtonSpacing,
+                    withoutKeyboard: Constants.keyboardHiddenButtonSpacing)

[Woo POS] [Cash & Receipts] UI updates from testing #14919

[Woo POS] [Cash & Receipts] UI updates from testing #14919

Conversation

iamgabrielma commented Jan 20, 2025 • edited Loading

Description

Changes

Testing information

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

dangermattic commented Jan 20, 2025 • edited Loading

wpmobilebot commented Jan 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshheald left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iamgabrielma commented Jan 23, 2025 • edited Loading

joshheald commented Jan 23, 2025

iamgabrielma commented Jan 23, 2025

joshheald left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iamgabrielma commented Jan 24, 2025

iamgabrielma commented Jan 20, 2025 •

edited

Loading

dangermattic commented Jan 20, 2025 •

edited

Loading

wpmobilebot commented Jan 20, 2025 •

edited

Loading

iamgabrielma commented Jan 23, 2025 •

edited

Loading