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.

Sign up for GitHub

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

Jump to bottom

Consolidate presentation logic to ensure style configuration is applied #4518

Merged
merged 7 commits into from
Feb 3, 2025
Merged

Consolidate presentation logic to ensure style configuration is applied #4518

merged 7 commits into from
Feb 3, 2025
+86 −4

Conversation

mats-stripe
Copy link
Collaborator

Summary

Turns out that the initial style configuration PR missed applying the configuration to some view controllers we show - it was an approach prone to developer error. This consolidates our view controller presentation logic into a PresentationManager which applies the specified style configuration to each controller we shown.

Motivation

Reduce possibility of developer errors cause our theming system to not be correctly applied to each view.

Testing

Unit tests added, and manually tested.

Changelog

N/a

@mats-stripe mats-stripe requested review from a team as code owners January 30, 2025 21:04
@mats-stripe mats-stripe mentioned this pull request Jan 30, 2025
Closed
Base automatically changed from mats/fc_configuration_api to master January 31, 2025 16:37
@emerge-tools Emerge Tools
Copy link

emerge-tools bot commented Jan 31, 2025

⚠️ 1 new unused protocol, 2 builds increased size, 5 builds decreased size, 1 build errored

Name Version Download Change Install Change Approval
StripeSize
com.stripe.StripeSize		 1.0 (1) 2.1 MB ⬇️ 2.3 kB (-0.11%) 6.9 MB ⬇️ 1.4 kB (-0.02%) N/A
StripePaymentsSize
com.stripe.StripePaymentsSize		 1.0 (1) 1.2 MB ⬇️ 2.8 kB (-0.23%) 4.2 MB ⬇️ 1.4 kB (-0.03%) N/A
StripePaymentsUISize
com.stripe.StripePaymentsUISize		 1.0 (1) 1.9 MB ⬇️ 2.8 kB (-0.15%) 6.4 MB ⬇️ 1.4 kB (-0.02%) N/A
StripePaymentSheetSize
com.stripe.StripePaymentSheetSize		 1.0 (1) 3.7 MB ⬇️ 8.4 kB (-0.22%) 11.1 MB ⬇️ 21.6 kB (-0.2%) N/A
StripeIdentitySize
com.stripe.StripeIdentitySize		 1.0 (1) 1.4 MB ⬇️ 2.7 kB (-0.2%) 4.4 MB ⬆️ 288 B N/A
StripeApplePaySize
com.stripe.StripeApplePaySize		 1.0 (1) 486.8 kB ⬇️ 2.0 kB (-0.42%) 1.7 MB ⬇️ 108 B N/A
StripeFinancialConnectionsSize
com.stripe.StripeFinancialConnectionsSize		 1.0 (1) 1.5 MB ⬇️ 1.5 kB (-0.1%) 4.8 MB ⬆️ 1.3 kB (0.03%) N/A

StripeSize 1.0 (1)
com.stripe.StripeSize

⚠️ Found new unused protocol: EmbeddedPaymentElementDelegate
⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬇️ 1.4 kB (-0.02%)
Total download size change: ⬇️ 2.3 kB (-0.11%)

Largest size changes

Item Install Size Change
StripeCore.STPAnalyticEvent.init(rawValue) ⬇️ -4.2 kB
Other ⬆️ 2.8 kB
View Treemap

Image of diff

StripePaymentsSize 1.0 (1)
com.stripe.StripePaymentsSize

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬇️ 1.4 kB (-0.03%)
Total download size change: ⬇️ 2.8 kB (-0.23%)

Largest size changes

Item Install Size Change
StripeCore.STPAnalyticEvent.init(rawValue) ⬇️ -4.2 kB
DYLD.Exports ⬇️ -720 B
Other ⬆️ 3.5 kB
View Treemap

Image of diff

StripePaymentsUISize 1.0 (1)
com.stripe.StripePaymentsUISize

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬇️ 1.4 kB (-0.02%)
Total download size change: ⬇️ 2.8 kB (-0.15%)

Largest size changes

Item Install Size Change
StripeCore.STPAnalyticEvent.init(rawValue) ⬇️ -4.2 kB
Other ⬆️ 2.7 kB
View Treemap

Image of diff

StripePaymentSheetSize 1.0 (1)
com.stripe.StripePaymentSheetSize

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬇️ 21.6 kB (-0.2%)
Total download size change: ⬇️ 8.4 kB (-0.22%)

Largest size changes

Item Install Size Change
🗑 StripePaymentSheet.EmbeddedPaymentElementViewModel ⬇️ -5.6 kB
StripeCore.STPAnalyticEvent.init(rawValue) ⬇️ -4.2 kB
DYLD.Exports ⬇️ -1.6 kB
DYLD.Fixups ⬇️ -960 B
DYLD.String Table ⬇️ -904 B
View Treemap

Image of diff

StripeIdentitySize 1.0 (1)
com.stripe.StripeIdentitySize

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬆️ 288 B
Total download size change: ⬇️ 2.7 kB (-0.2%)

Largest size changes

Item Install Size Change
StripeCore.STPAnalyticEvent.init(rawValue) ⬇️ -4.2 kB
Other ⬆️ 4.5 kB
View Treemap

Image of diff

StripeApplePaySize 1.0 (1)
com.stripe.StripeApplePaySize

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬇️ 108 B
Total download size change: ⬇️ 2.0 kB (-0.42%)

Largest size changes

Item Install Size Change
StripeCore.STPAnalyticEvent.init(rawValue) ⬇️ -4.2 kB
Other ⬆️ 4.1 kB
View Treemap

Image of diff

StripeFinancialConnectionsSize 1.0 (1)
com.stripe.StripeFinancialConnectionsSize

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬆️ 1.3 kB (0.03%)
Total download size change: ⬇️ 1.5 kB (-0.1%)

Largest size changes

Item Install Size Change
StripeCore.STPAnalyticEvent.init(rawValue) ⬇️ -4.2 kB
Other ⬆️ 5.4 kB
View Treemap

Image of diff

Unsuccessful Builds

Name Message
StripeConnectSize
com.stripe.StripeConnectSize		 The diff could not be determined because no build for 847b7cb was uploaded

🛸 Powered by Emerge Tools

@emerge-tools Emerge Tools
Copy link

emerge-tools bot commented Jan 31, 2025 •
edited
Loading

2 builds increased size, 6 builds had no size change

Name Version Download Change Install Change Approval
StripeSize
com.stripe.StripeSize		 1.0 (1) 2.1 MB ⬇️ 9 B 6.9 MB - N/A
StripePaymentsSize
com.stripe.StripePaymentsSize		 1.0 (1) 1.2 MB ⬆️ 1 B 4.2 MB - N/A
StripePaymentsUISize
com.stripe.StripePaymentsUISize		 1.0 (1) 1.9 MB ⬇️ 2 B 6.4 MB - N/A
StripePaymentSheetSize
com.stripe.StripePaymentSheetSize		 1.0 (1) 3.8 MB ⬆️ 2 B 11.1 MB - N/A
StripeIdentitySize
com.stripe.StripeIdentitySize		 1.0 (1) 1.4 MB ⬆️ 2 B 4.4 MB - N/A
StripeApplePaySize
com.stripe.StripeApplePaySize		 1.0 (1) 488.9 kB - 1.7 MB - N/A
StripeFinancialConnectionsSize
com.stripe.StripeFinancialConnectionsSize		 1.0 (1) 1.5 MB ⬆️ 773 B (0.05%) 4.8 MB ⬆️ 1.4 kB (0.03%) N/A
StripeConnectSize
com.stripe.StripeConnectSize		 1.0 (1) 1.7 MB ⬆️ 652 B (0.04%) 5.4 MB ⬆️ 1.4 kB (0.03%) N/A

StripeSize 1.0 (1)
com.stripe.StripeSize

No changes to report

StripePaymentsSize 1.0 (1)
com.stripe.StripePaymentsSize

No changes to report

StripePaymentsUISize 1.0 (1)
com.stripe.StripePaymentsUISize

No changes to report

StripePaymentSheetSize 1.0 (1)
com.stripe.StripePaymentSheetSize

No changes to report

StripeIdentitySize 1.0 (1)
com.stripe.StripeIdentitySize

No changes to report

StripeApplePaySize 1.0 (1)
com.stripe.StripeApplePaySize

No changes to report

StripeFinancialConnectionsSize 1.0 (1)
com.stripe.StripeFinancialConnectionsSize

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬆️ 1.4 kB (0.03%)
Total download size change: ⬆️ 773 B (0.05%)

Largest size changes

Item Install Size Change
Other ⬆️ 1.4 kB
View Treemap

Image of diff

StripeConnectSize 1.0 (1)
com.stripe.StripeConnectSize

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬆️ 1.4 kB (0.03%)
Total download size change: ⬆️ 652 B (0.04%)

Largest size changes

Item Install Size Change
Other ⬆️ 1.4 kB
View Treemap

Image of diff

🛸 Powered by Emerge Tools

@mats-stripe mats-stripe force-pushed the mats/consolidate_presentation_management branch from dba79ce to 653c587 Compare January 31, 2025 18:20
@mats-stripe mats-stripe merged commit 658f0f9 into master Feb 3, 2025
5 checks passed
@mats-stripe mats-stripe deleted the mats/consolidate_presentation_management branch February 3, 2025 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tillh-stripe tillh-stripe

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mats-stripe @tillh-stripe