-
Notifications
You must be signed in to change notification settings - Fork 559
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
DTPPCPSDK-2227: Return V2 Variables from getHostedButtonDetails 馃懢 #2385
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small comment re: preference types
src/hosted-buttons/types.js
Outdated
@@ -34,6 +34,11 @@ export type HostedButtonsInstance = {| | |||
render: (string | HTMLElement) => Promise<void>, | |||
|}; | |||
|
|||
export type HostedButtonPreferences = {| | |||
buttonPreferences: $ReadOnlyArray<string>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possible to narrow these string
types to known preferences and sources? Could do that in follow-up implementation PR too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Donezo funzo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! 馃 馃殌 Should we add tagline
to the tests?
|
Description
/ncp/api/form-fields/
, I added some quick unit tests to make suregetHostedButtonDetails
can handle either response appropriately.Why are we making these changes? Include references to any related Jira tasks or GitHub Issues
JIRA: DTPPCPSDK-2227
Reproduction Steps (if applicable)
Screenshots (if applicable)
Dependent Changes (if applicable)
Groups who should review (if applicable)
鉂わ笍 Thank you!