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: allow apple pay shipping events to emit #1298

Merged

Conversation

p-sebastian
Copy link
Contributor

@p-sebastian p-sebastian commented Feb 14, 2023

Summary

onDidSetShippingMethod & onDidSetShippingContact events will again send data back to the useApplePay hook listeners as well as by using the standard const eventEmitter = new NativeEventEmitter(NativeModules.StripeSdk); listeners

What happened was that the:

if let callback = self.shippingMethodUpdateJSCallback { ... }

Would never be set in any place other than using the ApplePayButton component from Stripe, meaning that if you were using the useApplePay hook or needed to listen to the shipping changes, it would skip and never trigger the sendEvent function

Motivation

I needed to listen to the shipping address changes programatically because the app we use has shipping available to different countries which require different tax calculations and currencies.

Testing

  • I tested this manually
  • I added automated tests

Documentation

Select one:

  • I have added relevant documentation for my changes.
  • This PR does not result in any developer-facing changes.

@CLAassistant
Copy link

CLAassistant commented Feb 14, 2023

CLA assistant check
All committers have signed the CLA.

@charliecruzan-stripe
Copy link
Collaborator

hey @p-sebastian , thanks for the contribution!

So you're using the useApplePay hook without using the ApplePayButton component? Apple requires using their specific button for this (otherwise your app will be rejected), so can I ask what you're using for the button then?

@p-sebastian
Copy link
Contributor Author

hey @charliecruzan-stripe , Apple allows you to create the button as long as it follows the guidelines, so it is an ApplePay button but created normally by me in react-native. (it is already running in production so it has been approved by apple)

Difference is that in the onPress handler I call the presentApplePay like the docs in stripe say

const { presentApplePay } = useApplePay({
    onShippingMethodSelected,
    onShippingContactSelected,
});
 
const onApplePayPress = async () => {
  const { error, paymentMethod } = await presentApplePay({
    ...
  });

  // Handle confirmation
};

While the button exported by stripe -> ApplePayButton that uses the ApplePayButtonNative contains these functions as props, even though the Props type doesn't define them.

export interface NativeProps {
  ...
  onShippingMethodSelectedAction?: (
    ...
  ) => void;
  onShippingContactSelectedAction?: (
    ...
  ) => void;
  onCouponCodeEnteredAction?: (
    ...
  ) => void;
}

The ApplePayButtonView.swift has the following mapping where the self.shippingMethodUpdateJSCallback is supposed to get set.

 @objc func handleApplePayButtonTapped() {
        if onPressAction != nil {
            onPressAction!(["true": true])
            // JS Callbacks are all no-ops since in legacy code (useApplePay hook),
            // this behavior is controlled via the onDidSetShippingMethod and onDidSetShippingContact
            // events
            stripeSdk?.shippingMethodUpdateJSCallback = doesNothing
            stripeSdk?.shippingContactUpdateJSCallback = doesNothing
            stripeSdk?.couponCodeEnteredJSCallback = doesNothing
        } else {
            stripeSdk?.shippingMethodUpdateJSCallback = onShippingMethodSelectedAction
            stripeSdk?.shippingContactUpdateJSCallback = onShippingContactSelectedAction
            stripeSdk?.couponCodeEnteredJSCallback = onCouponCodeEnteredAction
        }
    }

On a side note, I noticed that you will be switching to start using the:

const { error, paymentMethod } = await createPlatformPayPaymentMethod({ applePay: {...} })

But that doesn't have the 2 event listeners and the manual eventEmitter setup also doesn't work, it only works if it runs with the useApplePay hook, probably because of the self.hasLegacyApplePayListeners that doesn't get set.

@charliecruzan-stripe
Copy link
Collaborator

Apple allows you to create the button as long as it follows the guidelines

Interesting, I didn't know that. FWIW, as Apple changes the design requirements, I would imagine it's easier to just use their PKPaymentButton rather than having to update your custom component every now and then to re-follow their guidelines. But maybe that doesn't happen often enough to be an issue for you!

@p-sebastian
Copy link
Contributor Author

p-sebastian commented Feb 14, 2023

Interesting, I didn't know that. FWIW, as Apple changes the design requirements, I would imagine it's easier to just use their PKPaymentButton rather than having to update your custom component every now and then to re-follow their guidelines. But maybe that doesn't happen often enough to be an issue for you!

Hasn't happened to me yet, but they can get very picky when it comes to approving it, it's the same for the sign-in with apple button

@charliecruzan-stripe
Copy link
Collaborator

Gotchya, and just out of curiosity, why do you prefer using your own custom component then? Do you do the same thing for the Google Pay button component? Is there something i could add to our pre-provided components?

@p-sebastian
Copy link
Contributor Author

p-sebastian commented Feb 14, 2023

@charliecruzan-stripe It allows you a bit more flexibility on the style and it is not that difficult to make. Also if you are using reanimated you could check RectButton for cancelling pan handlers when a button is inside, which I am not sure you can do with the stripe implemented one

Stripes version:

IMG_0131

Our version:

Font size and icon (an SVG) is the same as the checkout next to it, as well as borderRadius (although that is editable on the stripe one)

IMG_0129

@charliecruzan-stripe
Copy link
Collaborator

I see, Apple does not recommend altering the icon or text size in the Apple Pay button, so that is not something we would offer since it would open up the possibility of app rejections 😞

@charliecruzan-stripe charliecruzan-stripe merged commit 4cce506 into stripe:master Feb 22, 2023
@charliecruzan-stripe
Copy link
Collaborator

Merging to fix since this affected existing behavior, but I do want to mention that we design the Apple and Google Pay APIs to be used with our components (specifically, the PlatformPayButton), so your mileage may vary if you're using your own component. My biggest concern would probably be app review in that case, but we also do not test using our APIs with 3rd party apple/google pay button components

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants