woocommerce-ios icon indicating copy to clipboard operation
woocommerce-ios copied to clipboard

[Woo POS] Use POS styles in reader connection modals

Open joshheald opened this issue 1 year ago • 2 comments

Part of: #13731 Merge after: #12771

Description

This is part of a series of PRs to improve the card reader connnection screens. You may want to test at the end of the series, as it could get quite repetitive, but doing it all in one was quite big. Up to you! The last one is #13775

This PR updates the POS card reader connection modals to use POS buttons and on the Multiple readers view, font styles. Other views previously had the correct font styles applied.

Steps to reproduce

  1. Launch the app
  2. Enter the POS
  3. Tap Connect your reader
  4. Wait for the Do you want to connect screen – if it just auto-connects, tap Reader connected then Disconnect reader and try again
  5. Observe that the buttons use the new style
  6. Tap Keep searching
  7. Turn on another reader
  8. Observe that the multiple readers screen shows and uses POS fonts (except the connect buttons – handled later)
  9. Tap connect on a reader, but turn it off at the same time
  10. The error screen will show shortly after that – observe that it uses the new buttons.

Address issues:

First, delete the first line of your store address in WP-admin > Woocommerce > Settings

  1. Launch the app
  2. Enter the POS
  3. Tap Connect your reader
  4. The error screen will show shortly after that – observe that it uses the new buttons.

Testing information

I have tested these on an iPad running iOS 16. No unit tests as it's UI only.

Screenshots

Note that this video is from this branch – if you test later on, the rest of the modal UI will look a bit different.

https://github.com/user-attachments/assets/b384cced-0888-49de-8535-372b64fe5427


  • [x] 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.

joshheald avatar Aug 28 '24 14:08 joshheald

1 Warning
:warning: View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.
1 Message
:book: This PR is still a Draft: some checks will be skipped.

Generated by :no_entry_sign: Danger

dangermattic avatar Aug 28 '24 14:08 dangermattic

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

App NameWooCommerce iOS WooCommerce iOS
Build Numberpr13772-d8098b8
Version20.1
Bundle IDcom.automattic.alpha.woocommerce
Commitd8098b802f6d3597e8904e49eee5200a3120a3d7
App Center BuildWooCommerce - Prototype Builds #10623
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

wpmobilebot avatar Aug 28 '24 14:08 wpmobilebot

I think is not from this PR, but I found the alert for the foundReader case seems to cut the name of the reader off, it could be from updating the font sizes without multiline/wrapping:

The rest tests well! 🎉

Thanks! That's fixed in #13773 because the string gets made much shorter.

joshheald avatar Aug 29 '24 08:08 joshheald