-
-
Notifications
You must be signed in to change notification settings - Fork 28
Added darkmode #376
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Added darkmode #376
Conversation
+Added a darkmode button in the main header + added a react state for whether darkmode is toggled + changed various <b> and <p> tags to use andt Typography + Removed some white backgrounds of various images
WalkthroughThis pull request introduces dark mode support to the app by implementing Ant Design's ConfigProvider with theme switching logic in App.tsx, removes global Ant Design styling from App.css, and systematically replaces raw HTML elements and Ant Design Title components with Typography.Text components across the web package. Additionally, backgroundColor styles are commented out in several components and theme tokens are integrated into helper components for token-aware styling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App as App.tsx
participant ConfigProvider as ConfigProvider
participant MainHeader as MainHeader
participant Theme as Ant Design Theme
User->>MainHeader: Click theme toggle button
MainHeader->>App: onToggleTheme()
App->>App: Set isDarkMode = !isDarkMode
App->>ConfigProvider: Pass isDarkMode state
ConfigProvider->>Theme: Select theme algorithm<br/>(dark or default)
Theme->>App: Apply theme tokens
App->>MainHeader: Update isDarkMode prop
MainHeader->>MainHeader: Show SunOutlined or MoonOutlined
MainHeader->>User: Render updated theme UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/web/src/pages/desktop-app/desktop-app.tsx (1)
83-83: Inconsistent download button text formatting.The first download button (Line 83) displays
Download v{AppVersionFile.version}with a "v" prefix, while the second button (Line 229) showsDownload {AppVersionFile.version}without it. For consistency, both buttons should use the same format.Apply this diff to make both buttons consistent:
> - Download v{AppVersionFile.version} + Download {AppVersionFile.version} </Button>Also applies to: 229-229
🧹 Nitpick comments (14)
packages/web/src/pages/commanders/commandersList.tsx (2)
41-41: Consider replacing hard-coded styling with theme tokens for consistency.The PR implements theme tokens via Ant Design's ConfigProvider across the codebase. This component uses inline styled objects rather than token-aware styling. While the current change removes the problematic white background, consider refactoring this to use theme tokens (if the background image opacity or overlay needs theme-aware adjustment in the future) to maintain consistency with the broader theming approach introduced in this PR.
41-41: Remove or restore the commented code.The commented-out line suggests a transitional state. If this styling is no longer needed, remove the comment entirely. If it's meant to be restored conditionally or for a fallback, clarify the intent with an explanatory comment.
packages/web/src/pages/match/single-match.tsx (1)
123-129: Consider using Typography.Paragraph or separate Text elements for multi-line content.Typography.Text is an inline component, so wrapping multiple lines with
<br />inside it is semantically unusual and may cause unexpected layout behavior. For clarity and better semantics (especially important for dark mode rendering consistency), consider using Typography.Paragraph or splitting into separate elements:- <Text>Match type {formatMatchtypeID(matchData.matchtype_id)} - <br /> - Map {matchData.mapname} - <br /> - Match duration {getMatchDuration(matchData.startgametime, matchData.completiontime)} - <br /> - Played on {formatMatchTime(matchData.completiontime, true)}</Text> + <Typography.Paragraph style={{ marginBottom: 0 }}> + <div>Match type {formatMatchtypeID(matchData.matchtype_id)}</div> + <div>Map {matchData.mapname}</div> + <div>Match duration {getMatchDuration(matchData.startgametime, matchData.completiontime)}</div> + <div>Played on {formatMatchTime(matchData.completiontime, true)}</div> + </Typography.Paragraph>packages/web/src/App.tsx (2)
2-2: Remove unusedButtonimport.The
Buttoncomponent is imported but never used in this file. It's only used inmain-header.tsx.Apply this diff:
-import { ConfigProvider, theme, Button, Layout } from "antd"; +import { ConfigProvider, theme, Layout } from "antd";
35-38: Consider persisting the dark mode preference.The dark mode state is reset to
falseon every page refresh, which means users must re-enable dark mode each time they visit. Consider persisting the preference usinglocalStorageor a similar mechanism.Here's an example implementation:
const [isDarkMode, setIsDarkMode] = useState(() => { const saved = localStorage.getItem('darkMode'); return saved ? JSON.parse(saved) : false; }); const handleToggleTheme = () => { setIsDarkMode((prev) => { const newValue = !prev; localStorage.setItem('darkMode', JSON.stringify(newValue)); return newValue; }); };packages/web/src/components/main-header.tsx (1)
214-218: Add accessibility attributes to the theme toggle button.The button implementation is functional, but it lacks accessibility features. Screen reader users won't know what this button does.
Apply this diff to improve accessibility:
<Button shape="circle" icon={isDarkMode ? <SunOutlined /> : <MoonOutlined />} onClick={onToggleTheme} + aria-label={isDarkMode ? "Switch to light mode" : "Switch to dark mode"} + title={isDarkMode ? "Switch to light mode" : "Switch to dark mode"} />packages/web/src/pages/desktop-app/desktop-app.tsx (1)
65-72: Align font size units for consistency.The parent
<ul>specifiesfontSize: "20px"while the child<Text>components usefontSize: "large". The CSS keyword "large" typically resolves to approximately 18px, which is smaller than the parent's 20px setting. For consistent sizing and predictability, consider using the same unit system.Apply this diff to align the font sizes:
- <ul style={{ fontSize: "20px" }}> - <li><Text strong style={{ fontSize: "large" }}>Easy to use, no configuration required - just start the app</Text></li> - <li><Text strong style={{ fontSize: "large" }}>See detailed leaderboard stats of players in your game</Text></li> - <li><Text strong style={{ fontSize: "large" }}>Game results prediction based on map analysis</Text></li> - <li><Text strong style={{ fontSize: "large" }}>Loads automatically when joining a new game</Text></li> - <li><Text strong style={{ fontSize: "large" }}>Twitch stream overlay extension</Text></li> - <li><Text strong style={{ fontSize: "large" }}>Streamer mode with OBS and Twitch Studio support</Text></li> + <ul> + <li><Text strong style={{ fontSize: "20px" }}>Easy to use, no configuration required - just start the app</Text></li> + <li><Text strong style={{ fontSize: "20px" }}>See detailed leaderboard stats of players in your game</Text></li> + <li><Text strong style={{ fontSize: "20px" }}>Game results prediction based on map analysis</Text></li> + <li><Text strong style={{ fontSize: "20px" }}>Loads automatically when joining a new game</Text></li> + <li><Text strong style={{ fontSize: "20px" }}>Twitch stream overlay extension</Text></li> + <li><Text strong style={{ fontSize: "20px" }}>Streamer mode with OBS and Twitch Studio support</Text></li> </ul>packages/web/src/pages/about/about.tsx (2)
134-172: Large Text wrapper spans many lines.The extensive Text wrapper is valid and achieves the dark mode theming objective. However, consider whether breaking this into multiple Paragraph or Text components would improve maintainability without compromising the dark mode functionality.
208-211: Use Typography for consistency.For consistency with the dark mode migration approach throughout this file, replace the
<i>tag with Typography components.Apply this diff if not restructuring the entire section:
- <i> + <Text italic> You can Donate via PayPal or Card at Ko-Fi, <br /> no registration required. - </i> + </Text>packages/web/src/pages/ladders/leaderboards.tsx (1)
501-506: Consider simplifying spacing and template literal.The Typography.Text replacement is good for dark mode support. However, the spacing structure may result in double spaces:
" as of {" "}has both a trailing space in the text and an explicit space character. Additionally, the template literal could be simplified.Consider this refactor for cleaner code:
- <Text> as of {" "} - {`${selectedTimeStamp === "now" - ? "now" - : new Date(parseInt(selectedTimeStamp) * 1000).toLocaleString() - }`}{" "} - </Text> + <Text> + {" "}as of{" "} + {selectedTimeStamp === "now" + ? "now" + : new Date(parseInt(selectedTimeStamp) * 1000).toLocaleString()} + {" "} + </Text>packages/web/src/pages/players/stats/player-stats.tsx (1)
115-160: UseTitlecomponents with appropriate levels instead ofTextfor semantic structure.Replacing
Titlecomponents withTextremoves semantic heading elements from the page. The codebase usesTitlethroughout (inabout.tsx,recent-matches.tsx,match/single-match.tsx, and stats pages) with proper heading levels, and the app'sConfigProvideralready supports dark mode via the theme algorithm—so the original components should work without replacement.Consider using
Title level={3}orlevel={4}for the stat labels instead ofText strong style={{ fontSize: "large" }}to maintain:
- Proper heading hierarchy for screen readers
- Semantic HTML structure for accessibility and SEO
- Consistency with the rest of the codebase
Lines 115–160 need this adjustment across all stat label elements.
packages/web/src/pages/players/player-card/player-card.tsx (1)
319-319: Consider consistent spacing approach.The
{" "}expression inside the Text component works but could be simplified for consistency with other parts of the code. Consider moving the space outside or using it consistently across all similar patterns.Example refactor:
- <Text>Most played{" "}</Text> + <Text>Most played </Text>packages/web/src/pages/commanders/commanderDetails.tsx (1)
75-80: Consider removing unnecessary Row wrappers.Each
Textcomponent is wrapped in its ownRow, which adds flex layout that may not be necessary for single text elements. This could affect spacing and layout.Simplify by removing the Row wrappers:
- <Row> - <Text strong style={{ fontSize: "large" }}>{commanderData.commanderName}</Text> - </Row> - <Row> - <Text>{commanderData.description}</Text> - </Row> + <Text strong style={{ fontSize: "large" }}>{commanderData.commanderName}</Text> + <br /> + <Text>{commanderData.description}</Text>packages/web/src/pages/live-matches/live-matches.tsx (1)
4-14: Remove unused imports.
Table,Tooltip,ConfigProvider, andSwitchare imported but never used in this file.Apply this diff to clean up the imports:
import { - Table, Space, Col, Row, - Tooltip, - ConfigProvider, Select, Typography, - Switch, } from "antd";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (21)
.yarnrc.yml(1 hunks)packages/app/package.json(2 hunks)packages/web/src/App.css(0 hunks)packages/web/src/App.tsx(2 hunks)packages/web/src/components/export-date.tsx(1 hunks)packages/web/src/components/helper.tsx(1 hunks)packages/web/src/components/main-header.tsx(6 hunks)packages/web/src/components/patch-notifications.tsx(1 hunks)packages/web/src/components/tip.tsx(1 hunks)packages/web/src/pages/about/about.tsx(3 hunks)packages/web/src/pages/commanders/commanderDetails.tsx(4 hunks)packages/web/src/pages/commanders/commandersList.tsx(1 hunks)packages/web/src/pages/desktop-app/desktop-app.tsx(9 hunks)packages/web/src/pages/ladders/leaderboards.tsx(3 hunks)packages/web/src/pages/live-matches/live-matches.tsx(4 hunks)packages/web/src/pages/map-stats/map-stats-details.tsx(1 hunks)packages/web/src/pages/match/single-match.tsx(2 hunks)packages/web/src/pages/matches/match-details-table.tsx(5 hunks)packages/web/src/pages/players/player-card/player-card.tsx(3 hunks)packages/web/src/pages/players/stats/player-stats.tsx(2 hunks)packages/web/src/pages/stats/stats-header.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- packages/web/src/App.css
🧰 Additional context used
🧬 Code graph analysis (6)
packages/web/src/components/export-date.tsx (1)
packages/web/src/config.tsx (1)
commanderAndBulletinDate(41-41)
packages/web/src/components/main-header.tsx (1)
packages/web/src/components/header-search.tsx (1)
PlayerSearchInput(5-19)
packages/web/src/App.tsx (1)
packages/web/src/components/main-header.tsx (1)
MainHeader(117-236)
packages/web/src/pages/match/single-match.tsx (2)
packages/web/src/utils/table-functions.tsx (3)
formatMatchtypeID(133-198)getMatchDuration(16-18)formatMatchTime(46-73)packages/web/src/components/cohstats-icon.tsx (1)
COHStatsIcon(4-15)
packages/web/src/pages/players/player-card/player-card.tsx (2)
packages/web/src/pages/players/player-card/helpers.ts (1)
convertTeamNames(61-61)packages/web/src/utils/helpers.ts (2)
capitalize(171-171)timeAgo(177-177)
packages/web/src/pages/about/about.tsx (1)
packages/web/src/pages/about/kofi-donate.tsx (1)
KofiDonate(5-28)
🪛 Biome (2.1.2)
packages/web/src/pages/matches/match-details-table.tsx
[error] 69-69: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🔇 Additional comments (27)
.yarnrc.yml (1)
1-1: Clarify the purpose of this Yarn configuration addition.The
nodeLinker: node-modulesconfiguration is valid and enables traditional node-modules resolution, but it appears orthogonal to the dark mode feature PR objectives. This change may be incidental or addressing an unrelated tooling concern.Can you clarify whether this configuration change is necessary for the dark mode implementation, or if it's part of a separate tooling/build optimization effort?
packages/app/package.json (1)
35-35: Clarify the intent behind dependency reordering.The changes at lines 35 and 63 reorder dependencies within their respective sections (devDependencies and dependencies) without version or semantic changes. The PR introduces Ant Design dark mode theming but doesn't add any new dependencies—it reuses the existing
[email protected].Is this reordering intentional (e.g., organizing by category or priority) or accidental? If intentional, consider adding a comment or documenting the desired organization convention to prevent future confusion.
Also applies to: 63-63
packages/web/src/pages/commanders/commandersList.tsx (1)
35-42: Verify background styling maintains contrast and readability in both themes.The removal of the white background overlay aligns well with the dark mode implementation. However, since the component now relies solely on
backgroundBlendMode: "overlay"and the underlying theme background, please verify that the background image (the race icon) maintains adequate contrast and readability in both light and dark modes. Pay special attention to text readability over the blended background.packages/web/src/pages/match/single-match.tsx (2)
143-143: Tooltip content wrapping is appropriate.Wrapping single-line text in Typography.Text for the expiry notice is semantically correct and follows Ant Design conventions for this dark mode update.
146-146: Icon + text combination is well-structured.Combining the database icon and COHStatsIcon with text in a single Typography.Text element is appropriate for inline content mixing and should render consistently in both light and dark modes.
packages/web/src/App.tsx (2)
40-40: Good implementation of theme switching.The ConfigProvider setup correctly switches between dark and default themes based on the state.
44-44: Props correctly wired to MainHeader.The
isDarkModeandonToggleThemeprops are properly passed to enable theme control from the header.packages/web/src/components/main-header.tsx (3)
10-10: Imports look good.The new
Buttonand icon imports are properly added and used in the theme toggle implementation.Also applies to: 26-26
27-30: Well-defined props interface.The
MainHeaderPropsinterface clearly defines the required props for theme control.
117-117: Component signature properly updated.The MainHeader component correctly accepts and destructures the new theme-related props.
packages/web/src/pages/desktop-app/desktop-app.tsx (2)
89-89: LGTM!The download count display properly uses separate
Textcomponents to apply different styling to the numeric value and label, which will adapt correctly to dark mode.
101-105: LGTM!The systematic replacement of
<p>tags with Ant Design<Text>components is consistent throughout and properly enables dark mode support. All opening and closing tags are correctly matched, and thefontSize: "20px"styling is consistently applied.Also applies to: 126-129, 135-138, 158-160, 170-180, 206-218
packages/web/src/pages/about/about.tsx (2)
85-85: LGTM!The replacement of HTML bold tags with Typography.Text strong is appropriate for dark mode support.
128-128: LGTM!Consistent with the Typography.Text strong replacement pattern.
packages/web/src/pages/ladders/leaderboards.tsx (3)
146-146: LGTM! Background color removal supports dark mode.Commenting out the white backgroundColor allows the component to inherit the theme-appropriate background from the ConfigProvider, which is essential for proper dark mode rendering.
467-467: Good use of Typography.Text for theme consistency.Replacing the plain text with the Typography.Text component ensures proper theme-aware styling for dark mode.
512-514: Clean Typography.Text implementation.The Text component usage here is clean and properly implements dynamic content with theme-aware styling.
packages/web/src/pages/players/player-card/player-card.tsx (2)
229-237: Typography wrapping looks correct.The XP and playtime information is properly wrapped with Typography.Text components for dark mode compatibility. The conditional logic for MAX and playtime is preserved correctly.
314-314: LGTM!The Typography.Text wrapping for the unranked state is clean and maintains proper spacing.
packages/web/src/components/patch-notifications.tsx (1)
44-44: LGTM!Proper use of Typography.Text for plain text content, consistent with the dark mode typography standardization across the project.
packages/web/src/components/export-date.tsx (1)
3-11: LGTM!Clean implementation of Typography.Text for plain text content, consistent with the dark mode typography pattern.
packages/web/src/components/tip.tsx (1)
2-13: LGTM!Excellent implementation of theme token usage for color styling and Typography.Text for text content. This aligns perfectly with the dark mode theming pattern.
packages/web/src/pages/commanders/commanderDetails.tsx (1)
37-37: LGTM!Commenting out the fixed
backgroundColoris appropriate for dark mode support, allowing the theme's background color to apply.packages/web/src/components/helper.tsx (1)
2-12: LGTM!Excellent improvements: the type change from
string | anytostring | React.ReactNodeimproves type safety, and the theme token usage for icon color styling is properly implemented for dark mode support.packages/web/src/pages/matches/match-details-table.tsx (2)
448-450: LGTM!Activating the smallView filter logic is appropriate. The implementation correctly filters columns based on the
smallViewOnlyIndexesarray.
465-466: LGTM!Proper implementation of theme token usage for icon styling and Typography.Text for text content, consistent with the dark mode pattern.
packages/web/src/pages/live-matches/live-matches.tsx (1)
117-117: Good typography migration for dark mode support.Replacing h3 elements with Typography.Text components aligns well with the PR's dark mode objectives and Ant Design's typography system.
Minor observation: Both text strings have trailing spaces (
"Display live games "and"sort by "), which may be intentional for spacing but could be more explicitly handled with margin/padding on the Text component or Space component configuration.Also applies to: 132-132
| <Text> | ||
| We are still planning to deliver advanced match overview which would bring more | ||
| functionality and better insight into your past games. But we will most likely not be able | ||
| to hold more matches than relic already does because of the increased cost without some | ||
| kind of income (donations/ads?). | ||
| <br /> | ||
| no registration required. | ||
| </i> | ||
| <br /> | ||
| If you like the site please consider donating. | ||
| <br /> | ||
| 100% of donations will go towards this project costs <br /> | ||
| <br /> | ||
| <KofiDonate /> | ||
| <br /> | ||
| <i> | ||
| You can Donate via PayPal or Card at Ko-Fi, | ||
| <br /> | ||
| no registration required. | ||
| </i></Text> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review HTML semantics: block component inside Text wrapper.
The <KofiDonate /> component (line 206) returns a <div> element (as shown in the relevant code snippet), but it's nested inside a <Text> component that typically renders as an inline element. This violates HTML semantics and may cause layout or rendering issues.
Consider restructuring this section to avoid nesting block-level components inside inline Text wrappers.
Apply this diff to restructure the donations section:
- <Text>
- We are still planning to deliver advanced match overview which would bring more
- functionality and better insight into your past games. But we will most likely not be able
- to hold more matches than relic already does because of the increased cost without some
- kind of income (donations/ads?).
- <br />
- <br />
- If you like the site please consider donating.
- <br />
- 100% of donations will go towards this project costs <br />
- <br />
- <KofiDonate />
- <br />
- <i>
- You can Donate via PayPal or Card at Ko-Fi,
- <br />
- no registration required.
- </i></Text>
+ <Paragraph>
+ We are still planning to deliver advanced match overview which would bring more
+ functionality and better insight into your past games. But we will most likely not be able
+ to hold more matches than relic already does because of the increased cost without some
+ kind of income (donations/ads?).
+ </Paragraph>
+ <Paragraph>
+ If you like the site please consider donating.
+ <br />
+ 100% of donations will go towards this project costs
+ </Paragraph>
+ <KofiDonate />
+ <Paragraph>
+ <Text italic>
+ You can Donate via PayPal or Card at Ko-Fi,
+ <br />
+ no registration required.
+ </Text>
+ </Paragraph>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/web/src/pages/about/about.tsx around lines 195 to 212, the
KofiDonate component (which renders a div) is incorrectly nested inside a Text
wrapper causing block-inside-inline HTML semantics issues; move the <KofiDonate
/> out of the Text component and wrap the whole donations section with a block
container (e.g., Box/Stack/Stacked VStack) so the explanatory text remains in
Text elements while the KofiDonate component is placed as its own block sibling,
and adjust spacing/br elements accordingly to preserve visual layout.
| <Row justify={"center"}> | ||
| <div style={{ textAlign: "center" }}> | ||
| <span style={{ fontSize: 20, fontWeight: 600 }}>{gamesAnalyzed}</span> | ||
| <Text style={{ fontSize: 20, fontWeight: 600 }}>{gamesAnalyzed}</Text> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid nesting complex JSX inside Typography.Text.
The gamesAnalyzed variable (lines 116-130) contains other Text components and a Helper component. Wrapping this complex JSX inside another Typography.Text component creates improper nesting.
Remove the Text wrapper and apply styles directly to the gamesAnalyzed content:
- <Text style={{ fontSize: 20, fontWeight: 600 }}>{gamesAnalyzed}</Text>
+ <div style={{ fontSize: 20, fontWeight: 600 }}>{gamesAnalyzed}</div>Alternatively, wrap the text portions inside the gamesAnalyzed definition with Text components instead.
📝 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.
| <Text style={{ fontSize: 20, fontWeight: 600 }}>{gamesAnalyzed}</Text> | |
| <div style={{ fontSize: 20, fontWeight: 600 }}>{gamesAnalyzed}</div> |
🤖 Prompt for AI Agents
In packages/web/src/pages/map-stats/map-stats-details.tsx around line 142, the
JSX nests the complex gamesAnalyzed fragment (which itself contains Text
components and a Helper) inside another Typography.Text wrapper; remove that
outer Text wrapper and instead apply the desired fontSize/fontWeight styles
directly to the gamesAnalyzed content (or move Text wrappers into the
gamesAnalyzed definition for only the plain text pieces) so no Text component
contains other Text/Helper components as children.
| const commanders = matchhistoryitems.filter((item) => item.itemlocation_id === 3); | ||
| const bulletins = matchhistoryitems.filter((item) => item.itemlocation_id === 4); | ||
|
|
||
| const { token } = theme.useToken(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix React hooks violation: move useToken before early return.
The useToken hook is called after the conditional early return (lines 61-63), which violates React's Rules of Hooks. Hooks must be called unconditionally at the top level of the component.
Move the hook call before the early return:
}> = ({ record, matchhistoryitems }) => {
+ const { token } = theme.useToken();
+
useEffect(() => {
firebaseAnalytics.singleMatchPlayerDetailsDisplayed();
}, []);
if (!record || !matchhistoryitems) {
return <></>;
}
matchhistoryitems = matchhistoryitems.filter((item) => item.profile_id === record.profile_id);
const commanders = matchhistoryitems.filter((item) => item.itemlocation_id === 3);
const bulletins = matchhistoryitems.filter((item) => item.itemlocation_id === 4);
- const { token } = theme.useToken();📝 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.
| const { token } = theme.useToken(); | |
| }> = ({ record, matchhistoryitems }) => { | |
| const { token } = theme.useToken(); | |
| useEffect(() => { | |
| firebaseAnalytics.singleMatchPlayerDetailsDisplayed(); | |
| }, []); | |
| if (!record || !matchhistoryitems) { | |
| return <></>; | |
| } | |
| matchhistoryitems = matchhistoryitems.filter((item) => item.profile_id === record.profile_id); | |
| const commanders = matchhistoryitems.filter((item) => item.itemlocation_id === 3); | |
| const bulletins = matchhistoryitems.filter((item) => item.itemlocation_id === 4); |
🧰 Tools
🪛 Biome (2.1.2)
[error] 69-69: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🤖 Prompt for AI Agents
In packages/web/src/pages/matches/match-details-table.tsx around line 69, the
theme.useToken() hook is being called after an early return which violates React
Hooks rules; move the const { token } = theme.useToken(); call up above the
conditional return (i.e., place it with the other hook calls at the top of the
component) so hooks are invoked unconditionally, then keep the early return
logic unchanged after the hooks.
| {bestRank.rank !== Infinity ? ( | ||
| <> | ||
| Current best rank <Text strong>{bestRank.rank}</Text> in{" "} | ||
| <Text>Current best rank </Text><Text strong>{bestRank.rank}</Text> <Text>in{" "}</Text> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor spacing issue with multiple Text components.
The JSX whitespace between <Text strong>{bestRank.rank}</Text> and <Text>in{" "}</Text> may create double spacing. Consider removing either the JSX space or the {" "} expression for consistent spacing.
Apply this diff to fix the spacing:
- <Text>Current best rank </Text><Text strong>{bestRank.rank}</Text> <Text>in{" "}</Text>
+ <Text>Current best rank </Text><Text strong>{bestRank.rank}</Text><Text> in </Text>📝 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.
| <Text>Current best rank </Text><Text strong>{bestRank.rank}</Text> <Text>in{" "}</Text> | |
| <Text>Current best rank </Text><Text strong>{bestRank.rank}</Text><Text> in </Text> |
🤖 Prompt for AI Agents
In packages/web/src/pages/players/player-card/player-card.tsx around line 307,
the JSX currently uses separate Text components with both an explicit space in
the markup and a {" "} expression which can yield double spacing; remove the
redundant {" "} expression (or the literal space) so there is exactly one space
between the rank and the following "in" — alternatively consolidate the pieces
into a single <Text> (e.g., <Text>Current best rank <Text
strong>{bestRank.rank}</Text> in</Text>) to ensure consistent spacing.
| <Text>Last match{" "} | ||
| {timeAgo.format(Date.now() - (Date.now() - lastGameDate * 1000), "round-minute")} | ||
| </Text> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix incorrect date calculation in timeAgo.format.
The calculation Date.now() - (Date.now() - lastGameDate * 1000) simplifies to just lastGameDate * 1000, which passes the timestamp itself instead of the time difference. According to the helper documentation, timeAgo.format expects the millisecond difference, not the absolute timestamp.
Apply this diff to fix the calculation:
- <Text>Last match{" "}
- {timeAgo.format(Date.now() - (Date.now() - lastGameDate * 1000), "round-minute")}
- </Text>
+ <Text>Last match{" "}
+ {timeAgo.format(Date.now() - lastGameDate * 1000, "round-minute")}
+ </Text>📝 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.
| <Text>Last match{" "} | |
| {timeAgo.format(Date.now() - (Date.now() - lastGameDate * 1000), "round-minute")} | |
| </Text> | |
| <Text>Last match{" "} | |
| {timeAgo.format(Date.now() - lastGameDate * 1000, "round-minute")} | |
| </Text> |
🤖 Prompt for AI Agents
In packages/web/src/pages/players/player-card/player-card.tsx around lines 333
to 335, the expression currently passes the absolute timestamp (it simplifies to
lastGameDate * 1000) to timeAgo.format; change it to pass the millisecond
difference by using Date.now() - (lastGameDate * 1000) (i.e., compute Date.now()
- lastGameDate * 1000) and call timeAgo.format with that difference and the same
"round-minute" option so the helper receives the expected elapsed milliseconds.
| <div style={{ textAlign: "center" }}> | ||
| <span style={{ fontSize: 20, fontWeight: 600 }}> | ||
| <GamesAnalyzed /> | ||
| <Text strong style={{ fontSize: "large" }}><GamesAnalyzed /></Text> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid nesting complex components inside Typography.Text.
The GamesAnalyzed component returns complex JSX containing other Text components, Helper components, and JSX fragments. Wrapping this entire structure inside a Typography.Text component creates improper nesting and could cause styling conflicts.
Apply the styling to the existing span element instead:
- <span style={{ fontSize: 20, fontWeight: 600 }}>
- <Text strong style={{ fontSize: "large" }}><GamesAnalyzed /></Text>
-
- </span>
+ <Text style={{ fontSize: 20, fontWeight: 600 }}>
+ <GamesAnalyzed />
+ </Text>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/web/src/pages/stats/stats-header.tsx around line 114 the code wraps
the entire GamesAnalyzed component inside a Typography.Text, which causes
improper nesting because GamesAnalyzed itself returns complex JSX including Text
and Helper elements; remove the outer <Text> wrapper and instead apply the
existing strong/style (or equivalent className/inline styles) to the span inside
GamesAnalyzed (or pass a prop like className/style into GamesAnalyzed to forward
to its root span) so the styling is preserved without nesting Text components.
|
👀 |
Summary by CodeRabbit
New Features
Style
Chores