Default to Identity tab and show contributors’ rating count in tooltip#1981
Merged
Default to Identity tab and show contributors’ rating count in tooltip#1981
Conversation
Signed-off-by: ragnep <ragneinfo@gmail.com>
📝 WalkthroughWalkthroughConsolidates the user identity subpath into the root profile path ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
Signed-off-by: ragnep <ragneinfo@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/common/OverlappingAvatars.tsx (1)
18-120:⚠️ Potential issue | 🟠 MajorTooltip won’t render when only
tooltipContentis provided.
showTooltip(Line 79) still checks onlyitem.title, sotooltipContentis ignored for visibility. If callers passtooltipContentwithouttitle, the tooltip never shows. Consider deriving visibility from the same value you render.✅ Suggested fix
- const showTooltip = - !isTouchDevice && item.title !== undefined && item.title !== ""; + const tooltipBody = item.tooltipContent ?? item.title; + const showTooltip = + !isTouchDevice && tooltipBody !== undefined && tooltipBody !== ""; const tooltipId = showTooltip ? `${baseId}-${index}` : undefined; @@ - {item.tooltipContent ?? item.title} + {tooltipBody}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/common/OverlappingAvatars.tsx` around lines 18 - 120, The tooltip visibility logic currently uses the local showTooltip variable (in OverlappingAvatars) which only checks item.title, so item.tooltipContent is ignored; change showTooltip to true when either item.tooltipContent or item.title is a non-empty value (e.g., item.tooltipContent !== undefined && item.tooltipContent !== "" || item.title !== undefined && item.title !== ""), keep using tooltipId (derived from baseId and index) and the same conditional that mounts <Tooltip id={tooltipId}> so tooltips render when callers provide tooltipContent only.
🧹 Nitpick comments (1)
app/[user]/identity/page.tsx (1)
1-31: UsepermanentRedirect()instead ofredirect()for the retired/identityroute.
redirect()returns a 307 (Temporary) HTTP redirect; for a 308 (Permanent), usepermanentRedirectinstead. Since/[user]/identityis being permanently retired and consolidated into/[user], a 307 misleads crawlers into re-fetching the old URL on every visit and prevents browsers from caching the redirect.permanentRedirectallows you to redirect the user to another URL and can be used in Server Components, Client Components, Route Handlers, and Server Functions.♻️ Proposed fix
-import { redirect } from "next/navigation"; +import { permanentRedirect } from "next/navigation"; // ... (rest of file unchanged) - redirect(`/${user}${qs ? `?${qs}` : ""}`); + permanentRedirect(`/${user}${qs ? `?${qs}` : ""}`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/`[user]/identity/page.tsx around lines 1 - 31, The IdentityRedirectPage currently calls redirect() which issues a 307; change to use permanentRedirect() to emit a 308 permanent redirect. Update the import from next/navigation to import permanentRedirect instead of redirect, and replace the redirect(`/${user}${qs ? `?${qs}` : ""}`) call in the IdentityRedirectPage function with permanentRedirect(...) while keeping the same URL construction logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/common/OverlappingAvatars.tsx`:
- Line 18: The prop type tooltipContent uses React.ReactNode without importing
React; update OverlappingAvatars.tsx to import the type and use it directly by
adding import type { ReactNode } from "react" and change the prop declaration to
tooltipContent?: ReactNode so TypeScript resolves the type without needing the
React namespace.
---
Outside diff comments:
In `@components/common/OverlappingAvatars.tsx`:
- Around line 18-120: The tooltip visibility logic currently uses the local
showTooltip variable (in OverlappingAvatars) which only checks item.title, so
item.tooltipContent is ignored; change showTooltip to true when either
item.tooltipContent or item.title is a non-empty value (e.g.,
item.tooltipContent !== undefined && item.tooltipContent !== "" || item.title
!== undefined && item.title !== ""), keep using tooltipId (derived from baseId
and index) and the same conditional that mounts <Tooltip id={tooltipId}> so
tooltips render when callers provide tooltipContent only.
---
Nitpick comments:
In `@app/`[user]/identity/page.tsx:
- Around line 1-31: The IdentityRedirectPage currently calls redirect() which
issues a 307; change to use permanentRedirect() to emit a 308 permanent
redirect. Update the import from next/navigation to import permanentRedirect
instead of redirect, and replace the redirect(`/${user}${qs ? `?${qs}` : ""}`)
call in the IdentityRedirectPage function with permanentRedirect(...) while
keeping the same URL construction logic.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
__tests__/components/user/brain/UserPageBrainWrapper.test.tsx__tests__/components/user/layout/UserPageTabs.test.tsx__tests__/components/user/user-page-header/stats/UserPageHeaderStats.test.tsx__tests__/components/user/utils/raters-table/ProfileRatersTableItem.test.tsx__tests__/components/user/utils/set-up-profile/UserSetUpProfileCta.test.tsx__tests__/pages/userPageRep.test.tsxapp/[user]/brain/page.tsxapp/[user]/identity/page.tsxapp/[user]/page.tsxcomponents/brain/notifications/identity-rating/NotificationIdentityRating.tsxcomponents/common/OverlappingAvatars.tsxcomponents/react-query-wrapper/ReactQueryWrapper.tsxcomponents/user/brain/UserPageBrainWrapper.tsxcomponents/user/layout/userTabs.config.tscomponents/user/rep/header/TopRaterAvatars.tsxcomponents/user/rep/new-rep/UserPageRepNewRepSearch.tsxcomponents/user/user-page-header/name/UserPageHeaderName.tsxcomponents/user/utils/set-up-profile/UserSetUpProfileCta.tsxcomponents/user/utils/stats/UserStatsRow.tsx
Signed-off-by: ragnep <ragneinfo@gmail.com>
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/common/OverlappingAvatars.tsx (1)
79-81:⚠️ Potential issue | 🟠 Major
showTooltipgate must also check fortooltipContentThe
showTooltipcondition (lines 79–80) gates the tooltip solely onitem.title, but the interface allowstooltipContentto be provided independently. Any caller that suppliestooltipContentwithout atitlewill see the tooltip silently omitted — the<Tooltip>is never mounted (line 110) and line 120's fallback is unreachable. This violates the API contract suggested by the optional signature of both fields.
TopRaterAvatarsmitigates this by always providing bothtitleandtooltipContenttogether (or neither), so the issue is latent here. However, the component should handletooltipContentas a first-class gate to prevent future bugs.🐛 Proposed fix
- const showTooltip = - !isTouchDevice && item.title !== undefined && item.title !== ""; + const hasTooltipContent = + item.tooltipContent !== undefined || + (item.title !== undefined && item.title !== ""); + const showTooltip = !isTouchDevice && hasTooltipContent;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/common/OverlappingAvatars.tsx` around lines 79 - 81, The showTooltip gating currently only checks item.title and thus prevents mounting <Tooltip> when item.tooltipContent is supplied alone; update the logic that defines showTooltip (and tooltipId usage) in OverlappingAvatars (the lines that compute showTooltip and tooltipId) to consider tooltipContent as an alternative: set showTooltip = !isTouchDevice && ((item.title !== undefined && item.title !== "") || (item.tooltipContent !== undefined && item.tooltipContent !== "")), and keep tooltipId conditional on that same showTooltip so the <Tooltip> component and the fallback path are reachable when only tooltipContent is provided.
♻️ Duplicate comments (1)
components/common/OverlappingAvatars.tsx (1)
7-7: Import fix and interface field LGTM — resolves the previously flagged missingReactNodeimport.Also applies to: 18-18
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/common/OverlappingAvatars.tsx` at line 7, The import statement in OverlappingAvatars.tsx must include ReactNode so the component's interface types compile; update the existing import line (import type { MouseEvent, ReactNode } from "react";) or add ReactNode to it and ensure any interface/type declarations that reference ReactNode (e.g., the props interface for OverlappingAvatars) use that imported symbol consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/`[user]/identity/page.tsx:
- Around line 24-30: The redirect uses an unencoded path segment `user`, which
can break URLs; update the code that builds `destination` (where `user`,
`resolvedParams`, `resolvedSearchParams`, `buildQueryString`, and
`permanentRedirect` are used) to URL-encode the path segment (e.g., compute an
`encodedUser = encodeURIComponent(user)` and use `encodedUser` when composing
`destination`) so the redirect always produces a valid path; keep the
querystring logic with `buildQueryString(resolvedSearchParams)` unchanged and
pass the resulting `destination` to `permanentRedirect`.
---
Outside diff comments:
In `@components/common/OverlappingAvatars.tsx`:
- Around line 79-81: The showTooltip gating currently only checks item.title and
thus prevents mounting <Tooltip> when item.tooltipContent is supplied alone;
update the logic that defines showTooltip (and tooltipId usage) in
OverlappingAvatars (the lines that compute showTooltip and tooltipId) to
consider tooltipContent as an alternative: set showTooltip = !isTouchDevice &&
((item.title !== undefined && item.title !== "") || (item.tooltipContent !==
undefined && item.tooltipContent !== "")), and keep tooltipId conditional on
that same showTooltip so the <Tooltip> component and the fallback path are
reachable when only tooltipContent is provided.
---
Duplicate comments:
In `@components/common/OverlappingAvatars.tsx`:
- Line 7: The import statement in OverlappingAvatars.tsx must include ReactNode
so the component's interface types compile; update the existing import line
(import type { MouseEvent, ReactNode } from "react";) or add ReactNode to it and
ensure any interface/type declarations that reference ReactNode (e.g., the props
interface for OverlappingAvatars) use that imported symbol consistently.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/[user]/identity/page.tsxcomponents/common/OverlappingAvatars.tsxcomponents/user/layout/userTabs.config.ts
analyticsflowee
approved these changes
Feb 24, 2026
This was referenced Feb 26, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Refactor
Style