Skip to content

Conversation

@staskus
Copy link
Contributor

@staskus staskus commented Jun 13, 2025

WOOPRD-565

Description

Show barcode scanning information, used Android as an example for copy and structure:

image

Solution

  • Created a modal for barcode scanning with a new modal style
  • For reusability, allowed to inject content into a new modal and created a reusable ParagraphView
  • Reused the style between barcode scanning and

Steps to reproduce

  1. Open POS
  2. Open popover menu
  3. Select "Barcode scanning"
  4. Confirm it appears according to requirements and "More details" link works
  5. Select "Where are my products?"
  6. Confirm it uses a consistent style

Testing information

Tested on iPad Air 13 Inch M3

Screenshots

Barcode

Simulator Screenshot - iPad Air 13-inch (M3) - 2025-06-13 at 22 42 41

Product Limitations

Simulator Screenshot - iPad Air 13-inch (M3) - 2025-06-13 at 23 41 06

Dynamic Type

dynamic.type.mp4

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@staskus staskus added this to the 22.7 milestone Jun 13, 2025
@staskus staskus requested a review from Copilot June 13, 2025 20:48
@staskus staskus added type: task An internally driven task. feature: POS labels Jun 13, 2025
@dangermattic
Copy link
Collaborator

dangermattic commented Jun 13, 2025

1 Warning
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a reusable information modal component for POS, adds a dedicated barcode-scanning info modal, and updates the floating control menu to expose this new modal.

  • Introduce PointOfSaleInformationModal and PointOfSaleInformationModalParagraphView for consistent info‐modal styling.
  • Implement PointOfSaleBarcodeScannerInformationModal with localized messages and link support.
  • Refactor SimpleProductsOnlyInformation to use the new modal and add a “Barcode scanning” button in POSFloatingControlView.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
WooCommerce/WooCommerce.xcodeproj/project.pbxproj Added new modal source file references
WooCommerce/Classes/POS/Presentation/SimpleProductsOnlyInformation.swift Refactored to use PointOfSaleInformationModal
WooCommerce/Classes/POS/Presentation/PointOfSaleInformationModal.swift New reusable POS information modal and paragraph view
WooCommerce/Classes/POS/Presentation/PointOfSaleBarcodeScannerInformationModal.swift New barcode scanning info modal
WooCommerce/Classes/POS/Presentation/POSFloatingControlView.swift Added “Barcode scanning” button and modal integration
Comments suppressed due to low confidence (4)

WooCommerce/Classes/POS/Presentation/PointOfSaleInformationModal.swift:48

  • Using a fixed width of 896 points may cause overflow on smaller devices. Consider using .frame(maxWidth: Constants.modalFrameWidth) or constraining to the screen width.
.frame(width: Constants.modalFrameWidth)

WooCommerce/Classes/POS/Presentation/PointOfSaleInformationModal.swift:1

  • No UI or snapshot tests were added for this new reusable modal. Consider adding tests to verify layout, spacing, and behavior when isPresented toggles.
import SwiftUI

WooCommerce/Classes/POS/Presentation/PointOfSaleBarcodeScannerInformationModal.swift:1

  • The Foundation import isn’t used in this file. You can remove it to keep the imports minimal.
import Foundation

WooCommerce/Classes/POS/Presentation/PointOfSaleInformationModal.swift:70

  • Ensure a custom .if view‐modifier extension is available in scope, or replace this with a standard conditional modifier pattern to avoid compile errors.
.if(style == .default, transform: { view in

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jun 13, 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 Number30560
VersionPR #15750
Bundle IDcom.automattic.alpha.woocommerce
Commitf99c050
Installation URL20jq23hi7ch8o
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@staskus staskus marked this pull request as ready for review June 16, 2025 11:48
@staskus staskus requested a review from joshheald June 16, 2025 11:48
Copy link
Contributor

@joshheald joshheald left a comment

Choose a reason for hiding this comment

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

Some suggestions inline, but generally looks good, thanks for doing this 👍

@Binding private var showSupport: Bool
@Binding private var showDocumentation: Bool
@State private var showProductRestrictionsModal: Bool = false
@State private var showBarcodeScanning: Bool = false
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@State private var showBarcodeScanning: Bool = false
@State private var showBarcodeScanningInformation: Bool = false

Nit: with plans to add camera-based scanning, which will also have a view to show, this might be a bit clearer in the future.


private var bulletPointWithLink: AttributedString {
var secondary = AttributedString(Localization.barcodeInfoSecondaryMessage + " ")
var moreDetails = AttributedString(Localization.barcodeInfoMoreDetailsLink)
Copy link
Contributor

Choose a reason for hiding this comment

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

This link relates primarily to setting up barcodes. I mentioned to Andrey that it should be on the first item in the list, not the second, and I believe he changed it there, the screenshot from Android is probably out of date.

Could you move this to the first bullet point please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, moved.

)
static let barcodeInfoTertiaryMessage = NSLocalizedString(
"pos.barcodeInfoModal.tertiaryMessage",
value: "• Connect your barcode scanner in System Bluetooth settings.",
Copy link
Contributor

Choose a reason for hiding this comment

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

"System" here was to avoid it being platform specific in the wireframes. I think we should say "iOS Bluetooth settings" or just "Bluetooth settings". cc @kidinov

static let barcodeInfoQuinaryMessage = NSLocalizedString(
"pos.barcodeInfoModal.quinaryMessage",
value: "The scanner emulates a keyboard, so sometimes it will prevent the software keyboard from showing, e.g. in search. " +
"Tap on the keyboard icon to show the software keyboard back.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"Tap on the keyboard icon to show the software keyboard back.",
"Tap on the keyboard icon to show it again.",

Just a slight tweak here to make it read a little better – cc @kidinov

let content: Content

// Used to make ScrollView height increase together with the content height.
@State private var contentHeight: CGFloat = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that this (or something else about this struct) makes the modals grow as they transition for presentation/dismissal as well. That didn't happen in the past, they just faded in. It would be good if they could already have the correct size when we try to present them...

I'm not sure the best way to do it though...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's only an iOS 17 animation issue, not for the first time.

Yes, it's likely related to contentHeight, I haven't found a quick way to properly size content within the ScrollView.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found a quick workaround, applying a 0-second ease-in animation to contentHeigh transition does the job:

Simulator.Screen.Recording.-.iPad.Air.11-inch.17.5.-.2025-06-18.at.17.45.30.mp4

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice one, and thanks for commenting about it too. I checked on device and it looks good.

Comment on lines +15 to +17
PointOfSaleInformationModal(isPresented: $isPresented, title: AttributedString(Localization.modalTitle)) {
PointOfSaleInformationModalParagraphView {
Text(issueMessage)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice one for sharing this code 👍 Looks better all round

@staskus staskus enabled auto-merge June 18, 2025 14:56
@staskus staskus merged commit 12c61cd into trunk Jun 18, 2025
13 checks passed
@staskus staskus deleted the wooprd-565-woo-posbarcodes-show-barcode-scanning-information-from-the branch June 18, 2025 15:18
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.

5 participants