fix(finance): dark mode theme for finance page#5382
Merged
princerajpoot20 merged 4 commits intoMay 13, 2026
Conversation
sammy200-ui
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg,
princerajpoot20 and
sambhavgupta0705
as code owners
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughAdds Tailwind dark-mode classes across FinancialSummary components and pages/finance.tsx, and introduces an isMounted mount gate in BarChartComponent to avoid hydration mismatches. ChangesFinance Page Dark Mode
Sequence Diagram(s)sequenceDiagram
participant Browser
participant BarChartComponent
participant useEffect
participant BarChart
participant ExpensesCard
Browser->>BarChartComponent: mount
BarChartComponent->>useEffect: register resize listener, compute windowWidth
useEffect->>BarChartComponent: set isMounted = true
BarChartComponent->>BarChart: render (if isMounted && windowWidth >= 900)
BarChartComponent->>ExpensesCard: render (if isMounted && windowWidth < 900)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
Pull request overview
Adds dark-mode theming support to the Finance page by applying Tailwind dark: variants across multiple FinancialSummary sections so backgrounds and text render appropriately when the site is in dark mode.
Changes:
- Add dark-mode page/section backgrounds (
dark:bg-dark-background,dark:bg-dark-card) across Finance components. - Add dark-mode text styling (
dark:text-dark-text,dark:text-white) for headings, paragraphs, and table cells. - Improve BarChart theming for dark mode (axis/grid/legend styling) and invert the GitHub logo in dark mode.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pages/finance.tsx | Adds a dark-mode background wrapper for the finance page. |
| components/FinancialSummary/AsyncAPISummary.tsx | Adds dark-mode text styling and inverts the GitHub logo in dark mode. |
| components/FinancialSummary/BarChartComponent.tsx | Adds dark-mode text styling and adjusts grid/axis/legend coloring for the chart. |
| components/FinancialSummary/Card.tsx | Adds dark-mode card background and text color. |
| components/FinancialSummary/CustomTooltip.tsx | Adds dark-mode tooltip background/border styling. |
| components/FinancialSummary/ExpenseBreakdown.tsx | Adds dark-mode section/card backgrounds and text colors. |
| components/FinancialSummary/OtherFormsComponent.tsx | Adds dark-mode section/card backgrounds and text colors. |
| components/FinancialSummary/SponsorshipTiers.tsx | Adds dark-mode section background and text colors for the tier table. |
| components/FinancialSummary/SuccessStories.tsx | Adds dark-mode section background and text colors for story content. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const shouldUseContainer = windowWidth > 1700; | ||
|
|
||
| return <div>{shouldUseContainer ? <Container wide>{renderComponents()}</Container> : renderComponents()}</div>; | ||
| return <div className='dark:bg-dark-background'>{shouldUseContainer ? <Container wide>{renderComponents()}</Container> : renderComponents()}</div>; |
Comment on lines
18
to
21
| <div className='bg-opacity-90/90 rounded-md border border-gray-300 dark:border-gray-600 bg-white dark:bg-dark-card p-2 shadow-md'> | ||
| <p className='text-14 mb-1 font-bold'>{data.Category}</p> | ||
| <p className='text-12 text-gray-900'>${data.Amount.toFixed(2)}</p> | ||
| <p>Click the bar to learn more</p> |
Comment on lines
18
to
20
| <div className='bg-opacity-90/90 rounded-md border border-gray-300 dark:border-gray-600 bg-white dark:bg-dark-card p-2 shadow-md'> | ||
| <p className='text-14 mb-1 font-bold'>{data.Category}</p> | ||
| <p className='text-12 text-gray-900'>${data.Amount.toFixed(2)}</p> |
| @@ -69,7 +69,7 @@ export default function AsyncAPISummary() { | |||
| </a> | |||
| <a href='https://github.com/sponsors/asyncapi' target='_blank'> | |||
| className='mx-4 inline size-10 transition-transform hover:scale-110 active:scale-90' | ||
| className='mx-4 inline size-10 transition-transform hover:scale-110 active:scale-90 dark:invert' | ||
| src='/img/logos/github-black.svg' | ||
| alt='Github' |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## website-ui #5382 +/- ##
============================================
Coverage 100.00% 100.00%
============================================
Files 22 22
Lines 830 830
Branches 159 159
============================================
Hits 830 830 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Author
|
Ready for Review |
Sourya07
approved these changes
May 8, 2026
Contributor
Sourya07
left a comment
Sourya07
left a comment
There was a problem hiding this comment.
Thanks for the contribution
Member
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
![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: 7
🤖 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 `@components/FinancialSummary/BarChartComponent.tsx`:
- Around line 231-233: Replace the direct window.open call in BarChartComponent
where matchedLinkObject is used: call const newWindow =
window.open(matchedLinkObject.link, '_blank', 'noopener,noreferrer') and then if
(newWindow) newWindow.opener = null; this ensures the external link is opened
with noopener,noreferrer and defensively severs the opener reference to prevent
tabnabbing for the matchedLinkObject handling code.
- Around line 215-248: The BarChart is rendered whenever isMounted is true even
though barWidth and barHeight are undefined for windowWidth < 900; update the
render condition so the BarChart (component using BarChart, barWidth, barHeight,
chartData, and the Bar onClick logic) is only rendered when isMounted &&
windowWidth >= 900, while retaining the existing ExpensesCard render for small
screens (windowWidth < 900); locate the JSX where BarChart is returned and
wrap/guard it with the windowWidth >= 900 check.
In `@components/FinancialSummary/CustomTooltip.tsx`:
- Around line 18-20: Replace the invalid Tailwind opacity utility and add a
dark-mode text variant: in the CustomTooltip component, update the wrapper div's
class to use a valid opacity utility (replace "bg-opacity-90/90" with
"bg-opacity-90"), and update the amount paragraph (the <p> that renders
{data.Amount.toFixed(2)}) to include "dark:text-dark-text" alongside
"text-gray-900" (e.g., "text-12 text-gray-900 dark:text-dark-text") so the
amount remains readable on dark:bg-dark-card.
In `@components/FinancialSummary/ExpenseBreakdown.tsx`:
- Line 41: The long JSX heading line in ExpenseBreakdown.tsx (the h2 rendering
expense.title) violates Prettier; split the element's attributes and content
across multiple lines so it conforms to Prettier formatting (e.g., put the
className on its own line and the children on a new line) for the h2 that
renders expense.title to resolve the CI formatting failure.
In `@components/FinancialSummary/OtherFormsComponent.tsx`:
- Line 29: Prettier fails because the JSX props on the mapped div in
OtherFormsComponent are on the same line; split the props so each prop (key and
className) is on its own line and reformat the JSX accordingly, and replace the
unstable key={index} with a stable identifier such as key={form.id ?? form.title
?? index} (use the form object from the map callback) to avoid reordering
issues—update the div in the map inside OtherFormsComponent to use the new key
expression and multiline props.
In `@components/FinancialSummary/SponsorshipTiers.tsx`:
- Line 22: Prettier is flagging the single-line JSX props on the Paragraph
component; update the Paragraph element (the one using ParagraphTypeStyle.md and
the className with Tailwind classes) to format props across multiple lines so
each prop is on its own line (e.g., place the typeStyle prop on its own line,
the className prop on its own line) and close the tag on a new line to satisfy
the formatter.
In `@pages/finance.tsx`:
- Line 58: The JSX return expression in the component currently uses a
single-line conditional which breaks Prettier; rewrite the return so the outer
<div> and its child conditional are formatted across multiple lines (put the
<div className='dark:bg-dark-background'> on its own line, the conditional
expression on the next lines, with <Container
wide>{renderComponents()}</Container> and renderComponents() each on their own
lines) while keeping the existing logic that uses shouldUseContainer and the
renderComponents() call and the Container component.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5d2cee9b-e322-4ac7-a6c0-5cf669cc8344
📒 Files selected for processing (9)
components/FinancialSummary/AsyncAPISummary.tsxcomponents/FinancialSummary/BarChartComponent.tsxcomponents/FinancialSummary/Card.tsxcomponents/FinancialSummary/CustomTooltip.tsxcomponents/FinancialSummary/ExpenseBreakdown.tsxcomponents/FinancialSummary/OtherFormsComponent.tsxcomponents/FinancialSummary/SponsorshipTiers.tsxcomponents/FinancialSummary/SuccessStories.tsxpages/finance.tsx
|
Contributor
Author
|
@princerajpoot20 Resolved coderabbit suggestion , had to consider all of the reviews cause some were related to prettier , other were related to invalid Tailwind syntax |
Member
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
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)
components/FinancialSummary/AsyncAPISummary.tsx (1)
52-76:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
rel="noopener noreferrer"to external links opened in new tabs.All three anchors using
target='_blank'should includerel='noopener noreferrer'to prevent reverse-tabnabbing.Suggested patch
- <a href='https://opencollective.com/asyncapi' target='_blank'> + <a href='https://opencollective.com/asyncapi' target='_blank' rel='noopener noreferrer'> ... - <a + <a href='https://crowdfunding.lfx.linuxfoundation.org/projects/445898e9-42a2-4965-9e0a-c2a714f381bc' target='_blank' + rel='noopener noreferrer' > ... - <a href='https://github.com/sponsors/asyncapi' target='_blank'> + <a href='https://github.com/sponsors/asyncapi' target='_blank' rel='noopener noreferrer'>🤖 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 `@components/FinancialSummary/AsyncAPISummary.tsx` around lines 52 - 76, In the AsyncAPISummary React component update each anchor element that uses target='_blank' (the three <a> tags rendering Open Collective, LFX and GitHub links) to include rel='noopener noreferrer'; locate these anchors inside the JSX in AsyncAPISummary.tsx and add the rel attribute to prevent reverse-tabnabbing while keeping the existing target and other attributes unchanged.
🧹 Nitpick comments (1)
components/FinancialSummary/BarChartComponent.tsx (1)
248-248: 💤 Low valueConsider simplifying the ternary operator.
The ternary with
nullcan be simplified using the&&operator for more concise code.♻️ Proposed simplification
- {isMounted && windowWidth < 900 ? <ExpensesCard /> : null} + {isMounted && windowWidth < 900 && <ExpensesCard />}🤖 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 `@components/FinancialSummary/BarChartComponent.tsx` at line 248, Replace the ternary that returns null with a logical && expression to simplify the render in BarChartComponent: locate the JSX expression using isMounted and windowWidth that currently reads "{isMounted && windowWidth < 900 ? <ExpensesCard /> : null}" and change it to use short-circuit rendering with "isMounted && windowWidth < 900 && <ExpensesCard />" so the ExpensesCard component is only rendered when both conditions are true.
🤖 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.
Outside diff comments:
In `@components/FinancialSummary/AsyncAPISummary.tsx`:
- Around line 52-76: In the AsyncAPISummary React component update each anchor
element that uses target='_blank' (the three <a> tags rendering Open Collective,
LFX and GitHub links) to include rel='noopener noreferrer'; locate these anchors
inside the JSX in AsyncAPISummary.tsx and add the rel attribute to prevent
reverse-tabnabbing while keeping the existing target and other attributes
unchanged.
---
Nitpick comments:
In `@components/FinancialSummary/BarChartComponent.tsx`:
- Line 248: Replace the ternary that returns null with a logical && expression
to simplify the render in BarChartComponent: locate the JSX expression using
isMounted and windowWidth that currently reads "{isMounted && windowWidth < 900
? <ExpensesCard /> : null}" and change it to use short-circuit rendering with
"isMounted && windowWidth < 900 && <ExpensesCard />" so the ExpensesCard
component is only rendered when both conditions are true.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7dc26a6a-6843-46b6-bb14-81ed3b2a4e91
📒 Files selected for processing (9)
components/FinancialSummary/AsyncAPISummary.tsxcomponents/FinancialSummary/BarChartComponent.tsxcomponents/FinancialSummary/Card.tsxcomponents/FinancialSummary/CustomTooltip.tsxcomponents/FinancialSummary/ExpenseBreakdown.tsxcomponents/FinancialSummary/OtherFormsComponent.tsxcomponents/FinancialSummary/SponsorshipTiers.tsxcomponents/FinancialSummary/SuccessStories.tsxpages/finance.tsx
Member
|
@sammy200-ui I hope the suggestions by CodeRabbit that you marked as resolved have been addressed as I don't see any justification mentioned there |
Contributor
Author
|
@princerajpoot20 yep all the 7 coderabbit suggestion has been taken in consideration 358f90d |
Member
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Member
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
|
✅ Actions performedFull review triggered. |
Contributor
✅ Actions performedFull review triggered. |
1 similar comment
Contributor
✅ Actions performedFull review triggered. |
princerajpoot20
approved these changes
May 13, 2026
Member
princerajpoot20
left a comment
princerajpoot20
left a comment
There was a problem hiding this comment.
@sammy200-ui Thanks for your effort
github-project-automation
Bot
moved this from To Be Triaged
to Done
in Website - Kanban
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
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.
resolves : #5253 (comment)
Before : https://deploy-preview-5253--asyncapi-website.netlify.app/finance
After : https://deploy-preview-5382--asyncapi-website.netlify.app/finance
Summary by CodeRabbit
Style
Bug Fixes