-
Notifications
You must be signed in to change notification settings - Fork 89
Add display of ID if user does not have email #450
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
Conversation
src/modules/peers/PeerNameCell.tsx
Outdated
| > | ||
| <div className={"text-nb-gray-400 font-light truncate"}> | ||
| {userOfPeer?.email} | ||
| {!userOfPeer?.email && userOfPeer?.id} |
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.
could probably be userOfPeer?.email || userOfPeer?.name || userOfPeer?.user_id || userOfPeer?.id:
- email would come first as usual
- name second as it's still user readable
- synchronized
user_idfrom IdP (usually not readable) - internal object
idfor the user (never readable)
the existing construct looks weird to me, but I didn't write JS for a decade so don't take my word on it.
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 updated the code and made sure it works.
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.
One more thing that I thought of, we should probably prefix the user_id/id with "user:" string, but I'm not sure how to type it out correctly in TypeScript or JS.
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.
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.
@tropnikovvl exactly what you screenshotted
on the UX side, we could end the series of ||s with "<unknown>" so it's obvious nothing was found instead of id being "undefined" string
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.
oh, actually we shouldn't output anything at all when the user is undefined, this will simply mean there is no user attached to the peer
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.
something like (pseudocode):
user = userOfPeer?.email || userOfPeer?.name
userId = peer?.user_id || userOfPeer?.id
if !user && userId {
user = `user:${userId}`
}
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.
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.
great, @heisbrot can you review and merge this?
|
Hi @heisbrot , |
|
Hey @tropnikovvl right now it shows |
|
Hi @heisbrot , |


#449
netbirdio/netbird#3537
Before
After