-
Notifications
You must be signed in to change notification settings - Fork 6
refactor: Implement design tokens and pixel-perfect input UI #1393
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
base: bn/feature/checkout-components
Are you sure you want to change the base?
refactor: Implement design tokens and pixel-perfect input UI #1393
Conversation
a1598dc to
16f1972
Compare
Generated by 🚫 Danger Swift against 935ec59 |
|
Appetize link: https://appetize.io/app/xr7cjgvybkyhiuxfg7f773qeou |
3797385 to
990551b
Compare
990551b to
f08ddd3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing work! Few minor things to look at...
|
|
||
| #if DEBUG | ||
| @available(iOS 15.0, *) | ||
| struct CardNetworkBadge_Previews: PreviewProvider { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing job adding all these previews!
| .aspectRatio(contentMode: .fit) | ||
| .frame(width: iconSize, height: iconSize) | ||
| .foregroundColor(PrimerCheckoutColors.iconNegative(tokens: tokens)) | ||
| .offset(x: hasError ? 0 : -10) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Magic numbers
| .offset(y: hasError ? 0 : -10) | ||
| .opacity(hasError ? 1.0 : 0.0) | ||
| .padding(.top, hasError ? PrimerSpacing.xsmall(tokens: tokens) : 0) | ||
| .animation(.spring(response: 0.3, dampingFraction: 0.7), value: hasError) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Magic numbers... can we somehow centralize this animation for future reuse?
| forResource: fontNameWithoutExt, | ||
| withExtension: "ttf" | ||
| ) else { | ||
| return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Font loading failures are silently ignored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other places as well
|
|
||
| /// Generic container for Primer input fields that provides consistent styling, layout, and error handling | ||
| @available(iOS 15.0, *) | ||
| struct PrimerInputFieldContainer<Content: View, RightContent: View>: View { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Observation: Good use of generics, but the RightContent type is optional. Consider using a different pattern:
// Alternative approach
struct PrimerInputFieldContainer<Content: View>: View {
let rightComponent: (() -> AnyView)?
}
This would simplify the type signature when RightContent isn't needed.
| // | ||
| // CardNetworkBadge.swift | ||
| // PrimerSDK - CheckoutComponents | ||
| // | ||
| // Created by Boris on 16.7.25. | ||
| // | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@henry-cooper-primer is there a way to standardize these headers for all team members?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's just AI, marking everything as 'Created by Boris' 😂 I will remove those in the next commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am referring to this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep should be a swift format rule - added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file was created long before the Swift Format. It's a good point, Semir. Can we add the ticket to unify these in CheckoutComponents @OnurVar?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve created the task and will handle these headers separately.
e4587de to
d4c9da6
Compare
Replace hardcoded values with token-based design system and consolidate input components for pixel-perfect Figma implementation. Design System: - Add DesignTokens with light/dark mode JSON configurations - Implement token generator and processor for type-safe access - Add InterVariable font with optimized loading - Support dynamic appearance mode switching Component Refactoring: - Create PrimerInputFieldContainer for consistent input styling - Extract CardNetworkBadge as reusable component - Add UITextField styling extensions for shared behavior - Consolidate all input fields to use container pattern - Apply pixel-perfect height constraints matching Figma Technical Details: - Token system generates Swift code from JSON definitions - Container provides unified error handling and layout - Font registration uses DEBUG conditionals for preview support - All input components share consistent padding and spacing - Design tokens support theme overrides and customization This establishes the foundation for scalable, design-driven UI development with consistent styling across all components.
Font loading failures were previously silently ignored. Added error logging for all failure points and fixed memory leak in error handling.
Split container into focused files for better maintainability: - Core struct and convenience initializer in main file - View rendering logic in +Rendering extension - Styling properties in +Styling extension - Preview helpers in +PreviewHelpers extension Updated preview format to use #Preview macro and changed access control to internal to support multi-file organization.
d4c9da6 to
a0b503d
Compare
Simplifies color system naming by removing redundant Primer prefix. Updates all references across CheckoutComponents module.
Replace custom headers with standard Primer SDK copyright format in 5 internal CheckoutComponents files to pass CI linting checks. Files updated: - PrimerLayout.swift - MockCardFormScope.swift - MockDIContainer.swift - MockDesignTokens.swift - MockValidationService.swift
|
|
||
| // MARK: - Body | ||
|
|
||
| @ViewBuilder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is view builder by default
|
|
||
| #if DEBUG | ||
| @available(iOS 15.0, *) | ||
| struct CardNetworkBadge_Previews: PreviewProvider { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use modern preview macros?
| #if DEBUG | ||
| @available(iOS 15.0, *) | ||
| struct PreviewContainer: View { | ||
| let label: String? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these can be private
| self.label = label | ||
| self.text = text | ||
| self.errorMessage = errorMessage | ||
| self._currentText = State(initialValue: text) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to use wrappedValue (they do the same thing but initialValue is technically legacy. Same elsewhere
| isSecureTextEntry: false | ||
| ) | ||
|
|
||
| /// Configuration for email input |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please can we avoid these types of comments?
| autocorrectionType: .no, | ||
| textContentType: .emailAddress, | ||
| returnKeyType: .done, | ||
| isSecureTextEntry: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since there are default values in the initialiser, can we make use of them here and elsewhere so we can slim down these properties
| @available(iOS 15.0, *) | ||
| extension UITextField { | ||
| /// Configures the text field with Primer design tokens and standard settings | ||
| func configurePrimerStyle( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably slightly too long, I might consider breaking this up into two or three methods
| let doneButton = UIButton(type: .system) | ||
| doneButton.setTitle("Done", for: .normal) | ||
| doneButton.titleLabel?.font = UIFont.systemFont(ofSize: 17, weight: .medium) | ||
| if let target = doneButtonTarget { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can use shorthand syntax here
|
@henry-cooper-primer All review comments have been addressed. |
- Use modern #Preview macros instead of PreviewProvider - Make preview helper properties private - Use wrappedValue instead of initialValue for State - Remove redundant comments - Simplify configuration properties with default values - Refactor configurePrimerStyle into focused helper methods - Use Swift shorthand syntax for optional binding (if let x)
f739563 to
935ec59
Compare
|


Description
ACC-5719
Summary
Refactor CheckoutComponents to use a design token system and consolidate input field components for
pixel-perfect Figma implementation.
Changes
Design Token System
Component Refactoring
Font System
Benefits
Technical Implementation
DesignTokens/with JSON source filesInternal/Tokens/Presentation/Components/Containers/Presentation/Components/Utilities/Testing