Overhaul user detail page UX - #1046
Open
goldflag wants to merge 1 commit into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThe user profile page is reorganized around reusable summary, behavior, session, sidebar, journey, action, map, and localization components. New loading and error states, theme-aware visuals, journey navigation, session pagination behavior, and updated message catalogs are included. ChangesUser profile experience
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant UserPage
participant UserQueries
participant UserSummary
participant UserTopPages
participant UserSidebar
UserPage->>UserQueries: fetch user and session data
UserQueries-->>UserPage: return data and request states
UserPage->>UserSummary: render identity and metrics
UserPage->>UserTopPages: render pages, events, and journeys
UserPage->>UserSidebar: render visitor context and activity history
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
client/src/app/[site]/user/[userId]/page.tsx (1)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the prescribed translation hook.
The new components initialize translations with
useExtracted()instead ofuseTranslations().As per coding guidelines, “Use
useTranslations()from next-intl for user-facing strings.”Also applies to: 42-43, 74-76
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/app/`[site]/user/[userId]/page.tsx at line 4, Replace useExtracted with useTranslations from next-intl in the affected page components, and update each translation initialization to call useTranslations(). Preserve useLocale and all existing translation keys and user-facing text.Source: Coding guidelines
client/src/app/[site]/user/[userId]/components/UserSummary.tsx (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
useTranslations()in this new component.As per coding guidelines, “Use
useTranslations()from next-intl for user-facing strings.”Also applies to: 62-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/app/`[site]/user/[userId]/components/UserSummary.tsx at line 5, Update the new UserSummary component to use next-intl’s useTranslations() hook instead of useExtracted/useLocale for its user-facing strings. Replace the affected string usage around the component’s referenced lines while preserving the existing rendered content and translation namespace conventions.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/src/app/`[site]/user/[userId]/components/UserJourneys.tsx:
- Line 23: Update UserJourneys’ useJourneys destructuring and diagram rendering
to account for placeholder or in-flight data: use isPlaceholderData or
isFetching to hide or dim the existing journey map while step/time changes are
fetching fresh results, then restore normal rendering once current data arrives.
In `@client/src/app/`[site]/user/[userId]/components/UserLocationMap.tsx:
- Around line 25-29: Update the isDark calculation in UserLocationMap to treat
every resolvedTheme value other than "light", including the initial undefined
state, as dark. Keep the existing Mapbox style selection and marker behavior
driven by isDark.
In `@client/src/app/`[site]/user/[userId]/components/UserSidebar.tsx:
- Line 10: Remove the Favicon import and its usage in UserSidebar so loading a
user profile does not request the referrer domain from DuckDuckGo. Do not
replace it with another direct third-party favicon request; use an existing
controlled cache/proxy only if the component already provides one.
- Around line 181-185: Update the map rendering in UserSidebar so a failed or
unavailable UserLocationMap does not leave the fixed-height bordered wrapper
visible. Move the wrapper ownership into UserLocationMap or conditionally render
the wrapper only when the map produces valid content, while preserving the
existing Mapbox, country, region, and city conditions.
In `@client/src/app/`[site]/user/[userId]/components/UserSummary.tsx:
- Around line 71-72: Update the UserSummary local-time logic around localTime to
refresh once per minute by adding timer-driven state or an equivalent rerender
mechanism, and clear the timer during component unmount cleanup. Preserve the
existing timezone, locale, and validity handling while ensuring the displayed
badge updates without unrelated state changes.
In `@client/src/app/`[site]/user/[userId]/page.tsx:
- Around line 229-240: Update the SessionsList invocation in the user page to
set pageSize to 25, matching the number of session cards loaded and the skeleton
placeholders rendered by the component.
---
Nitpick comments:
In `@client/src/app/`[site]/user/[userId]/components/UserSummary.tsx:
- Line 5: Update the new UserSummary component to use next-intl’s
useTranslations() hook instead of useExtracted/useLocale for its user-facing
strings. Replace the affected string usage around the component’s referenced
lines while preserving the existing rendered content and translation namespace
conventions.
In `@client/src/app/`[site]/user/[userId]/page.tsx:
- Line 4: Replace useExtracted with useTranslations from next-intl in the
affected page components, and update each translation initialization to call
useTranslations(). Preserve useLocale and all existing translation keys and
user-facing text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 72631f84-4dd4-4792-97a0-750a563a7988
📒 Files selected for processing (23)
client/messages/cs.jsonclient/messages/de.jsonclient/messages/en.jsonclient/messages/es.jsonclient/messages/fr.jsonclient/messages/it.jsonclient/messages/ja.jsonclient/messages/ko.jsonclient/messages/pl.jsonclient/messages/pt.jsonclient/messages/uk.jsonclient/messages/zh.jsonclient/src/app/[site]/user/[userId]/components/Calendar.tsxclient/src/app/[site]/user/[userId]/components/LocationDevices.tsxclient/src/app/[site]/user/[userId]/components/SidebarPrimitives.tsxclient/src/app/[site]/user/[userId]/components/UserActions.tsxclient/src/app/[site]/user/[userId]/components/UserJourneys.tsxclient/src/app/[site]/user/[userId]/components/UserLocationMap.tsxclient/src/app/[site]/user/[userId]/components/UserSidebar.tsxclient/src/app/[site]/user/[userId]/components/UserSummary.tsxclient/src/app/[site]/user/[userId]/components/UserTopPages.tsxclient/src/app/[site]/user/[userId]/page.tsxclient/src/components/IdentifiedBadge.tsx
| const { time } = useStore(); | ||
|
|
||
| const { data, isLoading, error } = useJourneys({ | ||
| const { data, isLoading, error, refetch } = useJourneys({ |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the declared React Query version and the query's placeholder-data contract.
fd package.json client --exec sh -c \
'printf "\n%s\n" "$1"; jq -r ".dependencies[\"`@tanstack/react-query`\"] // .devDependencies[\"`@tanstack/react-query`\"] // empty" "$1"' _ {}
ast-grep outline client/src/api/analytics/hooks/useGetJourneys.ts \
--match useJourneys --view expanded
sed -n '20,38p' client/src/api/analytics/hooks/useGetJourneys.tsRepository: rybbit-io/rybbit
Length of output: 1115
Treat placeholder journeys as stale state
useJourneys() keeps the previous result via placeholderData, so isLoading stays false during step/time changes and the old journey map can remain visible under the new controls. Use isPlaceholderData or isFetching to hide or dim the diagram until the fresh result arrives.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/app/`[site]/user/[userId]/components/UserJourneys.tsx at line 23,
Update UserJourneys’ useJourneys destructuring and diagram rendering to account
for placeholder or in-flight data: use isPlaceholderData or isFetching to hide
or dim the existing journey map while step/time changes are fetching fresh
results, then restore normal rendering once current data arrives.
Comment on lines
+25
to
+29
| const isDark = resolvedTheme === "dark"; | ||
|
|
||
| const query = [city, region, country].filter(Boolean).join(", "); | ||
|
|
||
| const style = resolvedTheme === "dark" ? "mapbox://styles/mapbox/dark-v11" : "mapbox://styles/mapbox/light-v11"; | ||
| const style = isDark ? "mapbox://styles/mapbox/dark-v11" : "mapbox://styles/mapbox/light-v11"; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n -C3 'ThemeProvider|defaultTheme|resolvedTheme' client/src -g '*.tsx'Repository: rybbit-io/rybbit
Length of output: 15498
🏁 Script executed:
sed -n '1,220p' client/src/app/[site]/user/[userId]/components/UserLocationMap.tsxRepository: rybbit-io/rybbit
Length of output: 3742
🏁 Script executed:
sed -n '1,140p' client/src/app/Providers.tsxRepository: rybbit-io/rybbit
Length of output: 3351
🏁 Script executed:
sed -n '1,120p' client/src/app/[site]/feature-flags/components/JsonEditor.tsxRepository: rybbit-io/rybbit
Length of output: 2066
🏁 Script executed:
sed -n '1,120p' client/src/app/[site]/user/[userId]/components/UserLocationMap.tsxRepository: rybbit-io/rybbit
Length of output: 3736
Keep the map dark until resolvedTheme settles.
resolvedTheme is undefined on the first render, so this picks the light Mapbox style and marker before switching to dark and recreating the map. Use resolvedTheme !== "light" here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/app/`[site]/user/[userId]/components/UserLocationMap.tsx around
lines 25 - 29, Update the isDark calculation in UserLocationMap to treat every
resolvedTheme value other than "light", including the initial undefined state,
as dark. Keep the existing Mapbox style selection and marker behavior driven by
isDark.
Source: Coding guidelines
| import { UserInfo, UserSessionCountResponse } from "@/api/analytics/endpoints"; | ||
| import { ChannelIcon, extractDomain, getDisplayName } from "@/components/Channel"; | ||
| import { EditTraitsDialog } from "@/components/EditTraitsDialog"; | ||
| import { Favicon } from "@/components/Favicon"; |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid disclosing referrer domains to DuckDuckGo.
Favicon requests icons.duckduckgo.com/ip3/${domain}.ico, exposing each visitor’s referrer domain to a third party when this profile loads. Remove the remote icon here or serve favicons through a controlled cache/proxy.
Proposed privacy-safe fix
-import { Favicon } from "`@/components/Favicon`";
...
<InfoRow
- icon={
- firstReferrerDomain ? <Favicon domain={firstReferrerDomain} className="h-3.5 w-3.5" /> : undefined
- }
label={t("Referrer")}
value={firstReferrerDomain ? getDisplayName(firstReferrerDomain) : "—"}
/>Also applies to: 106-112
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/app/`[site]/user/[userId]/components/UserSidebar.tsx at line 10,
Remove the Favicon import and its usage in UserSidebar so loading a user profile
does not request the referrer domain from DuckDuckGo. Do not replace it with
another direct third-party favicon request; use an existing controlled
cache/proxy only if the component already provides one.
Comment on lines
+181
to
185
| {configs?.mapboxToken && data?.country && ( | ||
| <div className="mt-3 h-36 overflow-hidden rounded-lg border border-neutral-100 dark:border-neutral-800"> | ||
| <UserLocationMap country={data.country} region={data.region} city={data.city} /> | ||
| </div> | ||
| )} |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not leave an empty map frame after geocoding fails.
UserLocationMap returns null for failed or unavailable coordinates, but this fixed-height wrapper remains visible. Let the map component own the wrapper, or hide/remove it when no map can render.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/app/`[site]/user/[userId]/components/UserSidebar.tsx around lines
181 - 185, Update the map rendering in UserSidebar so a failed or unavailable
UserLocationMap does not leave the fixed-height bordered wrapper visible. Move
the wrapper ownership into UserLocationMap or conditionally render the wrapper
only when the map produces valid content, while preserving the existing Mapbox,
country, region, and city conditions.
Comment on lines
+71
to
+72
| const localTime = data?.timezone ? DateTime.now().setZone(data.timezone).setLocale(locale) : null; | ||
| const localTimeValid = localTime?.isValid ? localTime : null; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the component and nearby code.
FILE="client/src/app/[site]/user/[userId]/components/UserSummary.tsx"
wc -l "$FILE"
sed -n '1,220p' "$FILE"
# Look for any existing timers/refresh logic in the component or related user summary files.
rg -n "setInterval|setTimeout|DateTime\.now\(\)|localTime|timezone|clock|refresh" client/src/app/[site]/user/[userId]/components -SRepository: rybbit-io/rybbit
Length of output: 8793
Refresh the displayed local time. DateTime.now() is only re-evaluated on render, so this badge can drift out of date until some other state change causes a rerender. Add a minute timer and clear it on unmount.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/app/`[site]/user/[userId]/components/UserSummary.tsx around lines
71 - 72, Update the UserSummary local-time logic around localTime to refresh
once per minute by adding timer-driven state or an equivalent rerender
mechanism, and clear the timer during component unmount cleanup. Preserve the
existing timezone, locale, and validity handling while ensuring the displayed
badge updates without unrelated state changes.
Comment on lines
+229
to
+240
| <SessionsList | ||
| sessions={sessions} | ||
| isLoading={isLoadingSessions} | ||
| page={page} | ||
| onPageChange={handlePageChange} | ||
| hasNextPage={hasNextPage} | ||
| hasPrevPage={hasPrevPage} | ||
| userId={userId} | ||
| headerElement={sessionHeading} | ||
| emptyMessage={t("No sessions match this date range or the active filters.")} | ||
| pageSize={8} | ||
| /> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Match the skeleton count to the session page size.
Line 239 renders 8 placeholders before replacing them with up to 25 cards, causing avoidable layout shift.
Proposed fix
- pageSize={8}
+ pageSize={LIMIT}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <SessionsList | |
| sessions={sessions} | |
| isLoading={isLoadingSessions} | |
| page={page} | |
| onPageChange={handlePageChange} | |
| hasNextPage={hasNextPage} | |
| hasPrevPage={hasPrevPage} | |
| userId={userId} | |
| headerElement={sessionHeading} | |
| emptyMessage={t("No sessions match this date range or the active filters.")} | |
| pageSize={8} | |
| /> | |
| <SessionsList | |
| sessions={sessions} | |
| isLoading={isLoadingSessions} | |
| page={page} | |
| onPageChange={handlePageChange} | |
| hasNextPage={hasNextPage} | |
| hasPrevPage={hasPrevPage} | |
| userId={userId} | |
| headerElement={sessionHeading} | |
| emptyMessage={t("No sessions match this date range or the active filters.")} | |
| pageSize={LIMIT} | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/app/`[site]/user/[userId]/page.tsx around lines 229 - 240, Update
the SessionsList invocation in the user page to set pageSize to 25, matching the
number of session cards loaded and the skeleton placeholders rendered by the
component.
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
1 participant
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
Bug Fixes