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

Shopper insights rp2 feature selected event button #1247

Conversation

warmkesselj
Copy link
Contributor

Summary of changes

Refactor sendPayPalSelectedEvent() and sendVenmoSelectedEvent() to sendSelectedEvent()
Screenshot 2024-12-19 at 9 50 57 AM

Checklist

  • Added a changelog entry
  • Relevant test coverage
  • Tested and confirmed payment flows affected by this change are functioning as expected

Authors

@jwarmkessel
@warmkesselj

@warmkesselj warmkesselj requested a review from a team as a code owner December 19, 2024 17:55
@warmkesselj warmkesselj changed the base branch from main to shopper-insights-rp2-feature December 19, 2024 17:55
}
fun sendSelectedEvent(
buttonType: ButtonType,
presentmentDetails: PresentmentDetails
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we needed presentment details for this method, just the button type.

@@ -189,19 +188,19 @@ class ShopperInsightsClient internal constructor(
}

/**
* Call this method when the PayPal button has been selected/tapped by the buyer.
* Call this method when the PayPal, Venmo or Other button has been successfully displayed to the buyer.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Call this method when the PayPal, Venmo or Other button has been successfully displayed to the buyer.
* Call this method when the PayPal, Venmo or Other button has been selected/tapped by the buyer.

Copy link
Contributor

@sarahkoop sarahkoop left a comment

Choose a reason for hiding this comment

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

One doc string nitpick but LGTM!

Copy link
Contributor

@jaxdesmarais jaxdesmarais left a comment

Choose a reason for hiding this comment

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

:shipit:

@warmkesselj warmkesselj merged commit 934d2ba into shopper-insights-rp2-feature Dec 19, 2024
3 checks passed
@warmkesselj warmkesselj deleted the shopper-insights-rp2-feature-selected-event-button branch December 19, 2024 20:58
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.

3 participants