-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[MBL-1607] Add Estimated Shipping View to Late Pledge Checkout #2133
Conversation
b6d0c82
to
945f18e
Compare
Generated by 🚫 Danger |
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'm concerned about how this is making the late pledge VC layout worse, even when there's no estimated shipping. Can you confirm that Alison is okay with launching with this layout (extra left and right margins for the pledge view and weird gap between the scroll view and the pledge cta) even for late pledges without estimated shipping? If not, I would strongly recommend fixing it in this pr but I'm also okay with filing a follow-up ticket/fixing it in a separate pr (follow-up ticket should probably happen anyways so we can fix it for the next release, if it ends up not being launch blocking).
Kickstarter-iOS/Features/PledgeView/Controllers/PostCampaignCheckoutViewController.swift
Show resolved
Hide resolved
Kickstarter-iOS/Features/PledgeView/Controllers/PostCampaignCheckoutViewController.swift
Outdated
Show resolved
Hide resolved
Kickstarter-iOS/Features/PledgeView/Controllers/PostCampaignCheckoutViewController.swift
Outdated
Show resolved
Hide resolved
Kickstarter-iOS/Features/PledgeView/Controllers/PostCampaignCheckoutViewControllerTests.swift
Outdated
Show resolved
Hide resolved
...aignCheckoutViewControllerTests/testView_HasAddOns.lang_es_device_phone4_7inch_LoggedOut.png
Outdated
Show resolved
Hide resolved
private lazy var estimatedShippingStackView: UIStackView = { | ||
UIStackView(frame: .zero) | ||
|> \.translatesAutoresizingMaskIntoConstraints .~ 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.
@ifosli For the correct margins to be applied, the estimated shipping view needs to be added to a new stack view as a subview. This is true for UIKit views as well. I spent time rebuilding the view with UIKit to verify this.
📲 What
Adds a new SwiftUI to display estimated shipping cost
🤔 Why
JIRA TICKET
🛠 How
EstimatedShippingCheckoutView
toPostCampaignCheckoutViewController
👀 See
✅ Acceptance criteria