-
Notifications
You must be signed in to change notification settings - Fork 8
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
UI for mock googlepay token creation and conversion #167
Conversation
yarn.lock
Outdated
@@ -5602,7 +5607,7 @@ expect@^27.4.2: | |||
jest-message-util "^27.4.2" | |||
jest-regex-util "^27.4.0" | |||
|
|||
express@^4.17.3: | |||
express@4.17.3: |
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.
is it intentional?
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.
nope, not sure why that's in there.. just pushed an additional commit to remove
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.
(that happens with yarn add dependency to me also and then you need to go manually remove it when that happens)
<v-btn | ||
depressed | ||
class="mb-7" | ||
color="primary" | ||
:loading="loading" | ||
@click.prevent="autogenerateToken()" | ||
> | ||
Autogenerate token | ||
</v-btn> |
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.
Should this only be shown in sandbox/smokebox?
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.
Implemented conditional in latest commit
payload = { | ||
type, | ||
token_data: { | ||
protocolVersion: 'ECv1', |
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.
should this be a customer inputed value as well? If there's only 1 possible value. Let's do a drop down with only one selection pre-selected?
} | ||
} | ||
|
||
console.log(payload) |
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.
should we be logging this?
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.
removed this line
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.
lgtm, but I wonder if we could add some unit test
Happy to add unit tests - are you thinking a test for the token conversion? In that case I could add the test in when I actually wire up the button to the endpoint |
} | ||
|
||
showAutogenerateButton() { | ||
return getIsLocalHost() || getIsSmokebox() || getIsSandbox() |
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.
hahaha. I think you can just use !getLive()
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.
lgtm. Just one nit.
yes, thanks |
@@ -27,6 +27,7 @@ | |||
"@nuxtjs/dotenv": "1.4.1", | |||
"@nuxtjs/vuetify": "1.12.2", | |||
"@types/card-validator": "7.0.1", | |||
"@types/googlepay": "^0.6.4", |
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.
you shouldn't have these ^
in here
otherwise all is good
This is a PR for the UI to generate and convert a google pay token in payments sample app.
)