Skip to content

Highlight current user in signal suggested reviewers#2435

Open
rafaeelaudibert wants to merge 2 commits into
mainfrom
posthog-code/inbox-suggested-reviewer-me-tooltip
Open

Highlight current user in signal suggested reviewers#2435
rafaeelaudibert wants to merge 2 commits into
mainfrom
posthog-code/inbox-suggested-reviewer-me-tooltip

Conversation

@rafaeelaudibert
Copy link
Copy Markdown
Member

Problem

When a signal report assigns suggested reviewers, it can be hard to tell at a glance whether you are on the list, and why. The commit SHA-only tooltip explaining the rationale isn't obviously discoverable — you have to know to hover.

Changes

In the Suggested reviewers section of the report detail pane:

  • Sort the list so the current user appears first when present.
  • For the current user, render a small info icon next to each commit SHA. The icon is the tooltip trigger, and the tooltip now has a "Why was I assigned?" title above the existing rationale, making the explanation discoverable.
  • Other reviewers keep the existing behaviour (rationale tooltip on the SHA itself).

How did you test this?

  • pnpm --filter code typecheck (web tsconfig)
  • pnpm exec biome check on the edited file

Did not run the desktop app in this environment.

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Created with PostHog Code

Sorts the current user to the top of the suggested reviewers list, and when a commit explains why the current user was picked, shows a small info icon next to the SHA with a "Why was I assigned?" tooltip that surfaces the rationale. Other reviewers keep the existing hover-on-SHA tooltip.

Generated-By: PostHog Code
Task-Id: bf5f3c64-2f8b-4ca2-8716-b91f7527b080
Hovering either the commit link or the new info icon now opens the same tooltip, collapsing the per-isMe branching into one Tooltip with conditional content and a conditional trailing icon.

Generated-By: PostHog Code
Task-Id: bf5f3c64-2f8b-4ca2-8716-b91f7527b080
@rafaeelaudibert rafaeelaudibert requested a review from a team May 29, 2026 20:46
@rafaeelaudibert rafaeelaudibert marked this pull request as ready for review May 29, 2026 20:46
@rafaeelaudibert rafaeelaudibert enabled auto-merge (squash) May 29, 2026 20:46
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 29, 2026

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
apps/code/src/renderer/features/inbox/components/detail/ReportDetailPane.tsx:826-840
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
                                  )
                                }
```

Reviews (1): Last reviewed commit: "Wrap commit SHA and info icon in a singl..." | Re-trigger Greptile

Comment on lines +826 to +840
<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
)
}
Copy link
Copy Markdown
Contributor

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant