Conversation
Signed-off-by: ragnep <ragneinfo@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRefactors mobile dialog internals and introduces a new RepContributorsDialog with infinite-scroll contributor lists. Multiple rep-related components gain callbacks and UI changes to open the contributor dialog; state and handlers are wired in UserPageRep to control dialog lifecycle and fetching. Changes
Sequence DiagramsequenceDiagram
participant User
participant MobileUI as MobileTabCards/MobileRepTabContent
participant State as UserPageRep\n(State)
participant Dialog as RepContributorsDialog
participant API as API\n(contributors)
User->>MobileUI: tap "View all contributors"
MobileUI->>State: onOpenOverviewContributors()/onOpenCategoryContributors(category)
State->>State: set contributorsDialog (scope, category, count)
State->>Dialog: render isOpen=true
Dialog->>API: fetch initial page (scope, direction, category?)
API-->>Dialog: return contributors page
Dialog->>User: render list
User->>Dialog: scroll to bottom
Dialog->>API: fetchNextPage()
API-->>Dialog: return next page
Dialog->>Dialog: append items
User->>Dialog: click close
Dialog->>State: onClose()
State->>State: clear contributorsDialog
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
components/mobile-wrapper-dialog/MobileWrapperDialog.tsx (2)
49-75: Consider guarding against empty header rendering.When both
titleis undefined andshowDesktopCloseButtonis false, the component renders an empty wrapperdiv. While harmless, you could avoid the extra DOM node:♻️ Optional improvement to skip empty wrapper
function DialogHeader({ title, showDesktopCloseButton, onClose, }: { readonly title: string | undefined; readonly showDesktopCloseButton: boolean; readonly onClose: () => void; }) { + if (!title && !showDesktopCloseButton) { + return null; + } return ( <div className="tw-px-4 sm:tw-px-6">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/mobile-wrapper-dialog/MobileWrapperDialog.tsx` around lines 49 - 75, The DialogHeader currently always renders a wrapper div even when both title and showDesktopCloseButton are absent; update the DialogHeader function to early-return null (or skip rendering the outer wrapper) when both props are falsy so no empty DOM node is output. Locate the DialogHeader component and use the title and showDesktopCloseButton props to guard rendering before producing the outer <div>, leaving onClose, DialogTitle and DialogCloseButton behavior unchanged when one or both are present.
160-160: Optional: The!!is redundant.Since
&&already performs boolean coercion andshowDesktopHeaderCloseButtonis used in a boolean context, you can simplify to:- const showDesktopHeaderCloseButton = dismissible && !!tabletModal; + const showDesktopHeaderCloseButton = dismissible && tabletModal;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/mobile-wrapper-dialog/MobileWrapperDialog.tsx` at line 160, The declaration of showDesktopHeaderCloseButton uses redundant boolean coercion (!!tabletModal); simplify the expression to rely on logical && coercion by assigning showDesktopHeaderCloseButton = dismissible && tabletModal (or otherwise remove the double-bang) so the value is still correct where used; update the reference in MobileWrapperDialog.tsx around the showDesktopHeaderCloseButton variable (and any related conditional checks) accordingly.components/user/rep/header/UserPageRepHeader.tsx (1)
133-157: Consider extracting the overview-contributor trigger.The same count formatting, label derivation,
aria-label, and button/span split now lives here, incomponents/user/rep/MobileRepTabContent.tsx, and incomponents/user/rep/MobileTabCards.tsx. A tiny shared component/helper would make copy and accessibility tweaks much harder to drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/user/rep/header/UserPageRepHeader.tsx` around lines 133 - 157, Extract the repeated contributor-count UI into a small shared component (e.g., ContributorsCountTrigger) that takes props: count (overview.contributor_count), repDirection, onOpen (onOpenOverviewContributors), and any className overrides; inside the component use formatNumberWithCommas(count) and getContributorLabel(repDirection, count) to build the visible text and aria-label, and render a <button> when count > 0 that calls onOpen or a <span> when zero, preserving the current class names and accessibility attributes. Replace the duplicated block in UserPageRepHeader (the conditional button/span), and update components/user/rep/MobileRepTabContent.tsx and components/user/rep/MobileTabCards.tsx to import and use ContributorsCountTrigger so formatting, labels, and aria handling are centralized. Ensure the new component exports named props and types so callers pass repDirection and count 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 `@components/user/rep/MobileTabCards.tsx`:
- Around line 119-132: The selected REP tab is rendered as a non-focusable div;
always render a button so the active control remains focusable: in
MobileTabCards replace the conditional that renders a div when active with a
button element that uses the same repCardClasses and repCardContent, set
aria-pressed={activeTab === "rep"} and aria-current={activeTab === "rep"}, and
only call onTabChange("rep") from the button's onClick when activeTab !== "rep"
(or guard the handler inside the click to no-op when already selected) so visual
selection is preserved while keeping the control keyboard-focusable; reference
the activeTab, repCardClasses, repCardContent, and onTabChange symbols when
making the change.
In `@components/user/rep/RepContributorsDialog.tsx`:
- Around line 94-99: Trim and normalize the contributor handle before building
display and route values: replace uses of contributor.handle with the
already-trimmed handle variable (e.g., use handle for display and routeValue)
and change the fallback logic from the nullish-coalescing operator (??) to a
check that treats empty strings as missing (e.g., use fallbackWallet =
contributor.wallets[0] || contributor.handle and routeValue = handle.length > 0
? handle : fallbackWallet) so whitespace-only handles fall back to the wallet;
apply the same fixes in the other mapping block (mapOverviewContributor /
mapCategoryContributor sections).
---
Nitpick comments:
In `@components/mobile-wrapper-dialog/MobileWrapperDialog.tsx`:
- Around line 49-75: The DialogHeader currently always renders a wrapper div
even when both title and showDesktopCloseButton are absent; update the
DialogHeader function to early-return null (or skip rendering the outer wrapper)
when both props are falsy so no empty DOM node is output. Locate the
DialogHeader component and use the title and showDesktopCloseButton props to
guard rendering before producing the outer <div>, leaving onClose, DialogTitle
and DialogCloseButton behavior unchanged when one or both are present.
- Line 160: The declaration of showDesktopHeaderCloseButton uses redundant
boolean coercion (!!tabletModal); simplify the expression to rely on logical &&
coercion by assigning showDesktopHeaderCloseButton = dismissible && tabletModal
(or otherwise remove the double-bang) so the value is still correct where used;
update the reference in MobileWrapperDialog.tsx around the
showDesktopHeaderCloseButton variable (and any related conditional checks)
accordingly.
In `@components/user/rep/header/UserPageRepHeader.tsx`:
- Around line 133-157: Extract the repeated contributor-count UI into a small
shared component (e.g., ContributorsCountTrigger) that takes props: count
(overview.contributor_count), repDirection, onOpen (onOpenOverviewContributors),
and any className overrides; inside the component use
formatNumberWithCommas(count) and getContributorLabel(repDirection, count) to
build the visible text and aria-label, and render a <button> when count > 0 that
calls onOpen or a <span> when zero, preserving the current class names and
accessibility attributes. Replace the duplicated block in UserPageRepHeader (the
conditional button/span), and update components/user/rep/MobileRepTabContent.tsx
and components/user/rep/MobileTabCards.tsx to import and use
ContributorsCountTrigger so formatting, labels, and aria handling are
centralized. Ensure the new component exports named props and types so callers
pass repDirection and count consistently.
🪄 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: 342436bc-91a1-4636-b4ba-915ba895bb6a
📒 Files selected for processing (8)
components/mobile-wrapper-dialog/MobileWrapperDialog.tsxcomponents/user/rep/MobileRepTabContent.tsxcomponents/user/rep/MobileTabCards.tsxcomponents/user/rep/RepCategoryPill.tsxcomponents/user/rep/RepContributorsDialog.tsxcomponents/user/rep/UserPageRep.tsxcomponents/user/rep/UserPageRepMobile.tsxcomponents/user/rep/header/UserPageRepHeader.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/user/rep/RepContributorsDialog.tsx (1)
94-99:⚠️ Potential issue | 🟡 MinorHandle normalization is still inconsistent in display/route construction.
Line 94 trims into
handle, but Lines 97-98 still usecontributor.handle.
Line 113 can produce"", and Lines 114/118 use??, so empty handles still bypass wallet fallback.Suggested fix
function mapOverviewContributor( contributor: ApiRatingWithProfileInfoAndLevelWithPfp ): RepContributorListItem { const handle = contributor.handle.trim(); - const fallbackWallet = contributor.wallets[0] ?? contributor.handle; - const display = - handle.length > 0 ? contributor.handle : formatAddress(fallbackWallet); - const routeValue = handle.length > 0 ? contributor.handle : fallbackWallet; + const fallbackWallet = contributor.wallets[0] || contributor.handle; + const routeValue = handle.length > 0 ? handle : fallbackWallet; + const display = + handle.length > 0 ? handle : formatAddress(fallbackWallet); return { key: contributor.profile_id, @@ function mapCategoryContributor( contributor: ApiRepContributor ): RepContributorListItem { const fallbackWallet = contributor.profile.primary_address; - const handle = contributor.profile.handle?.trim() ?? null; - const display = handle ?? formatAddress(fallbackWallet); + const handle = contributor.profile.handle?.trim() || null; + const routeValue = handle ?? fallbackWallet; + const display = handle ?? formatAddress(fallbackWallet); return { key: contributor.profile.id, - href: getRouteHref(handle ?? fallbackWallet), + href: getRouteHref(routeValue), display, pfpUrl: contributor.profile.pfp ?? null, contribution: contributor.contribution,Also applies to: 112-119
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/user/rep/RepContributorsDialog.tsx` around lines 94 - 99, The trimmed handle variable is computed as handle but the later display and routeValue still reference contributor.handle (and use ?? which treats empty string as present), causing empty handles to bypass wallet fallback; update display and routeValue to use the trimmed handle variable and a proper empty-string check (e.g., handle.length > 0) so fallbackWallet (computed from contributor.wallets[0] or contributor.handle) is only used when handle is empty, e.g., set display = handle.length > 0 ? handle : formatAddress(fallbackWallet) and routeValue = handle.length > 0 ? handle : fallbackWallet; ensure all references to contributor.handle in this block are replaced with the normalized handle.
🤖 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/user/rep/RepContributorsDialog.tsx`:
- Around line 236-250: The query gating uses normalizedCategory but the current
normalization (normalizedCategory = category?.trim() ?? null) leaves empty
strings as "" which causes fetchRepContributorPage to throw; update the
normalization so empty or all-whitespace category values become null (e.g., set
normalizedCategory to null when category?.trim() === ""), then use that
normalizedCategory in the useInfiniteQuery queryKey, queryFn, and the enabled
condition (isOpen && (scope === "overview" || normalizedCategory !== null)) so
the query is properly gated; references: normalizedCategory, useInfiniteQuery,
fetchRepContributorPage, isOpen, scope, category.
---
Duplicate comments:
In `@components/user/rep/RepContributorsDialog.tsx`:
- Around line 94-99: The trimmed handle variable is computed as handle but the
later display and routeValue still reference contributor.handle (and use ??
which treats empty string as present), causing empty handles to bypass wallet
fallback; update display and routeValue to use the trimmed handle variable and a
proper empty-string check (e.g., handle.length > 0) so fallbackWallet (computed
from contributor.wallets[0] or contributor.handle) is only used when handle is
empty, e.g., set display = handle.length > 0 ? handle :
formatAddress(fallbackWallet) and routeValue = handle.length > 0 ? handle :
fallbackWallet; ensure all references to contributor.handle in this block are
replaced with the normalized handle.
🪄 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: 5b46e77c-9c7e-4bdb-9dd4-5a89306dd958
📒 Files selected for processing (1)
components/user/rep/RepContributorsDialog.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/user/rep/RepCategoryPill.tsx`:
- Around line 91-109: The hover/edit styling is applied to the outer div while
the actual click handler is on the inner button (RepCategoryPill, canEdit,
onEdit, baseClasses, detailClasses), so move the edit hover classes from the
outer wrapper into the clickable element or make the button fill the wrapper;
specifically, when canEdit is true remove the hover classes from the outer div
and add them to the button (and give the button full-size utility classes such
as tw-w-full tw-h-full and retain tw-cursor-pointer tw-border-none
tw-bg-transparent) so the visible hover state matches the real hit area; when
canEdit is false keep the outer div with tw-cursor-default and render a
non-interactive div with detailClasses.
🪄 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: fbe534ed-c46e-4ec9-8b55-87b6d0d33a0c
📒 Files selected for processing (6)
components/user/rep/MobileRepTabContent.tsxcomponents/user/rep/MobileTabCards.tsxcomponents/user/rep/RepCategoryPill.tsxcomponents/user/rep/RepContributorsDialog.tsxcomponents/user/rep/UserPageRepMobile.tsxcomponents/user/rep/header/UserPageRepHeader.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- components/user/rep/UserPageRepMobile.tsx
Signed-off-by: ragnep <ragneinfo@gmail.com>
|
simo6529
approved these changes
Apr 7, 2026
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
UX