[POS Orders] Display POS label in order details #15857
Conversation
|
|
| var formattedSalesChannel: String? { | ||
| guard let salesChannel = salesChannel else { | ||
| return nil | ||
| } | ||
| switch salesChannel { | ||
| case "pos-rest-api": | ||
| return "POS" | ||
| default: | ||
| return nil | ||
| } | ||
| } |
There was a problem hiding this comment.
This formattedSalesChannel is a code duplication with the order list badge that we can resolve moving the logic to Yosemite, I'll investigate this separately tomorrow as there's some build errors when trying to extend the Order, otherwise we can use some sort of simple object to share logic from Yosemite without needing to extend anything. Ref: p1751433227323889-slack-CGPNUU63E
iamgabrielma
added
type: task
There was a problem hiding this comment.
LGTM! 🚀 The label feels quite hidden, but I understand we'll update when the design is ready.
- xib looks good without warnings
- tested POS orders with largest font size in portrait where POS label is shown
- tested non-POS orders
- tested with feature flag disabled
|
|
||
| // MARK: - VoiceOver | ||
| /// |
There was a problem hiding this comment.
nit: any reasons for removing the comments here?
There was a problem hiding this comment.
Not really, mostly cleaning up since I found them to be not very useful in this case (ie marking as voiceover the only voiceover function, or marking as private a private extension). Other places are way more blatant with this, for example in places like UIImage+Woo the file is double the size just because we do things like:
/// Search Image
///
static var searchImage: UIImage {
UIImage(named: "search")!
}
The variable or function name already tells us what we need to know. I'm fine with bringing them back if you'd prefer 👍
| func configure(_ viewModel: SummaryTableViewCellViewModel) { | ||
| titleLabel.text = viewModel.billedPersonName | ||
| subtitleLabel.text = viewModel.subtitle | ||
| salesChannelLabel.text = viewModel.formattedSalesChannel |
There was a problem hiding this comment.
nit: might we want to hide the label if the text is nil? since this is in a stack view, if the stack view has a non-zero spacing (from the xib it looks like it has 4px spacing), the spacing still takes up space even if the label has zero width from nil text when it's not hidden.
There was a problem hiding this comment.
| salesChannelLabel.text = viewModel.formattedSalesChannel | |
| salesChannelLabel.text = viewModel.formattedSalesChannel | |
| salesChannelLabel.isHidden = salesChannelLabel.text == nil |
This fixes the order date not taking up the horizontal space when the sales channel label isn't shown, which is more obvious in large font sizes:
There was a problem hiding this comment.
if the stack view has a non-zero spacing (from the xib it looks like it has 4px spacing), the spacing still takes up space even if the label has zero width from nil text when it's not hidden.
TIL. Thanks for sharing the solution. Updated on 70df74e
|
Thanks for the review @jaclync !
Yeah, designs might still change so I'm not optimizing much for it. For the moment I've logged WOOMOB-751 to be sure we tackle it and displays correctly in all sizes. |
iamgabrielma
deleted the
task/WOOMOB-707-display-pos-badge-in-order-details
branch
Closes WOOMOB-707 , only 1 review is needed!
Description
This PR adds the
POSlabel to the order details view, for those orders which theircreated_viaproperty ispos-rest-api. As with the badge, there are no final designs yet, so we just render the text as the kick-off proposal in pdfdoF-7rY-p2Changes
buildConfig == .localDeveloper || buildConfig == .alpha, since POS labels now show only when appropriate.SummaryTableViewCellViewModelto its own filesalesChannelLabellabel property to the .xib file for the cell, and wrapped it into an horizontal stack along with the existingsubtitleLabelTesting information
created_viaproperty is not supported), observe how thePOSlabel appears in the top right corner of order details view, along the order date.pointOfSaleOrdersi1flag tofalse, re-run the app, we should see no POS labels in order details, nor in order list.Screenshots
RELEASE-NOTES.txtif necessary.