Dashboard stacked bar chart capturing membership across regions/chapter#6
Closed
ilydoa wants to merge 4 commits into
Closed
Dashboard stacked bar chart capturing membership across regions/chapter#6ilydoa wants to merge 4 commits into
ilydoa wants to merge 4 commits into
Conversation
…ers to Dashboard page
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughAdded a new MembershipTrendsGraph component, wired it into the dashboard page with a Firestore query for Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DashboardPage as Dashboard Page
participant Firestore as Firestore (useCollection)
participant Trends as MembershipTrendsGraph
participant Recharts as Recharts Renderer
User->>DashboardPage: open dashboard
DashboardPage->>Firestore: fetch "chapter_aliases"
Firestore-->>DashboardPage: return aliases
DashboardPage->>Trends: render with chapters + aliases
User->>Trends: change filter/view controls
Trends->>Trends: normalize labels & compute aggregates
Trends->>Recharts: supply chart data & config
Recharts-->>User: render interactive bar chart
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pnaa/components/dashboard/membership-trends-graph.tsx (1)
44-45: Generate region options from the data instead of a fixed union.
normalizedRegionpreserves any non-None/Unknownvalue coming from Firestore, but the state type and<SelectItem>list only admit five hard-coded labels. A renamed or newly added region will still appear in the “All Regions” aggregate and then be impossible to drill into at the chapter level.♻️ Possible refactor
-type FilterOption = "all" | "Eastern Region" | "Western Region" | "North Central Region" | "South Central Region" | "Other"; +type FilterOption = "all" | string; ... + const regionOptions = sortedRegionData.map((item) => item.region); ... - <SelectItem value="Eastern Region">East</SelectItem> - <SelectItem value="Western Region">West</SelectItem> - <SelectItem value="North Central Region">North</SelectItem> - <SelectItem value="South Central Region">South</SelectItem> - <SelectItem value="Other">Other</SelectItem> + {regionOptions.map((region) => ( + <SelectItem key={region} value={region}> + {region} + </SelectItem> + ))}Also applies to: 179-190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pnaa/components/dashboard/membership-trends-graph.tsx` around lines 44 - 45, The FilterOption union is too rigid: instead of the hard-coded type and SelectItem list, derive region options dynamically from the incoming data (use the same normalization logic used by normalizedRegion) and build the SelectItem array at render time (prepend an "All" option); change any state typing that referenced FilterOption to a simple string (or union with "all") so newly-named or added regions from Firestore can be selected, update the Select rendering code (where SelectItem is created and where selection is read — lines around membership-trends-graph.tsx usage and the block referenced at 179-190) to map over the computed uniqueRegions (filtering out None/Unknown) and keep normalizedRegion usage for grouping/aggregation.pnaa/app/(app)/dashboard/page.tsx (1)
71-71: Make the header counts data-driven.This copy hard-codes
65 chaptersand13,000+ members, even though the page already computes live totals from Firestore. Once the underlying data changes, the header will contradict the cards and chart unless these numbers come fromstatsor are removed from the sentence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pnaa/app/`(app)/dashboard/page.tsx at line 71, The header text currently hard-codes "65 chapters" and "13,000+ members"; update the copy in page.tsx to use the computed stats instead (e.g., replace the literal numbers with the live values from the `stats` object such as `stats.totalChapters` and `stats.totalMembers` or omit the numeric clause entirely). Locate the description string near the component that renders the hero/header and interpolate or format the live `stats` totals (ensure pluralization/formatting like thousands+ if needed) so the header remains consistent with the cards/chart that use `stats`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pnaa/components/dashboard/membership-trends-graph.tsx`:
- Around line 222-232: The Legend's wrapperStyle contains an invalid font size
string ("11px,") causing the style to be ignored; update the Legend declaration
in membership-trends-graph.tsx (the Legend component with the wrapperStyle prop)
to use a valid fontSize value—either remove the trailing comma to "11px" or use
the numeric 11—so the legend text renders at the intended size.
---
Nitpick comments:
In `@pnaa/app/`(app)/dashboard/page.tsx:
- Line 71: The header text currently hard-codes "65 chapters" and "13,000+
members"; update the copy in page.tsx to use the computed stats instead (e.g.,
replace the literal numbers with the live values from the `stats` object such as
`stats.totalChapters` and `stats.totalMembers` or omit the numeric clause
entirely). Locate the description string near the component that renders the
hero/header and interpolate or format the live `stats` totals (ensure
pluralization/formatting like thousands+ if needed) so the header remains
consistent with the cards/chart that use `stats`.
In `@pnaa/components/dashboard/membership-trends-graph.tsx`:
- Around line 44-45: The FilterOption union is too rigid: instead of the
hard-coded type and SelectItem list, derive region options dynamically from the
incoming data (use the same normalization logic used by normalizedRegion) and
build the SelectItem array at render time (prepend an "All" option); change any
state typing that referenced FilterOption to a simple string (or union with
"all") so newly-named or added regions from Firestore can be selected, update
the Select rendering code (where SelectItem is created and where selection is
read — lines around membership-trends-graph.tsx usage and the block referenced
at 179-190) to map over the computed uniqueRegions (filtering out None/Unknown)
and keep normalizedRegion usage for grouping/aggregation.
🪄 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: 6ae8efa4-f409-415c-904e-6b9087b5a171
⛔ Files ignored due to path filters (1)
pnaa/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
pnaa/app/(app)/dashboard/page.tsxpnaa/components/dashboard/membership-trends-graph.tsxpnaa/components/dashboard/stats-cards.tsxpnaa/package.json
Chase-Fournier
requested changes
Apr 8, 2026
Chase-Fournier
left a comment
Chase-Fournier
left a comment
Collaborator
There was a problem hiding this comment.
Great Job, looks great! Remove that bottom recent fundrasing events card as suggeted and I can merge this in
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
pnaa/components/dashboard/membership-trends-graph.tsx (3)
82-124: Consider memoizingregionDataanddisplayData.Similar to
chartData, these derived computations would benefit fromuseMemoto avoid recalculating on every render when their dependencies haven't changed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pnaa/components/dashboard/membership-trends-graph.tsx` around lines 82 - 124, Derived arrays regionData, sortedRegionData and displayData are being recomputed every render; wrap the reduce/sort/map/filter logic in React.useMemo to memoize results: compute regionData via useMemo depending on chartData, then compute sortedRegionData via useMemo depending on regionData, and compute displayData via useMemo depending on isAllRegions, sortedRegionData (or chartData) and filterBy; ensure dependency arrays reference the exact inputs (chartData, regionData, isAllRegions, filterBy) so recalculation only happens when inputs change.
64-77: Missing memoization forchartData.
chartDatais recomputed on every render even whenchaptersandaliasMaphaven't changed. With many chapters, this could impact performance. Wrap it inuseMemo.♻️ Proposed refactor
- const chartData = chapters.map(ch => { + const chartData = useMemo(() => chapters.map(ch => { const rawRegion = ch.region ?? "None"; const normalizedRegion = rawRegion === "None" || rawRegion === "Unknown" ? "Other" : rawRegion; return { chapter: (aliasMap[ch.id]|| ch.name).replace(/^PNA\s*/, "").replace(/\s*PNAC\d+\s*/i, ""), total: ch.totalMembers ?? 0, active: ch.totalActive ?? 0, lapsed: ch.totalLapsed ?? 0, region: normalizedRegion, } - }); + }), [chapters, aliasMap]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pnaa/components/dashboard/membership-trends-graph.tsx` around lines 64 - 77, chartData is being recalculated on every render; wrap its computation in React's useMemo so it only recomputes when dependencies change. Replace the direct const chartData = ... with a useMemo that returns the mapped array and list chapters and aliasMap as dependencies; keep the existing normalization logic (rawRegion/normalizedRegion and the chapter name transforms) and the returned fields (chapter, total, active, lapsed, region) inside the memo callback to preserve behavior.
132-158:CustomXAxisTickis recreated on every render.Defining this component inside the render function causes it to be a new reference each render, which can break React's reconciliation and cause the chart to re-mount tick components unnecessarily. Also, the
anytype loses type safety.Move it outside the component or memoize it. If it needs access to
isAllRegions, pass it as a prop or use a factory function.♻️ Proposed refactor - move outside and parameterize
// Outside the component const createCustomXAxisTick = (isAllRegions: boolean) => { const CustomXAxisTick = ({ x, y, payload }: { x: number; y: number; payload: { value: string } }) => { const fullText = payload.value; const maxLength = isAllRegions ? 20 : 12; const truncated = fullText.length > maxLength ? fullText.slice(0, maxLength) + "..." : fullText; return ( <g transform={`translate(${x}, ${y})`}> <title>{fullText}</title> <text x={0} y={0} dy={16} textAnchor={isAllRegions ? "middle" : "end"} fill="#6b7280" fontSize={12} transform={isAllRegions ? "rotate(0)" : "rotate(-50)"} > {truncated} </text> </g> ); }; return CustomXAxisTick; }; // Inside the component, memoize based on isAllRegions const CustomXAxisTick = useMemo(() => createCustomXAxisTick(isAllRegions), [isAllRegions]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pnaa/components/dashboard/membership-trends-graph.tsx` around lines 132 - 158, CustomXAxisTick is being recreated each render and typed as any; extract it out of the render or create a factory/memo so its identity is stable and add proper types. Move the component definition for CustomXAxisTick outside the parent component (or use useMemo with a createCustomXAxisTick(isAllRegions) factory) and accept typed props like { x: number; y: number; payload: { value: string } } while passing isAllRegions in as a prop or via the factory so the tick logic (maxLength/truncation, textAnchor, rotate) remains the same but the component reference is stable across renders.pnaa/app/(app)/dashboard/page.tsx (1)
10-10: Unused import:FundraisingProgressThis import is no longer used since the
FundraisingProgresswidget was removed from the dashboard grid. Remove it to avoid dead code.-import { FundraisingProgress } from "@/components/dashboard/fundraising-progress";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pnaa/app/`(app)/dashboard/page.tsx at line 10, Remove the unused import FundraisingProgress from the top of page.tsx: locate the import statement "import { FundraisingProgress } ..." and delete it (or remove FundraisingProgress from the import list if other named imports are present), then run the linter/build to ensure no remaining references.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pnaa/app/`(app)/dashboard/page.tsx:
- Line 71: The description prop string in page.tsx has an accidental trailing
space at the end of the value (description="... ") — remove the final space
character inside the quotes so the string ends immediately before the closing
quote; update the description attribute in the page component (the
description="..." JSX prop) to have no trailing whitespace.
In `@pnaa/components/dashboard/membership-trends-graph.tsx`:
- Line 163: The CardTitle component is using an invalid Tailwind class
"font-large"; update the className on CardTitle (component CardTitle in
membership-trends-graph.tsx) to use a valid Tailwind utility such as replacing
"font-large" with "font-semibold" or "font-bold" for weight, or use "text-lg" if
the intent was larger font size, e.g., adjust className="text-sm font-large" to
an appropriate combination like "text-sm font-semibold" or "text-lg
font-semibold".
- Around line 104-108: The code mutates regionData by calling regionData.sort(),
causing potential render bugs because regionData comes from chartData; fix it by
sorting a shallow copy instead (e.g., use a spread or Array.from) and assign
that to sortedRegionData so the original regionData/chartData remains immutable;
update the expression that produces sortedRegionData (referencing
sortedRegionData and regionData) to create the copy before calling .sort().
---
Nitpick comments:
In `@pnaa/app/`(app)/dashboard/page.tsx:
- Line 10: Remove the unused import FundraisingProgress from the top of
page.tsx: locate the import statement "import { FundraisingProgress } ..." and
delete it (or remove FundraisingProgress from the import list if other named
imports are present), then run the linter/build to ensure no remaining
references.
In `@pnaa/components/dashboard/membership-trends-graph.tsx`:
- Around line 82-124: Derived arrays regionData, sortedRegionData and
displayData are being recomputed every render; wrap the reduce/sort/map/filter
logic in React.useMemo to memoize results: compute regionData via useMemo
depending on chartData, then compute sortedRegionData via useMemo depending on
regionData, and compute displayData via useMemo depending on isAllRegions,
sortedRegionData (or chartData) and filterBy; ensure dependency arrays reference
the exact inputs (chartData, regionData, isAllRegions, filterBy) so
recalculation only happens when inputs change.
- Around line 64-77: chartData is being recalculated on every render; wrap its
computation in React's useMemo so it only recomputes when dependencies change.
Replace the direct const chartData = ... with a useMemo that returns the mapped
array and list chapters and aliasMap as dependencies; keep the existing
normalization logic (rawRegion/normalizedRegion and the chapter name transforms)
and the returned fields (chapter, total, active, lapsed, region) inside the memo
callback to preserve behavior.
- Around line 132-158: CustomXAxisTick is being recreated each render and typed
as any; extract it out of the render or create a factory/memo so its identity is
stable and add proper types. Move the component definition for CustomXAxisTick
outside the parent component (or use useMemo with a
createCustomXAxisTick(isAllRegions) factory) and accept typed props like { x:
number; y: number; payload: { value: string } } while passing isAllRegions in as
a prop or via the factory so the tick logic (maxLength/truncation, textAnchor,
rotate) remains the same but the component reference is stable across renders.
🪄 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: d9febddf-940c-4717-a98d-cc3849d859ba
📒 Files selected for processing (2)
pnaa/app/(app)/dashboard/page.tsxpnaa/components/dashboard/membership-trends-graph.tsx
| : `${user?.chapterName || "Chapter"} Dashboard` | ||
| } | ||
| description="Overview of your organization" | ||
| description="The Philippine Nurses Association of America represents 65 chapters with over 13,000+ members. We uphold the positive image and welfare of Filipino-American nurses, promote professional excellence, and contribute to significant outcomes to healthcare and society through education, research, and clinical practice. " |
Contributor
There was a problem hiding this comment.
Trailing space in description string.
There's a trailing space before the closing quote which is likely unintentional.
- description="The Philippine Nurses Association of America represents 65 chapters with over 13,000+ members. We uphold the positive image and welfare of Filipino-American nurses, promote professional excellence, and contribute to significant outcomes to healthcare and society through education, research, and clinical practice. "
+ description="The Philippine Nurses Association of America represents 65 chapters with over 13,000+ members. We uphold the positive image and welfare of Filipino-American nurses, promote professional excellence, and contribute to significant outcomes to healthcare and society through education, research, and clinical practice."📝 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
| description="The Philippine Nurses Association of America represents 65 chapters with over 13,000+ members. We uphold the positive image and welfare of Filipino-American nurses, promote professional excellence, and contribute to significant outcomes to healthcare and society through education, research, and clinical practice. " | |
| description="The Philippine Nurses Association of America represents 65 chapters with over 13,000+ members. We uphold the positive image and welfare of Filipino-American nurses, promote professional excellence, and contribute to significant outcomes to healthcare and society through education, research, and clinical practice." |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pnaa/app/`(app)/dashboard/page.tsx at line 71, The description prop string in
page.tsx has an accidental trailing space at the end of the value
(description="... ") — remove the final space character inside the quotes so the
string ends immediately before the closing quote; update the description
attribute in the page component (the description="..." JSX prop) to have no
trailing whitespace.
Comment on lines
+104
to
+108
| const sortedRegionData = regionData.sort((a, b) => { | ||
| if (a.region === "Other") return 1; | ||
| if(b.region === "Other") return -1; | ||
| return a.region.localeCompare(b.region); | ||
| }) |
Contributor
There was a problem hiding this comment.
Array mutation: .sort() mutates the original array.
regionData.sort() mutates the array in place. Since regionData is derived from chartData, this can lead to unpredictable behavior across renders. Create a copy before sorting.
🐛 Proposed fix
- const sortedRegionData = regionData.sort((a, b) => {
+ const sortedRegionData = [...regionData].sort((a, b) => {
if (a.region === "Other") return 1;
if(b.region === "Other") return -1;
return a.region.localeCompare(b.region);
- })
+ });📝 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
| const sortedRegionData = regionData.sort((a, b) => { | |
| if (a.region === "Other") return 1; | |
| if(b.region === "Other") return -1; | |
| return a.region.localeCompare(b.region); | |
| }) | |
| const sortedRegionData = [...regionData].sort((a, b) => { | |
| if (a.region === "Other") return 1; | |
| if(b.region === "Other") return -1; | |
| return a.region.localeCompare(b.region); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pnaa/components/dashboard/membership-trends-graph.tsx` around lines 104 -
108, The code mutates regionData by calling regionData.sort(), causing potential
render bugs because regionData comes from chartData; fix it by sorting a shallow
copy instead (e.g., use a spread or Array.from) and assign that to
sortedRegionData so the original regionData/chartData remains immutable; update
the expression that produces sortedRegionData (referencing sortedRegionData and
regionData) to create the copy before calling .sort().
| return ( | ||
| <Card> | ||
| <CardHeader className="flex flex-row items-center justify-between pb-2"> | ||
| <CardTitle className="text-sm font-large"> |
Contributor
There was a problem hiding this comment.
Invalid Tailwind class: font-large.
font-large is not a standard Tailwind CSS class. Did you mean font-semibold, font-bold, or text-lg?
- <CardTitle className="text-sm font-large">
+ <CardTitle className="text-sm font-semibold">📝 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
| <CardTitle className="text-sm font-large"> | |
| <CardTitle className="text-sm font-semibold"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pnaa/components/dashboard/membership-trends-graph.tsx` at line 163, The
CardTitle component is using an invalid Tailwind class "font-large"; update the
className on CardTitle (component CardTitle in membership-trends-graph.tsx) to
use a valid Tailwind utility such as replacing "font-large" with "font-semibold"
or "font-bold" for weight, or use "text-lg" if the intent was larger font size,
e.g., adjust className="text-sm font-large" to an appropriate combination like
"text-sm font-semibold" or "text-lg font-semibold".
Chase-Fournier
requested changes
Apr 8, 2026
Collaborator
There was a problem hiding this comment.
Look at the screenshots above.
The graph seems to be not respecting alias's, where the alias merges the aliased chapters into the main one, and it seems to have inaccuracte information in some places as compared with the chapters.
After fixing the alias's double check if problems still persist by ensuring that all the chapters you check match exactly and if they don't figure out why.
Edit:
The error causing this is unrelated and the only change - besides looking at the coderaibbit changes - is fixing the alias's not merging
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.
Added a stacked bar chart to the top of the dashboard page capturing membership across regions and chapters, sortable by membership status (lapsed/active), and region, showing counts for each chapter. "All regions" shows aggregate counts by region, and sorting by specific regions gives the chapter breakdown for chapters included in that region.
Type of change: Front-end feature
Related issues: "Design a new dashboard"
Summary by CodeRabbit
New Features
UI Copy
Removed
Style