Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Woo POS] Hide cart images when the view is too narrow #15028

Merged
merged 9 commits into from
Feb 5, 2025
45 changes: 45 additions & 0 deletions WooCommerce/Classes/POS/Presentation/CartView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ struct CartView: View {
posModel.cart.isNotEmpty && offSetPosition < 0
}

@State private var shouldShowItemImages: Bool = false

var body: some View {
VStack {
DynamicHStack(spacing: Constants.cartHeaderSpacing) {
Expand Down Expand Up @@ -70,6 +72,7 @@ struct CartView: View {
VStack(spacing: Constants.cartItemSpacing) {
ForEach(posModel.cart, id: \.id) { cartItem in
ItemRowView(cartItem: cartItem,
showImage: $shouldShowItemImages,
onItemRemoveTapped: posModel.orderStage == .building ? {
posModel.remove(cartItem: cartItem)
} : nil)
Expand All @@ -81,6 +84,15 @@ struct CartView: View {
.background(GeometryReader { geometry in
Color.clear.preference(key: ScrollOffSetPreferenceKey.self,
value: geometry.frame(in: coordinateSpace).origin.y)
.onAppear {
updateItemImageVisibility(cartListWidth: geometry.size.width)
}
.onChange(of: geometry.size.width) {
updateItemImageVisibility(cartListWidth: $0)
}
.onChange(of: dynamicTypeSize) {
updateItemImageVisibility(dynamicTypeSize: $0, cartListWidth: geometry.size.width)
}
})
.onPreferenceChange(ScrollOffSetPreferenceKey.self) { position in
self.offSetPosition = position
Expand Down Expand Up @@ -156,6 +168,39 @@ private extension CartView {
cart: posModel.cart,
orderStage: posModel.orderStage)
}

func updateItemImageVisibility(dynamicTypeSize: DynamicTypeSize? = nil, cartListWidth: CGFloat) {
let newVisibility = cartListWidth >= minimumWidthToShowItemImages(with: dynamicTypeSize ?? self.dynamicTypeSize)
guard newVisibility != shouldShowItemImages else {
return
}
shouldShowItemImages = newVisibility
}

func minimumWidthToShowItemImages(with dynamicTypeSize: DynamicTypeSize) -> CGFloat {
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that I chose to do the calculations on the cart, not each image. It just felt bad for performance to have lots of views using geometry readers to calculate the exact same answer... but it would be nicer from a code-readability perspective to have it on the item cards.

Theoretically, we could pass values of the geometry reader to the item view, and then it could decide whether it's shown or not. Of course, this way, responsibilities would be split between two views.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, if we ever want it to be different between different types of cart item then we could do that, but it doesn't seem worth it just now

switch dynamicTypeSize {
case .xSmall, .small:
240
case .medium:
260
case .large:
270
case .xLarge:
280
case .xxLarge, .xxxLarge:
300
case .accessibility1:
320
case .accessibility2:
400
case .accessibility3, .accessibility4:
420
case .accessibility5:
450
@unknown default:
450
}
}
}

private extension CartView {
Expand Down
11 changes: 7 additions & 4 deletions WooCommerce/Classes/POS/Presentation/ItemRowView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ struct ItemRowView: View {
private let onItemRemoveTapped: (() -> Void)?

@ScaledMetric private var scale: CGFloat = 1.0
@Environment(\.dynamicTypeSize) var dynamicTypeSize
@Environment(\.colorScheme) var colorScheme
@Binding private var showProductImage: Bool

init(cartItem: CartItem, onItemRemoveTapped: (() -> Void)? = nil) {
init(cartItem: CartItem, showImage: Binding<Bool> = .constant(true), onItemRemoveTapped: (() -> Void)? = nil) {
self.cartItem = cartItem
self._showProductImage = showImage
self.onItemRemoveTapped = onItemRemoveTapped
}

Expand All @@ -31,6 +32,7 @@ struct ItemRowView: View {
.font(Constants.itemPriceFont)
}
.frame(maxWidth: .infinity, alignment: .leading)
.padding(.leading, showProductImage ? 0 : Constants.cardContentHorizontalPadding)
.accessibilityElement(children: .combine)

if let onItemRemoveTapped {
Expand All @@ -41,7 +43,7 @@ struct ItemRowView: View {
.font(.posBodyRegular)
})
.accessibilityLabel(Localization.removeFromCartAccessibilityLabel)
.padding()
.padding(.trailing, Constants.cardContentHorizontalPadding)
.foregroundColor(Color.posTertiaryText)
}
}
Expand All @@ -57,7 +59,7 @@ struct ItemRowView: View {

@ViewBuilder
private var productImage: some View {
if dynamicTypeSize >= .accessibility3 {
if !showProductImage {
EmptyView()
} else if let imageSource = cartItem.item.productImageSource {
ProductImageThumbnail(productImageURL: URL(string: imageSource),
Expand Down Expand Up @@ -105,6 +107,7 @@ private extension ItemRowView {
static let cardOutlineWidth: CGFloat = 1
static let horizontalPadding: CGFloat = 16
static let horizontalElementSpacing: CGFloat = 16
static let cardContentHorizontalPadding: CGFloat = 16
static let itemTitleAndPriceSpacing: CGFloat = 4
static let itemTitleFont: POSFontStyle = .posDetailEmphasized
static let itemSubtitleFont: POSFontStyle = .posDetailLight
Expand Down
2 changes: 1 addition & 1 deletion WooCommerce/Classes/POS/Utils/Color+WooCommercePOS.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ extension Color {
Color(
UIColor(
light: .white,
dark: .tertiarySystemBackground
dark: UIColor(red: 44.0 / 255.0, green: 44.0 / 255.0, blue: 46.0 / 255.0, alpha: 1.0)
)
)
}
Expand Down