Repository navigation
fix(ui): show dnd indicator in member list - #27
Nexform-star wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: barbacane-dev/burst/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
ndreno
left a comment
There was a problem hiding this comment.
Thanks a lot for this, and congrats on the first contribution to Burst 🎉 Reusing DndIndicator and passing the user you already have is exactly the right approach. A few small things before we merge:
1. Spacing and truncation. The indicator sits inside the truncate span, right after the name, so it touches the name and gets clipped when a name is long. The sidebar does it like this, which would fit here too:
<span className="flex items-center gap-1">
<span className="truncate">
{name}
{isMe && <span className="text-gray-400"> (you)</span>}
</span>
<DndIndicator userId={m.userId} user={u} />
</span>2. Keep the fallback. u comes from usersById.get(m.userId) and can be undefined when a member isn't in the loaded users. Passing userId={m.userId} as well (as above) lets the indicator look the user up in that case.
3. The test. Making the shared bob fixture quiet changes him for every other test in the file. Could you add a dedicated quiet member instead, and also check that a member who isn't quiet shows no indicator? #19 asked for both cases. Also, the file is missing a final newline.
4. Sign-off. CONTRIBUTING asks for a DCO sign-off on each commit: git commit --amend -s and a force-push is enough.
About the two Windows failures in runtime-env.test.ts (sh ENOENT): that's on us, those tests shell out to sh. Not yours to fix here, it's tracked in #28.
Thanks again!
|
Thanks for the review! No problem—I'll fix all that and push the changes. Thanks also for the details about the Windows tests 👍 |
Signed-off-by: Nexform-star <potopilo123@hotmail.com>
873d7ba to
a68c29f
Compare
|
Am I talking to a bot? |
|
No lol, Do you speak French? |
|
oui :) |
|
Ah bah super, ça sera plus simple pour échanger alors ;) |
What
Show the do-not-disturb indicator next to users in the channel member list.
Changes
DndIndicatorcomponent.Testing
npm run lint✅npm run build✅npm testsh ENOENTfailures inruntime-env.test.ts.Closes #19