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

feat(venmo): relax eligibility #2393

Merged
merged 9 commits into from
May 20, 2024
Merged

feat(venmo): relax eligibility #2393

merged 9 commits into from
May 20, 2024

Conversation

mnicpt
Copy link
Contributor

@mnicpt mnicpt commented May 17, 2024

Description

Relaxing venmo first render eligibility now that Venmo web experience is live and can better manage fallback from native app switch.

image

PR

https://paypal.atlassian.net/browse/DTVP-75

**Note:**The last requirement, 1d, will be implemented at a later date.

Why are we making these changes? Include references to any related Jira tasks or GitHub Issues

Reproduction Steps (if applicable)

Screenshots (if applicable)

Dependent Changes (if applicable)

Groups who should review (if applicable)

❤️ Thank you!

@mnicpt mnicpt requested a review from a team as a code owner May 17, 2024 13:28
@@ -42,7 +42,6 @@ export function getVenmoConfig(): FundingSourceConfig {
experiment.venmoWebEnabled === false
) {
return {
native: true,
popup: true,
Copy link
Member

Choose a reason for hiding this comment

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

If device is Mobile & Venmo Web is disabled, then require popup? Am I reading that right? From this little snippet it seems like native should be true and popup should be false but I am likely missing context of the bigger picture

I just read your description though, is this saying:

we may still app-switch, but if we can't popup then definitely don't continue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wsbrunson By removing native, it is saying SFVC is ok. We will catch that just prior to app switching. If in SFVC there, then we will fallback to web. Popup relates to WebView and want to keep that there.

@mnicpt mnicpt merged commit face822 into main May 20, 2024
3 checks passed
@mnicpt mnicpt deleted the feat/venmoelig branch May 20, 2024 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants