feat(clerk-js): Use passkey as first factor in <SignIn/>#3000
feat(clerk-js): Use passkey as first factor in <SignIn/>#3000panteliselef merged 16 commits intomainfrom
<SignIn/>#3000Conversation
🦋 Changeset detectedLatest commit: 7163d99 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| const credential = (await navigator.credentials.get({ | ||
| publicKey: publicKeyOptions, | ||
| mediation: conditionalUI ? 'conditional' : 'optional', | ||
| signal: __internal_WebAuthnAbortService.createAbortSignal(), |
There was a problem hiding this comment.
Only one call to retrieve credentials can exist at time.
That means in the case of autofill user may never interact with it, this makes sure that every new call to retrieve the credentials will cancel the previews one.
| <Icon | ||
| elementDescriptor={descriptors.passkeyIcon} | ||
| icon={Fingerprint} | ||
| sx={t => ({ | ||
| color: t.colors.$neutralAlpha500, | ||
| marginInline: 'auto', | ||
| paddingBottom: t.sizes.$1, | ||
| width: t.sizes.$12, | ||
| height: t.sizes.$12, | ||
| })} |
There was a problem hiding this comment.
@desiprisg @anagstef Please let me know if we have new patterns that this code should align with.
In the designs this icon have a size of 48x48 which is not covered by the current Icon implementation and just adding an xl variant does not feel right as the lg represents a 20x20 so the difference would be quite large between them.
We may add a new Header.Icon component, but as long as this is used in only one place do we need them to ?
There was a problem hiding this comment.
Maybe the Header.Icon will make sense in order to have a proper descriptor instead of the passkeyIcon descriptor. WDYT ?
There was a problem hiding this comment.
It's ok to keep the width and height values.
There was a problem hiding this comment.
I'd say we don't add a Header.Icon yet. Let's keep the Icon as is for now. :)
panteliselef
force-pushed
the
elef/sdk-1402-ui-sign-in-passkey
branch
from
32e070c to
b9f8bf3
Compare
LekoArts
left a comment
LekoArts
left a comment
There was a problem hiding this comment.
Generally LGTM. Left a non-blocking comment.
I'd also make sure that your question is answered 👍
| }, | ||
| passkey: { | ||
| title: 'Use your passkey', | ||
| subtitle: "Using your passkey confirms it's you. Your device may ask for your fingerprint, face or screen lock.", |
There was a problem hiding this comment.
| subtitle: "Using your passkey confirms it's you. Your device may ask for your fingerprint, face or screen lock.", | |
| subtitle: "Your device may ask for your fingerprint, face or screen lock.", |
The first sentence feels like a filler sentence so this seems more concise.
What do you mean by "screen lock" exactly? Do you mean the phone PIN? If yes, that wasn't clear to me
There was a problem hiding this comment.
Hey @LekoArts this text is copied from the designs. I'm sure we can iterate more on this as more people use it during the beta. Yes, by "screen lock" we mean a PIN.
panteliselef
force-pushed
the
elef/sdk-1402-ui-sign-in-passkey
branch
from
78ee4fd to
7163d99
Compare
4 participants
Description
Things to consider
Autofill Passkey
Screen.Recording.2024-03-15.at.1.14.48.PM.mov
Discoverable Passkeys
Screen.Recording.2024-03-15.at.1.15.43.PM.mov
Sign in with identifier + passkeys as first factor
Screen.Recording.2024-03-15.at.1.15.56.PM.mov
Checklist
npm testruns as expected.npm run buildruns as expected.Type of change