Removed followers tab and created followers modal#1989
Conversation
Signed-off-by: ragnep <ragneinfo@gmail.com>
📝 WalkthroughWalkthroughReplaces the dedicated followers page with a modal-based followers UI. Adds a useFollowersList hook, a UserPageFollowersModal component, and wiring in header and stats components; removes the followers tab route and makes the followers page redirect to the parent profile. Changes
Sequence DiagramsequenceDiagram
actor User
participant Stats as UserStatsRow
participant Header as UserPageHeaderStats
participant Modal as UserPageFollowersModal
participant Hook as useFollowersList
participant API as API Server
User->>Stats: Click followers count
Stats->>Header: invoke onFollowersClick()
Header->>Header: set isFollowersModalOpen = true
Header->>Modal: render isOpen=true
Modal->>Hook: initialize useFollowersList(profileId, enabled=true)
Hook->>API: GET /identity-subscriptions/incoming/IDENTITY/{profileId}
API-->>Hook: return page 1
Hook-->>Modal: followers, isFetching, onBottomIntersection
Modal->>User: render followers in MobileWrapperDialog
User->>Modal: scroll to bottom
Modal->>Hook: onBottomIntersection(true)
Hook->>API: GET ...?page=2
API-->>Hook: return page 2
Hook-->>Modal: append followers
User->>Modal: close modal
Modal->>Header: call onClose()
Header->>Header: set isFollowersModalOpen = false
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
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 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
hooks/useFollowersList.ts (1)
50-56: Consider usinguseMemoinstead ofuseEffect+useStatefor derived state.The
followersarray is derived directly fromdata. UsinguseMemois more idiomatic for computed values and avoids an extra render cycle.♻️ Proposed refactor
- const [followers, setFollowers] = useState< - ApiIdentityAndSubscriptionActions[] - >([]); - useEffect( - () => setFollowers(data?.pages.flatMap((page) => page.data) ?? []), - [data] - ); + const followers = useMemo( + () => data?.pages.flatMap((page) => page.data) ?? [], + [data] + );Update the import:
-import { useEffect, useState } from "react"; +import { useMemo } from "react";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useFollowersList.ts` around lines 50 - 56, Replace the derived state logic that uses useState/setFollowers and useEffect with a memoized computed value: remove the followers state and the useEffect that sets it from data, and instead create followers via useMemo(() => data?.pages.flatMap(page => page.data) ?? [], [data]) so the array is computed idiomatically; update any references to the followers variable to use this memo and keep the dependency on data only.components/mobile-wrapper-dialog/MobileWrapperDialog.tsx (1)
170-177: Title now scrolls with content — verify this is intentional.The
DialogTitleis rendered inside the scrollable container, meaning it will scroll out of view as the user scrolls down. For modals with long content (like a followers list), users may lose context of what they're viewing. Consider whether the title should remain fixed at the top of the modal.💡 Suggestion to keep title fixed
<div className={`tw-flex tw-scroll-py-3 tw-flex-col tw-overflow-y-auto tw-flex-1 tw-min-h-0 ${ noPadding ? "tw-py-0" : "tw-py-6" }${ showScrollbar ? " tw-scrollbar-thin tw-scrollbar-track-iron-800 tw-scrollbar-thumb-iron-500 desktop-hover:hover:tw-scrollbar-thumb-iron-300" : "" }`} style={{ paddingBottom: bottomPadding }} > - <div className="tw-px-4 sm:tw-px-6"> - {title && ( - <DialogTitle className="tw-text-base tw-font-semibold tw-text-iron-50"> - {title} - </DialogTitle> - )} - </div> {children} </div>Then render the title before the scrollable container:
+ {title && ( + <div className="tw-px-4 sm:tw-px-6 tw-py-4 tw-border-b tw-border-iron-800"> + <DialogTitle className="tw-text-base tw-font-semibold tw-text-iron-50"> + {title} + </DialogTitle> + </div> + )} <div className={`tw-flex tw-scroll-py-3 tw-flex-col tw-overflow-y-auto ...`}🤖 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 170 - 177, The DialogTitle is currently inside the scrollable area in MobileWrapperDialog.tsx so the title scrolls away with children; move the title out of the scrollable container so it stays fixed at the top of the modal: render the title (check the title prop and DialogTitle usage) above the element that wraps the scrollable children (the container that currently contains {children}), adjust layout classes as needed so the scrollable container only contains the list/content and not the DialogTitle, and ensure accessibility/visual spacing is preserved after the change.
🤖 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]/followers/page.tsx:
- Around line 8-10: The current redirect call uses resolvedParams/params to
compute user and then unconditionally calls redirect(`/${user}`), which will
send an empty user to the root; instead validate resolvedParams and the derived
user before redirecting: if resolvedParams is missing or user is falsy, call
notFound() (or otherwise handle the missing resource) and only call
redirect(`/${user}`) when user is a non-empty string; update the logic around
resolvedParams, params, user, redirect and import/usage of notFound accordingly
so empty/undefined users do not redirect to "/".
---
Nitpick comments:
In `@components/mobile-wrapper-dialog/MobileWrapperDialog.tsx`:
- Around line 170-177: The DialogTitle is currently inside the scrollable area
in MobileWrapperDialog.tsx so the title scrolls away with children; move the
title out of the scrollable container so it stays fixed at the top of the modal:
render the title (check the title prop and DialogTitle usage) above the element
that wraps the scrollable children (the container that currently contains
{children}), adjust layout classes as needed so the scrollable container only
contains the list/content and not the DialogTitle, and ensure
accessibility/visual spacing is preserved after the change.
In `@hooks/useFollowersList.ts`:
- Around line 50-56: Replace the derived state logic that uses
useState/setFollowers and useEffect with a memoized computed value: remove the
followers state and the useEffect that sets it from data, and instead create
followers via useMemo(() => data?.pages.flatMap(page => page.data) ?? [],
[data]) so the array is computed idiomatically; update any references to the
followers variable to use this memo and keep the dependency on data only.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/[user]/followers/page.tsxcomponents/header/AppSidebarUserStats.tsxcomponents/mobile-wrapper-dialog/MobileWrapperDialog.tsxcomponents/user/followers/UserPageFollowers.tsxcomponents/user/followers/UserPageFollowersModal.tsxcomponents/user/layout/userTabs.config.tscomponents/user/user-page-header/stats/UserPageHeaderStats.tsxcomponents/user/utils/stats/UserStatsRow.tsxcomponents/utils/followers/Follower.tsxhooks/useFollowersList.ts
💤 Files with no reviewable changes (1)
- components/user/layout/userTabs.config.ts
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
__tests__/components/header/AppSidebarUserStats.test.tsx (1)
47-47:⚠️ Potential issue | 🟡 MinorMissing required props in test renders.
Based on
AppSidebarUserStatscomponent signature (seecomponents/header/AppSidebarUserStats.tsx), the following required props are missing in all test renders:tdh_rate,xtdh,xtdh_rate, andcic. This will cause TypeScript errors and may result inundefinedvalues being passed toUserStatsRow.Proposed fix
- render(<AppSidebarUserStats handle="alice" tdh={1500} rep={2} profileId="p1" />); + render(<AppSidebarUserStats handle="alice" tdh={1500} tdh_rate={0} xtdh={0} xtdh_rate={0} rep={2} cic={0} profileId="p1" />);- render(<AppSidebarUserStats handle="bob" tdh={10} rep={20} profileId={undefined} />); + render(<AppSidebarUserStats handle="bob" tdh={10} tdh_rate={0} xtdh={0} xtdh_rate={0} rep={20} cic={0} profileId={undefined} />);- render(<AppSidebarUserStats handle="carol" tdh={1} rep={0} profileId="pid" />); + render(<AppSidebarUserStats handle="carol" tdh={1} tdh_rate={0} xtdh={0} xtdh_rate={0} rep={0} cic={0} profileId="pid" />);Also applies to: 67-67, 78-78
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/header/AppSidebarUserStats.test.tsx` at line 47, Tests render AppSidebarUserStats without required props (tdh_rate, xtdh, xtdh_rate, cic) causing TypeScript errors and undefined values passed to UserStatsRow; update each render call in the test file to pass those missing props (e.g., provide mock numeric/string values for tdh_rate, xtdh, xtdh_rate and a value for cic) so AppSidebarUserStats receives a complete props object; locate the render invocations of AppSidebarUserStats in the test (the calls at or around the lines invoking render(<AppSidebarUserStats ... />)) and add the four props with sensible test values to each call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@__tests__/components/header/AppSidebarUserStats.test.tsx`:
- Line 47: Tests render AppSidebarUserStats without required props (tdh_rate,
xtdh, xtdh_rate, cic) causing TypeScript errors and undefined values passed to
UserStatsRow; update each render call in the test file to pass those missing
props (e.g., provide mock numeric/string values for tdh_rate, xtdh, xtdh_rate
and a value for cic) so AppSidebarUserStats receives a complete props object;
locate the render invocations of AppSidebarUserStats in the test (the calls at
or around the lines invoking render(<AppSidebarUserStats ... />)) and add the
four props with sensible test values to each call.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
__tests__/components/header/AppSidebarUserStats.test.tsxapp/[user]/followers/page.tsxcomponents/user/utils/stats/UserStatsRow.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- app/[user]/followers/page.tsx



Summary by CodeRabbit
New Features
Changes