-
Notifications
You must be signed in to change notification settings - Fork 12
[Autofill] ios credit card support and overall cc fixes #801
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
base: main
Are you sure you want to change the base?
Conversation
5c99972
to
553a50e
Compare
9c7a56b
to
beb2f9d
Compare
This is exactly what I was looking for regarding Windows and credit card support. |
|
||
const ddgCcIconFilled = | ||
'data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iVVRGLTgiPz4KPHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSIyNCIgaGVpZ2h0PSIyNCIgZmlsbD0ibm9uZSI+CiAgICA8cGF0aCBkPSJNNSA5Yy0uNTUyIDAtMSAuNDQ4LTEgMXYyYzAgLjU1Mi40NDggMSAxIDFoM2MuNTUyIDAgMS0uNDQ4IDEtMXYtMmMwLS41NTItLjQ0OC0xLTEtMUg1eiIgZmlsbD0iIzc2NDMxMCIvPgogICAgPHBhdGggZmlsbC1ydWxlPSJldmVub2RkIiBjbGlwLXJ1bGU9ImV2ZW5vZGQiIGQ9Ik0xIDZjMC0yLjIxIDEuNzktNCA0LTRoMTRjMi4yMSAwIDQgMS43OSA0IDR2MTJjMCAyLjIxLTEuNzkgNC00IDRINWMtMi4yMSAwLTQtMS43OS00LTRWNnptNC0yYy0xLjEwNSAwLTIgLjg5NS0yIDJ2OWgxOFY2YzAtMS4xMDUtLjg5NS0yLTItMkg1em0wIDE2Yy0xLjEwNSAwLTItLjg5NS0yLTJoMThjMCAxLjEwNS0uODk1IDItMiAySDV6IiBmaWxsPSIjNzY0MzEwIi8+Cjwvc3ZnPgo='; | ||
'data:image/svg+xml;base64,PHN2ZyBmaWxsPSJub25lIiB2aWV3Qm94PSIwIDAgMjQgMjQiIHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyI+PHBhdGggZmlsbD0iIzc2NDMxMCIgZD0iTTYuODQyIDhBLjg0MS44NDEgMCAwIDAgNiA4Ljg0MXYyLjMxOGMwIC40NjQuMzc3Ljg0MS44NDIuODQxaDMuMzE3YS44NDEuODQxIDAgMCAwIC44NDEtLjg0MVY4Ljg0YS44NDEuODQxIDAgMCAwLS44NC0uODRINi44NDJaIi8+PHBhdGggZmlsbD0iIzc2NDMxMCIgZmlsbC1ydWxlPSJldmVub2RkIiBkPSJNMiA4YTUgNSAwIDAgMSA1LTVoMTBhNSA1IDAgMCAxIDUgNXY4YTUgNSAwIDAgMS01IDVIN2E1IDUgMCAwIDEtNS01VjhabTUtMy41QTMuNSAzLjUgMCAwIDAgMy41IDh2NmgxN1Y4QTMuNSAzLjUgMCAwIDAgMTcgNC41SDdabTAgMTVBMy41MDIgMy41MDIgMCAwIDEgMy42NDUgMTdoMTYuNzFBMy41MDIgMy41MDIgMCAwIDEgMTcgMTkuNUg3WiIgY2xpcC1ydWxlPSJldmVub2RkIi8+PC9zdmc+'; | ||
|
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 wonder if we could start using URL references along with the new ESLint loader you recently created. That would really help cut down on some of the noise here 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.
We could in a separate build step, although maybe a bit better improvement as a follow-up.
src/Form/Form.js
Outdated
@@ -531,7 +531,7 @@ class Form { | |||
* @param {boolean} shouldCheckForDecorate | |||
*/ | |||
execOnInputs(fn, inputType = 'all', shouldCheckForDecorate = true) { | |||
const inputs = this.inputs[inputType]; | |||
const inputs = [...this.inputs[inputType]]; |
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.
Why is this needed? We're using a for…of
loop which is supported for all iterables.
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.
Not necessary at all, originally I had changed the type of SupportedMainTypes
to be an array, so it's from the remains of that work.
Anyway I am changing the approach overall.
getFirstViableCredentialsInput() { | ||
return [...this.inputs.credentials].find((input) => canBeInteractedWith(input) && isPotentiallyViewable(input)); | ||
getFirstViableInputForType(dataType) { | ||
return [...this.inputs[dataType]].find((input) => canBeInteractedWith(input) && isPotentiallyViewable(input)); | ||
} | ||
|
||
async promptLoginIfNeeded() { |
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 don't understand why we're doing it here. This method is about autoprompting for login forms (lacks a comment, ouch), whereas the change we're looking at should be done after the initial manual prompt of a credit card prompt.
9a756cf
to
1a19ce9
Compare
1a19ce9
to
fa7cef1
Compare
3a954f0
to
0d0d804
Compare
* Temporary speedup to cc testing Signed-off-by: Emanuele Feliziani <[email protected]> * Add Yahoo payment form Signed-off-by: Emanuele Feliziani <[email protected]> * test: add form for payment step on att * Better phone and more resilient clicking Signed-off-by: Emanuele Feliziani <[email protected]> * test: sams club card * Support prompting from all cc fields Signed-off-by: Emanuele Feliziani <[email protected]> * Fix Yahoo credit card form Signed-off-by: Emanuele Feliziani <[email protected]> * Fixed the cc touched thing Signed-off-by: Emanuele Feliziani <[email protected]> * Fix sams club test (it had wrong manual values) Signed-off-by: Emanuele Feliziani <[email protected]> * Prettier Signed-off-by: Emanuele Feliziani <[email protected]> * Minor regex tweak Signed-off-by: Emanuele Feliziani <[email protected]> * Add Time form + fix it Signed-off-by: Emanuele Feliziani <[email protected]> * Rename file, lol Signed-off-by: Emanuele Feliziani <[email protected]> * Add victoriassecret + fix it Signed-off-by: Emanuele Feliziani <[email protected]> * Add uhaul form + fix it Signed-off-by: Emanuele Feliziani <[email protected]> * fix: dispatch minimal events for creditcard number * Remove console.log Signed-off-by: Emanuele Feliziani <[email protected]> * fix: dispatch minimal events for creditcard number * Recompile Signed-off-by: Emanuele Feliziani <[email protected]> * Revert "fix: dispatch minimal events for creditcard number" This reverts commit f86d194. * fix: remove dummy code * Revert "fix: dispatch minimal events for creditcard number" This reverts commit e9b1b85. --------- Signed-off-by: Emanuele Feliziani <[email protected]> Co-authored-by: dbajpeyi <[email protected]>
aaaddee
to
d78fbaf
Compare
3079445
to
0b25311
Compare
0b25311
to
a8d7c02
Compare
src/Form/Form.js
Outdated
// Don't open the tooltip on input focus whenever it's showing in-context signup | ||
if (isIncontextSignupAvailable) return false; | ||
} | ||
|
||
// On ios, always send the calls to the native side, so they can decide on the UX | ||
// On ios we either show a tooltip or the keyboard extension which non-blocking. | ||
if (isIos && mainType === 'creditCards') return true; |
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.
WIP, just pass all calls to native side to try out how the whole extension UX fills. Probably we will be a bit more conservative after Ship Review/playing around with the ios build.
4924aea
to
b67d04b
Compare
Reviewer:
Asana: https://app.asana.com/1/137249556945/task/1210112883952078?focus=true
Description
creditCards
prompting behavior on mobile tocredentials
data type,Steps to test