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

fix: fixes ApplePay button rendering logic #685

Merged
merged 3 commits into from
Nov 2, 2023

Conversation

borisprimer
Copy link
Contributor

@borisprimer borisprimer commented Sep 28, 2023

Fixed a bug and added a few more config options for SDK, to show/hide ApplePay if the device is not capable of doing it, etc.

RFC

@borisprimer borisprimer self-assigned this Sep 28, 2023
@borisprimer borisprimer marked this pull request as draft September 28, 2023 12:14
@borisprimer borisprimer requested a review from jnewc September 28, 2023 12:14
@github-actions
Copy link
Contributor

github-actions bot commented Sep 28, 2023

Warnings
⚠️ This PR doesn't seem to contain any updated Unit Test 🤔. Please consider double checking it.🙏

Generated by 🚫 Danger Swift against 22c7a7b

@github-actions
Copy link
Contributor

github-actions bot commented Sep 28, 2023

@borisprimer borisprimer requested a review from NQuinn27 October 4, 2023 11:53
@borisprimer borisprimer force-pushed the bn/apple_pay_button_bug branch 2 times, most recently from 0e4ad0c to 73a0ec0 Compare October 4, 2023 13:51
Copy link
Contributor

@NQuinn27 NQuinn27 left a comment

Choose a reason for hiding this comment

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

Nice work, a few comments

@borisprimer borisprimer force-pushed the bn/apple_pay_button_bug branch from 73a0ec0 to eedd5db Compare October 5, 2023 08:36
@NQuinn27 NQuinn27 changed the title Change ApplePay button rendering logic fix: fixes ApplePay button rendering logic Oct 5, 2023
@borisprimer borisprimer force-pushed the bn/apple_pay_button_bug branch 2 times, most recently from 539e2b9 to 7772a8a Compare October 5, 2023 11:55
@borisprimer borisprimer marked this pull request as ready for review October 9, 2023 08:07
@borisprimer borisprimer force-pushed the bn/apple_pay_button_bug branch from b1843e6 to 749c789 Compare October 9, 2023 17:31
Copy link
Contributor

@jnewc jnewc left a comment

Choose a reason for hiding this comment

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

Is there a reason it isn't filtered out as a payment method as the source? i.e. in listAvailablePaymentMethodsTypes

I think this doesn't fix the problem we discussed with different parts of the flow having a different outcome when querying whether Apple Pay is available (see diagram)

image

As I mentioned previously, listAvailablePaymentMethodsTypes has exactly the same filtering logic as paymentMethodConfigViewModels except for the ApplePay filtering. I don't think we want to leave this inconsistency in place.

@borisprimer borisprimer requested review from jnewc and NQuinn27 October 17, 2023 10:41
@borisprimer borisprimer force-pushed the bn/apple_pay_button_bug branch from 749c789 to 34b3659 Compare October 20, 2023 08:05
@borisprimer borisprimer merged commit 3d00d19 into master Nov 2, 2023
5 checks passed
@borisprimer borisprimer deleted the bn/apple_pay_button_bug branch November 2, 2023 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants