Use loggedInUserData for Avatar props
#149
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/statelyai/xstate-viz/DtUvrUdXSdAGfsUmxEFMANVsQRdg |
src/Login.tsx
Outdated
| session?.user?.user_metadata?.full_name || | ||
| session?.user?.user_metadata?.user_name | ||
| } | ||
| src={loggedInUserData!.avatarUrl || undefined} |
There was a problem hiding this comment.
When is it possible that this can be undefined?
There was a problem hiding this comment.
If we assume that we only support GitHub-authorized users - then never. I think GitHub always provides an avatar for us - even if it's a default one. I've actually created a fresh user the other day for testing purposes and the avatar was in its data right from the beginning.
If we don't make that assumption though... this could happen. It's more a future-proof thing rather than a necessity for the current state of things.
Andarist
force-pushed
the
andarist/use-logged-in-user-data
branch
from
9912b33 to
19da0fe
Compare
| src={loggedInUserData.avatarUrl || undefined} | ||
| name={loggedInUserData.displayName || undefined} |
There was a problem hiding this comment.
I've checked the implementation of Avatar.
srcis not required - "initials" are rendered if you don't providesrc- initials require
namethough - if it's not provided a default icon is used instead
I would actually probably prefer using a fallback icon at all times, wdyt?
|
@Andarist Anything preventing this being merged? Other than the conflicts |
…se-logged-in-user-data
|
|
Merged conflicts, should be fine to go in |
|
@mattpocock I'll handle this soon-ish. It's just so low priority that it got pushed down on my priority list - since I still want to do some minor changes here |
It's a follow up to #110 (comment)