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

[Brand Updates] POS theme updates #14756

Open
wants to merge 3 commits into
base: feature/woo-2.0-brand-updates
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ struct POSProgressViewStyle: ProgressViewStyle {
lineWidth: lineWidth,
lineCap: .butt,
circleColor: Color(.wooCommercePurple(.shade10)),
fillColor: Color(.wooCommercePurple(.shade50))
fillColor: Color.accent
Copy link
Member Author

Choose a reason for hiding this comment

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

I updated this to match the updated app accent, which is purple_40 in light theme and purple_30 in dark theme.
This matches the color we use in the last Figma playground files (rNAclWFAdQRw7EXEcsNVai-fi-2533_5542) where we are are using purple_40 on light theme.

))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ struct POSSecondaryButtonStyle: ButtonStyle {
.font(.posBodyEmphasized)
.background(
RoundedRectangle(cornerRadius: POSButtonStyleConstants.framedButtonCornerRadius)
.stroke(Color.posPrimaryButtonBackground,
.stroke(Color.posSecondaryButtonForeground,
lineWidth: POSButtonStyleConstants.secondaryButtonBorderStrokeWidth)
.background(Color.posPrimaryBackground))
.foregroundColor(.posPrimaryButtonBackground)
.foregroundColor(.posSecondaryButtonForeground)
}
}
29 changes: 13 additions & 16 deletions WooCommerce/Classes/POS/Utils/Color+WooCommercePOS.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@ import SwiftUI

extension Color {

static var accent: Color {
return Color(
UIColor(
light: .withColorStudio(.wooCommercePurple, shade: .shade40),
dark: .withColorStudio(.wooCommercePurple, shade: .shade30)
)
)
}

// MARK: - Background

/* POS Background colors are defined in a similar philosophy as system background colors:
Expand Down Expand Up @@ -89,23 +98,11 @@ extension Color {

// MARK: - Buttons

static var posPrimaryButtonBackground: Color {
return Color(
UIColor(
light: .withColorStudio(.wooCommercePurple, shade: .shade50),
dark: .withColorStudio(.wooCommercePurple, shade: .shade30)
)
)
}
static var posPrimaryButtonBackground: Color = .accent
Copy link
Member Author

@hichamboushaba hichamboushaba Dec 23, 2024

Choose a reason for hiding this comment

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

For the regular app theme, the primary button uses purple_40 in all themes, this is to give a better contrast with the white text (more context here p1734705352565459/1727138732.960149-slack-C07E7D0R761), but for POS, I noticed the button text is black, so for now, it will use purple_30 in dark theme.

Was the usage of black text here a design decision or just a dev choice? Personally, I think white looks better and gives a better contrast, but that's just a thought.

Black White
Simulator Screenshot - iPad Air 13-inch (M2) - 2024-12-23 at 13 19 11 Simulator Screenshot - iPad Air 13-inch (M2) - 2024-12-23 at 12 56 17


static var posTextButtonForeground: Color {
return Color(
UIColor(
light: .withColorStudio(.wooCommercePurple, shade: .shade50),
dark: .withColorStudio(.wooCommercePurple, shade: .shade30)
)
)
}
static var posSecondaryButtonForeground: Color = .accent
Copy link
Member Author

Choose a reason for hiding this comment

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

The POSSecondaryButtonStyle was referencing posPrimaryButtonBackground. When I first changed posPrimaryButtonBackground to use a single color for both themes, I added this property. However, I later modified the primary button background again (see the comment above), so this property isn't strictly necessary now. Still, in my opinion, retaining it could enhance clarity and help decouple the components. Let me know your thoughts.


static var posTextButtonForeground: Color = .accent

static var posTextButtonForegroundPressed: Color {
return Color(
Expand Down
Loading