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

chore(deps): replace tipsi with official stripe lib #10451

Merged
merged 41 commits into from
Jul 11, 2024

Conversation

brainbicycle
Copy link
Contributor

@brainbicycle brainbicycle commented Jul 3, 2024

This PR resolves PHIRE-1018

Description

Updates our payment dependencies from old tipsi forked + deprecated dependency to stripes official react native dependency.

Credit @araujobarret for much of the initial implementation: #9189

Before
before-low.mov
After
after-low.mov

PR Checklist

  • I have tested my changes on iOS and Android.
  • I hid my changes behind a feature flag, or they don't need one.
  • I have included screenshots or videos, or I have not changed the UI.
  • I have added tests, or my changes don't require any.
  • I added an app state migration, or my changes do not require one.
  • I have documented any follow-up work that this PR will require, or it does not require any.
  • I have added a changelog entry below, or my changes do not require one.

To the reviewers 👀

  • I would like at least one of the reviewers to run this PR on the simulator or device.
Changelog updates

Changelog updates

Cross-platform user-facing changes

iOS user-facing changes

Android user-facing changes

Dev changes

  • Updates stripe lib + associated card inputs (register to bid, profile) - brian

Need help with something? Have a look at our docs, or get in touch with us.

@brainbicycle brainbicycle self-assigned this Jul 3, 2024
@brainbicycle brainbicycle marked this pull request as draft July 3, 2024 13:23
@ArtsyOpenSource
Copy link
Contributor

ArtsyOpenSource commented Jul 3, 2024

This PR contains the following changes:

  • Dev changes (Updates stripe lib + associated card inputs (register to bid, profile) - brian)

Generated by 🚫 dangerJS against bb0c1d0

@brainbicycle brainbicycle marked this pull request as ready for review July 10, 2024 17:52
// focus top field on mount
useEffect(() => {
paymentInfoRef.current?.focus()
}, [])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

should be handled by autofocus prop

joeyAghion
joeyAghion previously approved these changes Jul 10, 2024
Copy link
Contributor

@joeyAghion joeyAghion left a comment

Choose a reason for hiding this comment

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

Excited for this upgrade...

@araujobarret
Copy link
Contributor

Maybe we can also remove this extra call, I'm pretty sure it was there just because of tipsi

gkartalis
gkartalis previously approved these changes Jul 11, 2024
Comment on lines -63 to -72
## react-native-credit-card-input

#### When can we remove this:

When we upgrade our deps to a version of react-native that includes removal of deprecated PropTypes.

#### Explanation/Context:

When updating to rn-0.69.10 we had to patch this due to deprecation of PropTypes. For this reason we also installed `deprecated-react-native-prop-types` to avoid errors and we patched the `react-native` package to use the deprecated PropTypes coming from the `deprecated-react-native-prop-types` package.

Copy link
Member

Choose a reason for hiding this comment

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

👏

Comment on lines -166 to -181
## react-native-credit-card-input

#### When can we remove this:

We can remove these hacks once we switch to Stripe's forthcoming official react-native library.

#### Explanation/Context:

These are fairly superficial styling hacks for

- focused/error border states
- shrinking the icon size to work nicely with our inputs
- aligning inner inputs nicely
- icon animation to work properly on android
- palette v3 colors

Copy link
Member

Choose a reason for hiding this comment

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

👋 awesome! we got rid of two hacks already 🔥

Comment on lines -99 to -121
- Issue with installing `tipsi-stripe`

```
[!] CocoaPods could not find compatible versions for pod "tipsi-stripe":
```

You need to run

```
bundle exec pod update tipsi-stripe
```

- Error during `bundle exec pod update tipsi-stripe`

```
checking whether the C compiler works... no
xcrun: error: SDK "iphoneos" cannot be located
xcrun: error: SDK "iphoneos" cannot be located
xcrun: error: SDK "iphoneos" cannot be located
```

You need to go to Xcode -> Preferences -> Locations and select Command Line Tools from the dropdown

Copy link
Member

Choose a reason for hiding this comment

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

👋

araujobarret
araujobarret previously approved these changes Jul 11, 2024
@brainbicycle brainbicycle merged commit 6f3234d into main Jul 11, 2024
7 checks passed
@brainbicycle brainbicycle deleted the brian/tipsi-bye-bye branch July 11, 2024 21:27
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.

5 participants