-
Notifications
You must be signed in to change notification settings - Fork 16
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
DirectAuth updates for Storage #1248
Conversation
Your Render PR Server URL is https://files-ui-stage-pr-1248.onrender.com. Follow its progress at https://dashboard.render.com/static/srv-c3irutk7o9q2usc9k5r0. |
Your Render PR Server URL is https://storage-ui-stage-pr-1248.onrender.com. Follow its progress at https://dashboard.render.com/static/srv-c3iruu47o9q2usc9k6d0. |
… into feat/direct-auth-1207
… into feat/direct-auth-1207
… into feat/direct-auth-1207
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.
Working super well. Found a couple nits but that's it :)
packages/storage-ui/src/Components/Modules/LoginModule/PasswordlessEmail.tsx
Outdated
Show resolved
Hide resolved
packages/storage-ui/src/Components/Modules/LoginModule/PasswordlessEmail.tsx
Outdated
Show resolved
Hide resolved
packages/storage-ui/src/Components/Modules/LoginModule/PasswordlessEmail.tsx
Outdated
Show resolved
Hide resolved
packages/storage-ui/src/Components/Modules/LoginModule/PasswordlessEmail.tsx
Outdated
Show resolved
Hide resolved
packages/storage-ui/src/Components/Modules/LoginModule/PasswordlessEmail.tsx
Outdated
Show resolved
Hide resolved
const getIdentityToken = async ( | ||
loginType: IdentityProvider, | ||
tokenInfo?: {token: IdentityToken; email: string} | ||
): Promise<{identityToken: IdentityToken; userInfo: any}> => { |
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.
Any way we can type this strongly?
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 played around with it and have not managed to come up with a type definition that works. Given the tiny surface area of this type, I would say we should leave it as is.
Co-authored-by: Thibaut Sardan <[email protected]>
closes #1207