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

[NT-557] Hide "Other payment methods" label when Apple Pay button is hidden #940

Merged
merged 14 commits into from
Nov 13, 2019

Conversation

ifbarrera
Copy link
Contributor

📲 What

Hides the "Other payment methods" label above stored cards when the Apple Pay button is hidden.

🤔 Why

It's a bit strange to say "Other" payment methods when there is only one type of payment method available to the user.

🛠 How

Hiding the label when the Apple Pay button is hidden.

👀 See

Trello, screenshots, external resources?

After 🦋 Before 🐛
IMG_D5960EBB01E7-1 Simulator Screen Shot - iPhone 8 - 2019-11-11 at 18 09 53

♿️ Accessibility

N/A

🏎 Performance

N/A

✅ Acceptance criteria

  • Run the app on a device that doesn't have apple pay set up, or alternatively find a project in a country that doesn't support Apple Pay (ex. Mexico). Navigate to the pledge screen - you should not see the label that says "Other payment methods". The Apple Pay button should also be hidden.

@justinswart
Copy link
Contributor

Holding on this based on our Slack convo.

@ifbarrera ifbarrera added the blocked a PR that is blocked for external reasons label Nov 12, 2019
@ifbarrera ifbarrera added needs review and removed blocked a PR that is blocked for external reasons labels Nov 12, 2019
@@ -95,10 +96,14 @@ final class PledgePaymentMethodsViewController: UIViewController {
|> checkoutBackgroundStyle

_ = self.rootStackView
|> rootStackViewStyle
|> verticalStackViewStyle
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure what the point of this style is, I see we've had it for a while but it just sets the axis to vertical 🤷‍♂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ask @dusi 😆

|> \.isLayoutMarginsRelativeArrangement .~ true
|> \.layoutMargins .~ UIEdgeInsets(leftRight: CheckoutConstants.PledgeView.Inset.leftRight)
|> \.layoutMargins .~ .init(leftRight: Styles.grid(4))
Copy link
Contributor

Choose a reason for hiding this comment

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

What this 4 before?

@ifbarrera ifbarrera merged commit 3cf9d79 into master Nov 13, 2019
@ifbarrera ifbarrera deleted the NT-557-hide-cards-header branch November 13, 2019 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants