Fix conditional name in avatar - #102
Conversation
weltonfelix
left a comment
weltonfelix
left a comment
There was a problem hiding this comment.
The null checks only works if those properties are empty strings.
Also, String.chartAt() always return a string. I.e. "".charAt(0) returns "", so "" ?? "-" will always return "".
7750953 to
1887516
Compare
|
Address review feedback on the avatar fallbacks. The alt on the UserPopup avatar still dereferenced user.name[0] on the right-hand side of a ??, which is evaluated exactly when user.name is nullish, throwing instead of falling back. Derive displayName and avatarInitial once per component using ||, so empty strings are handled alongside null and undefined, and restore the username fallback that the previous version dropped. Also replace the unconditional outline: none on the hamburger button with a :focus:not(:focus-visible) rule, so the focus ring stays visible for keyboard navigation.
|
dcruzb
left a comment
dcruzb
left a comment
There was a problem hiding this comment.
Approving after addressing the review feedback in f06c695.
The remaining crash was in the UserPopup avatar: alt={user.name ?? user.name[0] ?? ''} dereferenced user.name[0] on the right-hand side of a ??, which is evaluated precisely when user.name is nullish — so it threw in the exact case the fix was meant to handle.
Both components now derive the value once, using || so empty strings are covered alongside null/undefined:
const displayName = user.name || user.username || '';
const avatarInitial = displayName.charAt(0) || '-';This also restores the username fallback that the previous revision dropped, which matters because Navbar populates username from preferred_username.
Additionally, the unconditional outline: none on the hamburger button was replaced with &:focus:not(:focus-visible), keeping the pointer-interaction behavior while preserving a visible focus ring for keyboard navigation.
Verified: tsc --noEmit clean and npm run build-lib succeeds.
3 participants
No description provided.