Skip to content

Conversation

@iamgabrielma
Copy link
Contributor

Closes WOOMOB-1227

Description

This PR updates the barcode scanner documentation link to the correct one, rather than to POS general documentation.

Steps to reproduce

  • Navigate to POS > Settings > Hardware > Barcode Scanners > Tap on Documentation
  • Observe the link opened is https://woocommerce.com/document/barcode-and-qr-code-scanner/
Settings Webview
Simulator Screenshot - iPad mini (A17 Pro) - US store - 2025-09-02 at 05 32 41 Simulator Screenshot - iPad mini (A17 Pro) - US store - 2025-09-02 at 05 32 47

@iamgabrielma iamgabrielma added type: task An internally driven task. feature: POS labels Sep 1, 2025
@iamgabrielma iamgabrielma added this to the 23.2 milestone Sep 1, 2025
@iamgabrielma iamgabrielma marked this pull request as ready for review September 1, 2025 22:39
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Sep 1, 2025

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

App NameWooCommerce iOS Prototype
Build Numberpr16069-5a94138
Version23.1
Bundle IDcom.automattic.alpha.woocommerce
Commit5a94138
Installation URL6m2goa5ligj4g
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@iamgabrielma iamgabrielma requested a review from jaclync September 1, 2025 23:12
Copy link
Contributor

@jaclync jaclync left a comment

Choose a reason for hiding this comment

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

Works nicely :shipit:

Nit in dark mode, the selected light purple color feels a bit close to the white text color. Maybe we could use a purple color that has more contrast from the text color in dark mode?

Simulator Screenshot - iPad (A16) - 2025-09-02 at 09 37 48


/// URL for Point of Sale's barcode scanner documentation
///
case pointOfSaleBarcodeScannerDocumentation = "https://woocommerce.com/document/barcode-and-qr-code-scanner/"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: from Povilas's effort to modularize POS, I wonder if we could separate the POS constants so that the POS code isn't dependent on the constants enums that are mostly used in the store management part of the app.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing that out, I've made a POSConstants on 2990f6b which copies the current usage of WooConstants, and moved the 2 documentation links we use for settings along with 4bb6fc5

Most likely we'll want to copy over as well the links regarding IPP, but I'll leave it out for the moment.

@iamgabrielma
Copy link
Contributor Author

Thanks for the review!

Nit in dark mode, the selected light purple color feels a bit close to the white text color. Maybe we could use a purple color that has more contrast from the text color in dark mode?

Yeah, this is something we brought up to design, as the dark version of the selection only works well when text is not primary. Today we should be clear on which one to use for i1. We also asked for dark versions of all views for next iteration to avoid these issues in the future 👍

@iamgabrielma
Copy link
Contributor Author

Nit in dark mode, the selected light purple color feels a bit close to the white text color. Maybe we could use a purple color that has more contrast from the text color in dark mode?

Updated the selection color on a different PR where it made more sense in context: #16070 👍

@iamgabrielma iamgabrielma merged commit 5313d38 into trunk Sep 2, 2025
14 checks passed
@iamgabrielma iamgabrielma deleted the task/WOOMOB-1227-pos-settings-update-doc-link branch September 2, 2025 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: POS type: task An internally driven task.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants