Skip to content

Highlight current user in signal suggested reviewers #2435

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.

Sign up for GitHub

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

Open
rafaeelaudibert wants to merge 2 commits into
base: main
Choose a base branch
from
+39 −11
Open

Highlight current user in signal suggested reviewers #2435

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9cfee5f
Highlight current user in signal suggested reviewers
rafaeelaudibert May 29, 2026
862b414
Wrap commit SHA and info icon in a single tooltip
rafaeelaudibert May 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 39 additions & 11 deletions apps/code/src/renderer/features/inbox/components/detail/ReportDetailPane.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
CaretRightIcon,
ChatCircleIcon,
EyeIcon,
InfoIcon,
LinkSimpleIcon,
Plus,
ThumbsDownIcon,
Expand Down Expand Up @@ -211,8 +212,12 @@ export function ReportDetailPane({
const reviewerArtefact = allArtefacts.find(
(a): a is SuggestedReviewersArtefact => a.type === "suggested_reviewers",
);
return reviewerArtefact?.content ?? [];
}, [allArtefacts]);
const reviewers = reviewerArtefact?.content ?? [];
if (!me?.uuid) return reviewers;
const meIndex = reviewers.findIndex((r) => r.user?.uuid === me.uuid);
if (meIndex <= 0) return reviewers;
return [reviewers[meIndex], ...reviewers.filter((_, i) => i !== meIndex)];
}, [allArtefacts, me?.uuid]);

const signalFindings = useMemo(() => {
const map = new Map<string, SignalFindingArtefact["content"]>();
Expand Down Expand Up @@ -818,15 +823,38 @@ export function ReportDetailPane({
{reviewer.relevant_commits.map((commit, i) => (
<span key={commit.sha}>
{i > 0 && ", "}
<Tooltip content={commit.reason || undefined}>
<a
href={commit.url}
target="_blank"
rel="noreferrer"
className="font-mono text-gray-9 hover:text-gray-11"
>
{commit.sha.slice(0, 7)}
</a>
<Tooltip
content={
isMe ? (
<Flex direction="column" gap="1">
<Text as="div" size="1" weight="bold">
Why was I assigned?
</Text>
<Text as="div" size="1">
{commit.reason}
</Text>
</Flex>
) : (
commit.reason || undefined
)
}
Comment on lines +826 to +840
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 When isMe is true but commit.reason is empty/null, the tooltip content is still a non-undefined <Flex> element (containing "Why was I assigned?" with a blank body), so hovering the SHA link will open an incomplete tooltip. The non-isMe branch already guards this with || undefined — the same guard is needed here.

Suggested change
<Tooltip
content={
isMe ? (
<Flex direction="column" gap="1">
<Text as="div" size="1" weight="bold">
Why was I assigned?
</Text>
<Text as="div" size="1">
{commit.reason}
</Text>
</Flex>
) : (
commit.reason || undefined
)
}
<Tooltip
content={
isMe ? (
commit.reason ? (
<Flex direction="column" gap="1">
<Text as="div" size="1" weight="bold">
Why was I assigned?
</Text>
<Text as="div" size="1">
{commit.reason}
</Text>
</Flex>
) : undefined
) : (
commit.reason || undefined
)
}
Prompt To Fix With AI 
This is a comment left during a code review.
Path: apps/code/src/renderer/features/inbox/components/detail/ReportDetailPane.tsx
Line: 826-840

Comment:
When `isMe` is `true` but `commit.reason` is empty/null, the tooltip content is still a non-`undefined` `<Flex>` element (containing "Why was I assigned?" with a blank body), so hovering the SHA link will open an incomplete tooltip. The non-`isMe` branch already guards this with `|| undefined` — the same guard is needed here.

```suggestion
                              <Tooltip
                                content={
                                  isMe ? (
                                    commit.reason ? (
                                      <Flex direction="column" gap="1">
                                        <Text as="div" size="1" weight="bold">
                                          Why was I assigned?
                                        </Text>
                                        <Text as="div" size="1">
                                          {commit.reason}
                                        </Text>
                                      </Flex>
                                    ) : undefined
                                  ) : (
                                    commit.reason || undefined
                                  )
                                }
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Member Author

@rafaeelaudibert rafaeelaudibert May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be inverse and just check for commit.reason before, right

>
<span className="inline-flex items-center gap-0.5">
<a
href={commit.url}
target="_blank"
rel="noreferrer"
className="font-mono text-gray-9 hover:text-gray-11"
>
{commit.sha.slice(0, 7)}
</a>
{isMe && commit.reason && (
<InfoIcon
size={11}
className="cursor-help text-gray-9"
/>
)}
</span>
</Tooltip>
</span>
))}
Expand Down
Loading