feat: Add Light Mode Switching#658
Open
TechHutTV wants to merge 18 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces a client-side theme provider, migrates from ChangesTheme System Consolidation and Palette Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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 |
Contributor
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
Contributor
|
✅ Created PR with unit tests: #659 |
![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: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/layouts/Header.tsx (1)
92-92:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
hover:text-whitewas missed by the token migration and breaks in light mode.The base color uses the theme-aware
text-nb-gray-300, but the hover state is still hardcodedhover:text-white. With the inverted light ramp, hovering this nav-collapse button turns the icon white on a light background, making it effectively invisible. Use a theme-aware token instead.🎨 Proposed fix
- "h-10 w-10 hover:text-white flex items-center justify-center text-nb-gray-300 transition-all ml-2", + "h-10 w-10 hover:text-nb-gray-100 flex items-center justify-center text-nb-gray-300 transition-all ml-2",🤖 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 `@src/layouts/Header.tsx` at line 92, In the Header component's nav-collapse button class string (the one containing "h-10 w-10 ... text-nb-gray-300"), remove the hardcoded "hover:text-white" and replace it with the theme-aware hover token used by our design system (e.g., "hover:text-nb-gray-50" or the equivalent theme foreground token), so the hover icon color follows light/dark ramps instead of becoming invisible in light mode.src/app/(dashboard)/team/user/page.tsx (1)
177-200:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInline style overrides className text color change.
The
classNamechange totext-nb-gray-100on line 179 has no effect because the inlinestyleattribute (lines 181–193) sets an explicitcolorproperty that will override any class-based text color. For service users,color: "white"is applied; for regular users, a generated color is used. Inline styles always take precedence over CSS classes.If the intent is to use the theme-aware
nb-gray-100token, remove thecolorproperty from the inline style object (keeping only the conditional logic if needed for other properties). Otherwise, the className text color is unreachable.Proposed fix to remove conflicting inline color
<div className={ "w-10 h-10 rounded-full relative flex items-center justify-center text-nb-gray-100 uppercase text-md font-medium bg-nb-gray-900" } - style={ - isServiceUser - ? { - color: "white", - } - : { - color: user?.name - ? generateColorFromString( - user?.name || user?.id || "System User", - ) - : "`#808080`", - } - } + style={ + !isServiceUser && user?.name + ? { + color: generateColorFromString( + user?.name || user?.id || "System User", + ), + } + : undefined + } >Note: If the generated color for regular users is intentional and should remain, then revert the
text-nb-gray-100className change since it serves no purpose.🤖 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 `@src/app/`(dashboard)/team/user/page.tsx around lines 177 - 200, The text color set by the className "text-nb-gray-100" is being overridden by the inline style color in the div that uses isServiceUser and generateColorFromString (the block around the JSX div with className and style props); either remove the color property from the inline style object so the class token applies (keep the conditional shape but drop color), or if the generated per-user color is intentional, revert removing the className so you don't claim nb-gray-100; update the JSX around isServiceUser, generateColorFromString, and the style prop accordingly.src/components/Code.tsx (1)
43-44: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winIncomplete theme token migration in background and border styles.
Lines 43-44 still use non-theme-aware classes (
bg-gray-50,border-neutral-200) that won't adapt between light and dark modes. For consistent theme support, these should use thenb-graypalette like the text colors.♻️ Proposed fix to complete the migration
className={cn( "rounded-md text-sm transition-all relative duration-300 border", dark - ? "bg-gray-50 border-neutral-200 dark:border-nb-gray-800 hover:dark:bg-nb-gray-900/80 dark:bg-nb-gray-930" - : "bg-gray-50 border-neutral-200 dark:border-nb-gray-700 hover:dark:bg-nb-gray-900/80 dark:bg-nb-gray-900", + ? "bg-nb-gray-50 border-nb-gray-200 dark:border-nb-gray-800 hover:dark:bg-nb-gray-900/80 dark:bg-nb-gray-930" + : "bg-nb-gray-50 border-nb-gray-200 dark:border-nb-gray-700 hover:dark:bg-nb-gray-900/80 dark:bg-nb-gray-900", dark ? "text-nb-gray-300 hover:text-nb-gray-300" : "text-nb-gray-200 hover:text-nb-gray-200",🤖 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 `@src/components/Code.tsx` around lines 43 - 44, In the Code component in src/components/Code.tsx update the class strings in the ternary that currently use "bg-gray-50" and "border-neutral-200" to use the nb-gray palette instead (e.g., replace with "bg-nb-gray-50" and "border-nb-gray-200" or your project's nb-gray equivalents) so they match the existing dark-mode tokens (like dark:border-nb-gray-800, dark:bg-nb-gray-930) and keep both branches consistent with the nb-gray theme tokens.src/modules/peers/PeerAddressCell.tsx (1)
57-57: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winComplete the text token migration for the container.
The IP address text was correctly migrated to
text-nb-gray-400on line 68, but the parent container on line 57 still uses staticdark:text-neutral-300 text-neutral-500. Complete the migration for consistency.♻️ Proposed fix
- <div className="flex flex-col gap-0 dark:text-neutral-300 text-neutral-500 font-light truncate"> + <div className="flex flex-col gap-0 text-nb-gray-300 font-light truncate">🤖 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 `@src/modules/peers/PeerAddressCell.tsx` at line 57, The parent container div in PeerAddressCell.tsx still uses the old classes "dark:text-neutral-300 text-neutral-500"; update that container's className (the div with "flex flex-col gap-0 ... truncate") to use the migrated token "text-nb-gray-400" instead of those static neutral classes so it matches the IP text migration while keeping the other classes (e.g., "flex flex-col gap-0 font-light truncate") intact.src/components/modal/Modal.tsx (1)
73-73: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winComplete the border token migration for light mode.
ModalContent uses
border-neutral-200for light mode while dark mode correctly usesdark:border-nb-gray-900. For consistency with the theme-aware token system, light mode should also use annb-gray-*token instead of the staticneutral-200.♻️ Proposed fix
- "mx-auto relative top-0 z-[52] grid w-full focus:outline-0 border border-neutral-200 bg-white py-6 dark:shadow-lg shadow-sm duration-200 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[state=closed]:slide-out-to-left-1 data-[state=open]:slide-in-from-left-1 sm:rounded-lg md:w-full dark:border-nb-gray-900 dark:bg-nb-gray", + "mx-auto relative top-0 z-[52] grid w-full focus:outline-0 border border-nb-gray-100 bg-white py-6 dark:shadow-lg shadow-sm duration-200 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[state=closed]:slide-out-to-left-1 data-[state=open]:slide-in-from-left-1 sm:rounded-lg md:w-full dark:border-nb-gray-900 dark:bg-nb-gray",🤖 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 `@src/components/modal/Modal.tsx` at line 73, The Modal content is still using the legacy border token "border-neutral-200"; update the class on the Modal component (in Modal.tsx where the ModalContent grid wrapper string is defined) to use the theme-aware token instead—replace "border-neutral-200" with the appropriate nb-gray token (e.g., "border-nb-gray-200") so light mode uses nb-gray like dark mode uses "dark:border-nb-gray-900".
🧹 Nitpick comments (3)
src/modules/dns/nameservers/table/NameserverNameCell.tsx (1)
14-14: 💤 Low valueBase text color left on
neutral-*while hover migrated tonb-gray.Only the hover state was switched to
hover:text-nb-gray-100; the base still usestext-neutral-500/dark:text-neutral-300. It renders fine in both themes, but for token consistency with the rest of this migration consider moving the base color onto thenb-grayramp too.🤖 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 `@src/modules/dns/nameservers/table/NameserverNameCell.tsx` at line 14, The class string in the NameserverNameCell component still uses neutral tokens for base text color while only hover uses nb-gray; update the class list in NameserverNameCell (the JSX element containing the className value "flex items-center gap-2 ...") to replace text-neutral-500 and dark:text-neutral-300 with the matching nb-gray tokens (e.g., text-nb-gray-300 / dark:text-nb-gray-300 or whatever nb-gray values your design system uses) so base and hover text colors are on the same nb-gray ramp for consistency.src/modules/control-center/nodes/SelectUserNode.tsx (1)
26-26: ⚡ Quick winUse the CSS variable for the handle color in
SelectUserNode(avoidresolvedThemebranching).
SelectUserNodecurrently branches onresolvedThemeto set the handlebackgroundColor; driving it from--nb-gray-800viargb(var(--nb-gray-800))removes theuseTheme()dependency and keeps the color tied to the active theme.♻️ Proposed change
-import { useTheme } from "next-themes"; import * as React from "react";export const SelectUserNode = ({ data, id }: UserNodeProps) => { - const { resolvedTheme } = useTheme(); const { data: users } = useFetchApi<User[]>("/users?service_user=false");height: 20, width: "1px", border: "none", - backgroundColor: resolvedTheme === "light" ? "`#dfe3e6`" : "`#3f444b`", + backgroundColor: "rgb(var(--nb-gray-800))", borderRadius: "0px 4px 4px 0px", right: -2,Consider aligning the same approach in
SelectGroupNode/SelectPeerNodeif they use the sameresolvedTheme/hex branching pattern.🤖 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 `@src/modules/control-center/nodes/SelectUserNode.tsx` at line 26, SelectUserNode currently uses useTheme()/resolvedTheme to branch the handle backgroundColor; remove the resolvedTheme dependency and set the handle style to use the CSS variable (e.g., backgroundColor: "rgb(var(--nb-gray-800))") so the handle color follows the active theme. Update the handle styling in the SelectUserNode component (where handle backgroundColor is set) to use the CSS variable string instead of conditional hex values and remove the useTheme import/usage. Also scan SelectGroupNode and SelectPeerNode for the same resolvedTheme/hex branching pattern and apply the same CSS-variable replacement to keep all node handles theme-driven.src/modules/remote-access/ssh/Terminal.tsx (1)
173-179: ⚡ Quick winxterm.js runtime theme toggle repaints existing content—no
refresh()needed.
In xterm.js 5.5.x, updatingterminal.options.themeapplies via internal renderer/color-manager updates and repaints the current buffer automatically, so an explicitterminal.refresh(…)call is unnecessary. EnsureterminalThemeis assigned as a new object reference on each toggle (avoid mutating the same theme object in place).🤖 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 `@src/modules/remote-access/ssh/Terminal.tsx` around lines 173 - 179, The useEffect that assigns terminal.options.theme should simply set the new theme object and not call terminal.refresh; update the code in the effect that reads terminalInstanceRef.current?.terminal and assigns terminal.options.theme = terminalTheme (keep as-is) and remove any explicit terminal.refresh calls; ensure wherever you toggle/update terminalTheme you create a new object reference (e.g., via spreading or constructing a new object) instead of mutating the existing theme so xterm.js detects the change and repaints automatically.
🤖 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 `@src/app/`(dashboard)/peer/page.tsx:
- Line 257: The class string for the icon/button still uses legacy tokens
"dark:text-neutral-300 text-neutral-500"; update that class value (the string
starting with "flex h-8 w-8 items-center justify-center gap-2 ...") to use the
theme-aware nb tokens, e.g. replace "dark:text-neutral-300 text-neutral-500"
with "text-nb-gray-400" (keeping the existing "hover:text-nb-gray-100" and other
classes intact) so it matches the pattern used elsewhere like the example with
"text-nb-gray-400 hover:text-nb-gray-100".
In `@src/components/modal/Modal.tsx`:
- Line 138: The light-mode border token is still using border-neutral-200;
update SidebarModalContent (and ModalContent if present) to use the theme-aware
token (e.g., replace border-neutral-200 with border-nb-gray-200) in the class
string so light-mode styling matches the nb-gray tokens (also scan for any other
occurrences of border-neutral-200 in these components and replace them
accordingly).
In `@src/components/PinCodeInput.tsx`:
- Around line 111-112: In PinCodeInput.tsx there’s an incomplete token
migration: replace the old neutral token in the placeholder class under the
PinCodeInput component (the class string containing
"dark:placeholder:text-neutral-400/70") with the corresponding nb-gray token
(e.g., "dark:placeholder:text-nb-gray-400/70") so the placeholder uses the
nb-gray-* palette consistently with the other migrated classes.
In `@src/components/table/Table.tsx`:
- Line 131: TableHead currently mixes a static tailwind token by using
"text-neutral-500 dark:text-nb-gray-400 bg-nb-gray-900"; update the light-mode
text token to the theme-aware nb-gray family so both modes use nb-gray tokens.
Replace "text-neutral-500" with the appropriate nb-gray token (for example
"text-nb-gray-500") so the class becomes "text-nb-gray-500 dark:text-nb-gray-400
bg-nb-gray-900" in the TableHead rendering (locate the TableHead JSX / className
string in Table.tsx).
In `@src/modules/common-table-rows/LastTimeRow.tsx`:
- Line 29: The component LastTimeRow still mixes old neutral tokens with the new
nb tokens: update the root class string (currently containing
"dark:text-neutral-300 text-neutral-500 hover:text-nb-gray-100") to use the
corresponding nb tokens (e.g., replace dark:text-neutral-300 →
dark:text-nb-gray-200 and text-neutral-500 → text-nb-gray-300) and update the
tooltip text classes (the tooltip content lines currently using text-neutral-300
and text-neutral-200) to their nb equivalents (e.g., text-nb-gray-200 and
text-nb-gray-100) so all text color classes in the LastTimeRow component
consistently use the nb-* tokens.
In `@src/modules/dns/zones/records/DNSRecordTimeToLiveCell.tsx`:
- Line 14: In DNSRecordTimeToLiveCell, update the class string that currently
uses "dark:text-neutral-300 text-neutral-500" so it no longer uses any dark:
prefix or text-neutral-* tokens; replace them with the appropriate
"text-nb-gray-*" token (choosing a shade that matches the intended contrast for
the cell) while keeping the existing hover state "hover:text-nb-gray-100";
modify the class on the element that contains the string "flex items-center
whitespace-nowrap gap-2 dark:text-neutral-300 text-neutral-500
hover:text-nb-gray-100 transition-all py-2 px-3 rounded-md" to remove dark: and
neutral tokens and use the unified text-nb-gray-* token (e.g., text-nb-gray-200
or other selected shade).
In `@src/modules/groups/AssignPeerToGroupModal.tsx`:
- Line 234: In AssignPeerToGroupModal update the static text color classes in
the JSX location where the class string contains "flex items-center gap-2
dark:text-neutral-300 text-neutral-500 hover:text-nb-gray-100 ..." to use
theme-aware nb-gray tokens for both base and dark states (e.g., replace
dark:text-neutral-300 and text-neutral-500 with the appropriate nb-gray-*
equivalents) so all text states are consistent with the migrated hover token;
ensure you update the same class string used in the component's render/return so
hover, base, and dark text colors all use nb-gray tokens.
In `@src/modules/networks/misc/NetworkInformationSquare.tsx`:
- Around line 23-25: Replace the neutral tokens in the NetworkInformationSquare
component's class string so the base text color uses the nb-gray palette;
specifically, in the JSX where the class list is built (the fragment containing
"flex w-full items-center..." and the conditional using onClick), replace
"dark:text-neutral-300 text-neutral-500" with "text-nb-gray-500
dark:text-nb-gray-300" so the base text color matches the hover state tokens
(hover:text-nb-gray-100 / hover:bg-nb-gray-900/60) and remains consistent across
the NetworkInformationSquare component and its onClick variant.
In `@src/modules/networks/resources/ResourceNameCell.tsx`:
- Around line 38-39: The base text color in ResourceNameCell.tsx still uses the
old token "text-neutral-300"; update it to the theme token "text-nb-gray-300" to
match the hover state and complete the color token migration—locate the
className array/strings in ResourceNameCell (the JSX element where "flex
flex-col gap-0 text-neutral-300 font-light truncate" is defined) and replace
"text-neutral-300" with "text-nb-gray-300".
In `@src/modules/peers/PeerLastSeenCell.tsx`:
- Line 15: The class string in PeerLastSeenCell.tsx still uses static tokens
"dark:text-neutral-300 text-neutral-500" while hover was migrated; update the
JSX element's className (the string "flex items-center whitespace-nowrap gap-2
dark:text-neutral-300 text-neutral-500 hover:text-nb-gray-100 transition-all
py-2 px-3 rounded-md") to use theme-aware tokens instead (e.g., replace the
neutral tokens with matching nb-gray tokens such as "text-nb-gray-500
dark:text-nb-gray-300" while keeping hover:text-nb-gray-100), ensuring the
element's className reflects consistent theme token usage.
In `@src/modules/peers/PeerNameCell.tsx`:
- Around line 33-35: In PeerNameCell.tsx update the static color classes to the
theme-aware tokens: replace "dark:text-neutral-300 text-neutral-500" in the
className string with the corresponding nb tokens (e.g. "text-nb-gray-300
dark:text-nb-gray-300") so the base text color uses the same theme token family
as the migrated hover state ("hover:text-nb-gray-100"); ensure the change is
made in the class concatenation where linkToPeer is applied.
In `@src/modules/peers/PeerOSCell.tsx`:
- Line 29: Replace the hard-coded neutral color classes in the className string
inside PeerOSCell (the JSX line containing "flex items-center gap-2 ...") with
the theme-aware tokens to match the hover token already migrated; specifically
change "dark:text-neutral-300 text-neutral-500" to their nb-gray equivalents
(e.g., "dark:text-nb-gray-300 text-nb-gray-500") so the element uses consistent
theme tokens across base, dark, and hover states.
In `@src/modules/reverse-proxy/events/ReverseProxyEventsTimeCell.tsx`:
- Line 22: ReverseProxyEventsTimeCell uses old neutral tokens
("dark:text-neutral-300 text-neutral-500") alongside the migrated hover token
"hover:text-nb-gray-100", causing inconsistent theming; update the component's
className in ReverseProxyEventsTimeCell to replace the legacy neutral tokens
with the theme-aware token "text-nb-gray-300" so the base and hover states use
the same new token set and the component responds to light/dark themes
consistently.
---
Outside diff comments:
In `@src/app/`(dashboard)/team/user/page.tsx:
- Around line 177-200: The text color set by the className "text-nb-gray-100" is
being overridden by the inline style color in the div that uses isServiceUser
and generateColorFromString (the block around the JSX div with className and
style props); either remove the color property from the inline style object so
the class token applies (keep the conditional shape but drop color), or if the
generated per-user color is intentional, revert removing the className so you
don't claim nb-gray-100; update the JSX around isServiceUser,
generateColorFromString, and the style prop accordingly.
In `@src/components/Code.tsx`:
- Around line 43-44: In the Code component in src/components/Code.tsx update the
class strings in the ternary that currently use "bg-gray-50" and
"border-neutral-200" to use the nb-gray palette instead (e.g., replace with
"bg-nb-gray-50" and "border-nb-gray-200" or your project's nb-gray equivalents)
so they match the existing dark-mode tokens (like dark:border-nb-gray-800,
dark:bg-nb-gray-930) and keep both branches consistent with the nb-gray theme
tokens.
In `@src/components/modal/Modal.tsx`:
- Line 73: The Modal content is still using the legacy border token
"border-neutral-200"; update the class on the Modal component (in Modal.tsx
where the ModalContent grid wrapper string is defined) to use the theme-aware
token instead—replace "border-neutral-200" with the appropriate nb-gray token
(e.g., "border-nb-gray-200") so light mode uses nb-gray like dark mode uses
"dark:border-nb-gray-900".
In `@src/layouts/Header.tsx`:
- Line 92: In the Header component's nav-collapse button class string (the one
containing "h-10 w-10 ... text-nb-gray-300"), remove the hardcoded
"hover:text-white" and replace it with the theme-aware hover token used by our
design system (e.g., "hover:text-nb-gray-50" or the equivalent theme foreground
token), so the hover icon color follows light/dark ramps instead of becoming
invisible in light mode.
In `@src/modules/peers/PeerAddressCell.tsx`:
- Line 57: The parent container div in PeerAddressCell.tsx still uses the old
classes "dark:text-neutral-300 text-neutral-500"; update that container's
className (the div with "flex flex-col gap-0 ... truncate") to use the migrated
token "text-nb-gray-400" instead of those static neutral classes so it matches
the IP text migration while keeping the other classes (e.g., "flex flex-col
gap-0 font-light truncate") intact.
---
Nitpick comments:
In `@src/modules/control-center/nodes/SelectUserNode.tsx`:
- Line 26: SelectUserNode currently uses useTheme()/resolvedTheme to branch the
handle backgroundColor; remove the resolvedTheme dependency and set the handle
style to use the CSS variable (e.g., backgroundColor: "rgb(var(--nb-gray-800))")
so the handle color follows the active theme. Update the handle styling in the
SelectUserNode component (where handle backgroundColor is set) to use the CSS
variable string instead of conditional hex values and remove the useTheme
import/usage. Also scan SelectGroupNode and SelectPeerNode for the same
resolvedTheme/hex branching pattern and apply the same CSS-variable replacement
to keep all node handles theme-driven.
In `@src/modules/dns/nameservers/table/NameserverNameCell.tsx`:
- Line 14: The class string in the NameserverNameCell component still uses
neutral tokens for base text color while only hover uses nb-gray; update the
class list in NameserverNameCell (the JSX element containing the className value
"flex items-center gap-2 ...") to replace text-neutral-500 and
dark:text-neutral-300 with the matching nb-gray tokens (e.g., text-nb-gray-300 /
dark:text-nb-gray-300 or whatever nb-gray values your design system uses) so
base and hover text colors are on the same nb-gray ramp for consistency.
In `@src/modules/remote-access/ssh/Terminal.tsx`:
- Around line 173-179: The useEffect that assigns terminal.options.theme should
simply set the new theme object and not call terminal.refresh; update the code
in the effect that reads terminalInstanceRef.current?.terminal and assigns
terminal.options.theme = terminalTheme (keep as-is) and remove any explicit
terminal.refresh calls; ensure wherever you toggle/update terminalTheme you
create a new object reference (e.g., via spreading or constructing a new object)
instead of mutating the existing theme so xterm.js detects the change and
repaints automatically.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e8a53ea7-d454-44ce-841f-1cb5b5dcf4ca
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/assets/netbird-full-light.svgis excluded by!**/*.svg
📒 Files selected for processing (67)
src/app/(dashboard)/group/page.tsxsrc/app/(dashboard)/peer/page.tsxsrc/app/(dashboard)/team/user/page.tsxsrc/app/globals.csssrc/app/invite/page.tsxsrc/components/Button.tsxsrc/components/Code.tsxsrc/components/DatePickerWithRange.tsxsrc/components/FullTooltip.tsxsrc/components/HelpText.tsxsrc/components/InlineLink.tsxsrc/components/Input.tsxsrc/components/Label.tsxsrc/components/NetBirdLogo.tsxsrc/components/NetworkRouteSelector.tsxsrc/components/Notification.tsxsrc/components/PeerSelector.tsxsrc/components/PinCodeInput.tsxsrc/components/PortSelector.tsxsrc/components/Separator.tsxsrc/components/Tooltip.tsxsrc/components/modal/Modal.tsxsrc/components/table/DataTableFilter.tsxsrc/components/table/DataTableMultiSelectPopup.tsxsrc/components/table/DataTablePagination.tsxsrc/components/table/DataTableRowsPerPage.tsxsrc/components/table/Table.tsxsrc/components/ui/DarkModeToggle.tsxsrc/components/ui/HelpAndSupportButton.tsxsrc/contexts/GlobalThemeProvider.tsxsrc/layouts/AppLayout.tsxsrc/layouts/Header.tsxsrc/modules/access-control/AccessControlModal.tsxsrc/modules/common-table-rows/LastTimeRow.tsxsrc/modules/control-center/nodes/SelectGroupNode.tsxsrc/modules/control-center/nodes/SelectPeerNode.tsxsrc/modules/control-center/nodes/SelectUserNode.tsxsrc/modules/dns/nameservers/table/NameserverNameCell.tsxsrc/modules/dns/zones/records/DNSRecordTimeToLiveCell.tsxsrc/modules/groups/AssignPeerToGroupModal.tsxsrc/modules/instance-setup/InstanceSetupWizard.tsxsrc/modules/networks/misc/NetworkInformationSquare.tsxsrc/modules/networks/resources/ResourceNameCell.tsxsrc/modules/onboarding/networks/OnboardingTestResource.tsxsrc/modules/onboarding/p2p/OnboardingTestP2P.tsxsrc/modules/peer/AddRouteDropdownButton.tsxsrc/modules/peer/RemoteJobDropdownButton.tsxsrc/modules/peers/PeerAddressCell.tsxsrc/modules/peers/PeerConnectButton.tsxsrc/modules/peers/PeerLastSeenCell.tsxsrc/modules/peers/PeerMultiSelect.tsxsrc/modules/peers/PeerNameCell.tsxsrc/modules/peers/PeerOSCell.tsxsrc/modules/peers/PeerVersionCell.tsxsrc/modules/posture-checks/ui/PostureCheckIcons.tsxsrc/modules/remote-access/rdp/RDPCertificateModal.tsxsrc/modules/remote-access/ssh/Terminal.tsxsrc/modules/reverse-proxy/ReverseProxyCrowdSecIPReputation.tsxsrc/modules/reverse-proxy/clusters/ClusterTypeIndicator.tsxsrc/modules/reverse-proxy/clusters/ClustersFeaturesCell.tsxsrc/modules/reverse-proxy/events/ReverseProxyEventsTimeCell.tsxsrc/modules/reverse-proxy/targets/ReverseProxyTargetModal.tsxsrc/modules/reverse-proxy/targets/ReverseProxyTargetSelector.tsxsrc/modules/settings/ClientSettingsTab.tsxsrc/modules/users/table-cells/ServiceUserNameCell.tsxsrc/modules/users/table-cells/UserStatusCell.tsxtailwind.config.ts
TechHutTV
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/components/table/filters/StatusPicker.tsx`:
- Around line 35-45: The buttons rendered in StatusPicker.tsx default to
type="submit" which can cause accidental form submissions; update the button
elements inside the options loop (the elements that call onClick ->
onChange(option.value) and close()) to include an explicit type="button"
attribute so clicking a status only triggers the picker logic (onChange/close)
and never submits a parent form.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ba1620cf-0bdf-45d9-9cb4-8d3382918ae2
📒 Files selected for processing (67)
.gitignoresrc/app/(dashboard)/control-center/page.tsxsrc/app/(dashboard)/group/page.tsxsrc/app/(dashboard)/peer/page.tsxsrc/components/Badge.tsxsrc/components/Callout.tsxsrc/components/FullTooltip.tsxsrc/components/Notification.tsxsrc/components/Paragraph.tsxsrc/components/PinCodeInput.tsxsrc/components/SquareIcon.tsxsrc/components/Steps.tsxsrc/components/Tabs.tsxsrc/components/ToggleSwitch.tsxsrc/components/modal/Modal.tsxsrc/components/table/DataTableHeader.tsxsrc/components/table/DataTableRefreshButton.tsxsrc/components/table/DataTableResetFilterButton.tsxsrc/components/table/Table.tsxsrc/components/table/TableFilters.tsxsrc/components/table/filters/CheckboxListPicker.tsxsrc/components/table/filters/RadioPicker.tsxsrc/components/table/filters/StatusPicker.tsxsrc/components/ui/AnnouncementBanner.tsxsrc/components/ui/GetStartedTest.tsxsrc/components/ui/GroupBadgeWithEditPeers.tsxsrc/components/ui/LoginExpiredBadge.tsxsrc/components/ui/TruncatedText.tsxsrc/contexts/GlobalThemeProvider.tsxsrc/modules/access-control/table/AccessControlDirectionCell.tsxsrc/modules/activity/ActivityEntryRow.tsxsrc/modules/activity/UsersDropdownSelector.tsxsrc/modules/common-table-rows/ActiveInactiveRow.tsxsrc/modules/common-table-rows/ExpirationDateRow.tsxsrc/modules/common-table-rows/LastTimeRow.tsxsrc/modules/control-center/edges/DirectionIn.tsxsrc/modules/control-center/edges/SimpleConnection.tsxsrc/modules/control-center/nodes/SelectUserNode.tsxsrc/modules/control-center/user/ControlCenterCurrentUserBadge.tsxsrc/modules/dns/nameservers/table/NameserverNameCell.tsxsrc/modules/dns/zones/records/DNSRecordContentCell.tsxsrc/modules/dns/zones/records/DNSRecordNameCell.tsxsrc/modules/dns/zones/records/DNSRecordTimeToLiveCell.tsxsrc/modules/groups/AssignPeerToGroupModal.tsxsrc/modules/groups/table/GroupsNameCell.tsxsrc/modules/networks/misc/NetworkInformationSquare.tsxsrc/modules/networks/resources/NetworkResourceModal.tsxsrc/modules/networks/resources/ResourceNameCell.tsxsrc/modules/peers/PeerAddressCell.tsxsrc/modules/peers/PeerLastSeenCell.tsxsrc/modules/peers/PeerNameCell.tsxsrc/modules/peers/PeerOSCell.tsxsrc/modules/peers/PeerVersionCell.tsxsrc/modules/posture-checks/checks/tooltips/GeoLocationTooltip.tsxsrc/modules/posture-checks/checks/tooltips/NetBirdVersionTooltip.tsxsrc/modules/posture-checks/checks/tooltips/PeerNetworkRangeTooltip.tsxsrc/modules/posture-checks/checks/tooltips/ProcessTooltip.tsxsrc/modules/posture-checks/ui/PostureCheckCard.tsxsrc/modules/reverse-proxy/events/ReverseProxyEventsRequestCell.tsxsrc/modules/reverse-proxy/events/ReverseProxyEventsTimeCell.tsxsrc/modules/reverse-proxy/events/ReverseProxyEventsUserCell.tsxsrc/modules/reverse-proxy/table/ReverseProxyDestinationCell.tsxsrc/modules/settings/DangerZoneTab.tsxsrc/modules/settings/GroupsSettings.tsxsrc/modules/users/UserInvitesTable.tsxsrc/modules/users/table-cells/UserNameCell.tsxsrc/modules/users/table-cells/UserStatusCell.tsx
✅ Files skipped from review due to trivial changes (43)
- src/components/Paragraph.tsx
- src/modules/dns/zones/records/DNSRecordContentCell.tsx
- src/components/ui/LoginExpiredBadge.tsx
- src/modules/dns/zones/records/DNSRecordNameCell.tsx
- src/modules/reverse-proxy/events/ReverseProxyEventsRequestCell.tsx
- src/components/table/DataTableResetFilterButton.tsx
- src/components/ui/TruncatedText.tsx
- src/modules/posture-checks/checks/tooltips/PeerNetworkRangeTooltip.tsx
- src/modules/reverse-proxy/table/ReverseProxyDestinationCell.tsx
- src/components/table/DataTableHeader.tsx
- src/modules/dns/nameservers/table/NameserverNameCell.tsx
- src/modules/groups/table/GroupsNameCell.tsx
- src/modules/common-table-rows/ExpirationDateRow.tsx
- src/components/ui/GetStartedTest.tsx
- src/app/(dashboard)/group/page.tsx
- src/components/table/TableFilters.tsx
- src/modules/common-table-rows/ActiveInactiveRow.tsx
- src/components/table/filters/RadioPicker.tsx
- src/modules/posture-checks/checks/tooltips/GeoLocationTooltip.tsx
- src/components/ToggleSwitch.tsx
- src/modules/activity/UsersDropdownSelector.tsx
- src/app/(dashboard)/peer/page.tsx
- src/modules/peers/PeerLastSeenCell.tsx
- src/modules/users/table-cells/UserStatusCell.tsx
- src/components/table/filters/CheckboxListPicker.tsx
- src/modules/posture-checks/checks/tooltips/NetBirdVersionTooltip.tsx
- src/modules/posture-checks/checks/tooltips/ProcessTooltip.tsx
- src/components/table/DataTableRefreshButton.tsx
- src/modules/control-center/user/ControlCenterCurrentUserBadge.tsx
- src/modules/users/UserInvitesTable.tsx
- src/modules/settings/DangerZoneTab.tsx
- src/modules/networks/resources/NetworkResourceModal.tsx
- src/modules/peers/PeerVersionCell.tsx
- src/components/PinCodeInput.tsx
- src/modules/reverse-proxy/events/ReverseProxyEventsUserCell.tsx
- src/components/table/Table.tsx
- src/modules/peers/PeerNameCell.tsx
- src/modules/reverse-proxy/events/ReverseProxyEventsTimeCell.tsx
- src/components/Tabs.tsx
- src/components/Notification.tsx
- src/components/Steps.tsx
- src/modules/peers/PeerOSCell.tsx
- src/components/modal/Modal.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- src/modules/groups/AssignPeerToGroupModal.tsx
- src/modules/networks/misc/NetworkInformationSquare.tsx
- src/components/FullTooltip.tsx
- src/modules/dns/zones/records/DNSRecordTimeToLiveCell.tsx
- src/modules/peers/PeerAddressCell.tsx
- src/modules/control-center/nodes/SelectUserNode.tsx
- src/contexts/GlobalThemeProvider.tsx
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 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 `@src/components/table/filters/StatusPicker.tsx`:
- Around line 35-45: The buttons rendered in StatusPicker.tsx default to
type="submit" which can cause accidental form submissions; update the button
elements inside the options loop (the elements that call onClick ->
onChange(option.value) and close()) to include an explicit type="button"
attribute so clicking a status only triggers the picker logic (onChange/close)
and never submits a parent form.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ba1620cf-0bdf-45d9-9cb4-8d3382918ae2
📒 Files selected for processing (67)
.gitignoresrc/app/(dashboard)/control-center/page.tsxsrc/app/(dashboard)/group/page.tsxsrc/app/(dashboard)/peer/page.tsxsrc/components/Badge.tsxsrc/components/Callout.tsxsrc/components/FullTooltip.tsxsrc/components/Notification.tsxsrc/components/Paragraph.tsxsrc/components/PinCodeInput.tsxsrc/components/SquareIcon.tsxsrc/components/Steps.tsxsrc/components/Tabs.tsxsrc/components/ToggleSwitch.tsxsrc/components/modal/Modal.tsxsrc/components/table/DataTableHeader.tsxsrc/components/table/DataTableRefreshButton.tsxsrc/components/table/DataTableResetFilterButton.tsxsrc/components/table/Table.tsxsrc/components/table/TableFilters.tsxsrc/components/table/filters/CheckboxListPicker.tsxsrc/components/table/filters/RadioPicker.tsxsrc/components/table/filters/StatusPicker.tsxsrc/components/ui/AnnouncementBanner.tsxsrc/components/ui/GetStartedTest.tsxsrc/components/ui/GroupBadgeWithEditPeers.tsxsrc/components/ui/LoginExpiredBadge.tsxsrc/components/ui/TruncatedText.tsxsrc/contexts/GlobalThemeProvider.tsxsrc/modules/access-control/table/AccessControlDirectionCell.tsxsrc/modules/activity/ActivityEntryRow.tsxsrc/modules/activity/UsersDropdownSelector.tsxsrc/modules/common-table-rows/ActiveInactiveRow.tsxsrc/modules/common-table-rows/ExpirationDateRow.tsxsrc/modules/common-table-rows/LastTimeRow.tsxsrc/modules/control-center/edges/DirectionIn.tsxsrc/modules/control-center/edges/SimpleConnection.tsxsrc/modules/control-center/nodes/SelectUserNode.tsxsrc/modules/control-center/user/ControlCenterCurrentUserBadge.tsxsrc/modules/dns/nameservers/table/NameserverNameCell.tsxsrc/modules/dns/zones/records/DNSRecordContentCell.tsxsrc/modules/dns/zones/records/DNSRecordNameCell.tsxsrc/modules/dns/zones/records/DNSRecordTimeToLiveCell.tsxsrc/modules/groups/AssignPeerToGroupModal.tsxsrc/modules/groups/table/GroupsNameCell.tsxsrc/modules/networks/misc/NetworkInformationSquare.tsxsrc/modules/networks/resources/NetworkResourceModal.tsxsrc/modules/networks/resources/ResourceNameCell.tsxsrc/modules/peers/PeerAddressCell.tsxsrc/modules/peers/PeerLastSeenCell.tsxsrc/modules/peers/PeerNameCell.tsxsrc/modules/peers/PeerOSCell.tsxsrc/modules/peers/PeerVersionCell.tsxsrc/modules/posture-checks/checks/tooltips/GeoLocationTooltip.tsxsrc/modules/posture-checks/checks/tooltips/NetBirdVersionTooltip.tsxsrc/modules/posture-checks/checks/tooltips/PeerNetworkRangeTooltip.tsxsrc/modules/posture-checks/checks/tooltips/ProcessTooltip.tsxsrc/modules/posture-checks/ui/PostureCheckCard.tsxsrc/modules/reverse-proxy/events/ReverseProxyEventsRequestCell.tsxsrc/modules/reverse-proxy/events/ReverseProxyEventsTimeCell.tsxsrc/modules/reverse-proxy/events/ReverseProxyEventsUserCell.tsxsrc/modules/reverse-proxy/table/ReverseProxyDestinationCell.tsxsrc/modules/settings/DangerZoneTab.tsxsrc/modules/settings/GroupsSettings.tsxsrc/modules/users/UserInvitesTable.tsxsrc/modules/users/table-cells/UserNameCell.tsxsrc/modules/users/table-cells/UserStatusCell.tsx
✅ Files skipped from review due to trivial changes (43)
- src/components/Paragraph.tsx
- src/modules/dns/zones/records/DNSRecordContentCell.tsx
- src/components/ui/LoginExpiredBadge.tsx
- src/modules/dns/zones/records/DNSRecordNameCell.tsx
- src/modules/reverse-proxy/events/ReverseProxyEventsRequestCell.tsx
- src/components/table/DataTableResetFilterButton.tsx
- src/components/ui/TruncatedText.tsx
- src/modules/posture-checks/checks/tooltips/PeerNetworkRangeTooltip.tsx
- src/modules/reverse-proxy/table/ReverseProxyDestinationCell.tsx
- src/components/table/DataTableHeader.tsx
- src/modules/dns/nameservers/table/NameserverNameCell.tsx
- src/modules/groups/table/GroupsNameCell.tsx
- src/modules/common-table-rows/ExpirationDateRow.tsx
- src/components/ui/GetStartedTest.tsx
- src/app/(dashboard)/group/page.tsx
- src/components/table/TableFilters.tsx
- src/modules/common-table-rows/ActiveInactiveRow.tsx
- src/components/table/filters/RadioPicker.tsx
- src/modules/posture-checks/checks/tooltips/GeoLocationTooltip.tsx
- src/components/ToggleSwitch.tsx
- src/modules/activity/UsersDropdownSelector.tsx
- src/app/(dashboard)/peer/page.tsx
- src/modules/peers/PeerLastSeenCell.tsx
- src/modules/users/table-cells/UserStatusCell.tsx
- src/components/table/filters/CheckboxListPicker.tsx
- src/modules/posture-checks/checks/tooltips/NetBirdVersionTooltip.tsx
- src/modules/posture-checks/checks/tooltips/ProcessTooltip.tsx
- src/components/table/DataTableRefreshButton.tsx
- src/modules/control-center/user/ControlCenterCurrentUserBadge.tsx
- src/modules/users/UserInvitesTable.tsx
- src/modules/settings/DangerZoneTab.tsx
- src/modules/networks/resources/NetworkResourceModal.tsx
- src/modules/peers/PeerVersionCell.tsx
- src/components/PinCodeInput.tsx
- src/modules/reverse-proxy/events/ReverseProxyEventsUserCell.tsx
- src/components/table/Table.tsx
- src/modules/peers/PeerNameCell.tsx
- src/modules/reverse-proxy/events/ReverseProxyEventsTimeCell.tsx
- src/components/Tabs.tsx
- src/components/Notification.tsx
- src/components/Steps.tsx
- src/modules/peers/PeerOSCell.tsx
- src/components/modal/Modal.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- src/modules/groups/AssignPeerToGroupModal.tsx
- src/modules/networks/misc/NetworkInformationSquare.tsx
- src/components/FullTooltip.tsx
- src/modules/dns/zones/records/DNSRecordTimeToLiveCell.tsx
- src/modules/peers/PeerAddressCell.tsx
- src/modules/control-center/nodes/SelectUserNode.tsx
- src/contexts/GlobalThemeProvider.tsx
🛑 Comments failed to post (1)
src/components/table/filters/StatusPicker.tsx (1)
35-45:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSet explicit button type to avoid accidental form submissions.
<button>defaults totype="submit". If this picker is rendered inside a form context, selecting a status can submit the form unexpectedly.Proposed fix
<button key={option.label} + type="button" className={cn( "flex items-center gap-2.5 px-2 py-1.5 rounded text-sm transition-colors", "text-nb-gray-300 hover:bg-nb-gray-900 hover:text-nb-gray-100", selected && "text-nb-gray-100", )}🤖 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 `@src/components/table/filters/StatusPicker.tsx` around lines 35 - 45, The buttons rendered in StatusPicker.tsx default to type="submit" which can cause accidental form submissions; update the button elements inside the options loop (the elements that call onClick -> onChange(option.value) and close()) to include an explicit type="button" attribute so clicking a status only triggers the picker logic (onChange/close) and never submits a parent form.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/contexts/GlobalThemeProvider.tsx (1)
25-26:⚠️ Potential issue | 🟠 MajorUse
--nb-gray-*values for skeleton colors (two match exactly, two drift).In
src/contexts/GlobalThemeProvider.tsx, lighthighlightColor(#f8fafb= 248 250 251) matches--nb-gray-940, and darkbaseColor(#25282d= 37 40 45) matches--nb-gray-920. LightbaseColor(#edeff1) and darkhighlightColor(#33373e) don’t match any--nb-gray-*value exactly (they’re slightly off from the nearest stops). Derive them from CSS vars to prevent theme drift, e.g.:baseColor={isLight ? "rgb(var(--nb-gray-900))" : "rgb(var(--nb-gray-920))"} highlightColor={isLight ? "rgb(var(--nb-gray-940))" : "rgb(var(--nb-gray-850))"}🤖 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 `@src/contexts/GlobalThemeProvider.tsx` around lines 25 - 26, Replace the hardcoded hex skeleton colors in GlobalThemeProvider (the baseColor/highlightColor ternaries using isLight) with CSS variable references so they exactly match the design tokens and avoid drift; set light baseColor to rgb(var(--nb-gray-900)), light highlightColor to rgb(var(--nb-gray-940)), dark baseColor to rgb(var(--nb-gray-920)) and dark highlightColor to rgb(var(--nb-gray-850)) so the baseColor/highlightColor branches use those rgb(var(--nb-gray-...)) vars instead of the current hex literals.
🧹 Nitpick comments (2)
src/contexts/GlobalThemeProvider.tsx (1)
14-18: 💤 Low valueClarify or remove misleading comment.
The comment states "Defaults to the dark palette before mount to match the dark default theme," but the component always uses the current
resolvedThemevalue from context. There is no explicit "before mount" default in this component—the palette is determined by whateverresolvedThemereturns on first render (which comes fromgetStoredTheme()or system preference).📝 Suggested comment revision
/** * Wraps the skeleton loader theme so its colors follow the active theme. * Uses `resolvedTheme` so the "system" option resolves to the real OS value. - * Defaults to the dark palette before mount to match the dark default theme. */Or revise to clarify:
/** * Wraps the skeleton loader theme so its colors follow the active theme. * Uses `resolvedTheme` so the "system" option resolves to the real OS value. + * On first render, falls back to the stored theme or "dark" default (via ThemeProvider). */🤖 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 `@src/contexts/GlobalThemeProvider.tsx` around lines 14 - 18, Update the top comment in GlobalThemeProvider.tsx to remove the inaccurate "before mount" claim and state that the skeleton loader palette follows the current resolvedTheme on first render; mention that resolvedTheme (from useTheme/getStoredTheme or system preference) determines the initial palette rather than an explicit dark default. Reference the component name GlobalThemeProvider and the resolvedTheme/getStoredTheme sources so the comment accurately reflects behavior.src/contexts/ThemeProvider.tsx (1)
98-98: ⚡ Quick winConsider guarding
useThemeagainst usage outside provider.The hook returns the default context value (with a no-op
setTheme) when called outsideThemeProvider. This silently fails instead of surfacing the integration bug. Consider throwing an error in development or logging a warning.♻️ Proposed guard for development
-export const useTheme = () => React.useContext(ThemeContext); +export const useTheme = () => { + const context = React.useContext(ThemeContext); + if (process.env.NODE_ENV !== "production" && context.setTheme === ThemeContext._currentValue.setTheme) { + console.warn("useTheme() called outside ThemeProvider. Theme changes will not persist."); + } + return context; +};Alternatively, throw an error to catch bugs earlier:
-export const useTheme = () => React.useContext(ThemeContext); +export const useTheme = () => { + const context = React.useContext(ThemeContext); + if (context === ThemeContext._currentValue) { + throw new Error("useTheme must be used within ThemeProvider"); + } + return context; +};🤖 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 `@src/contexts/ThemeProvider.tsx` at line 98, The useTheme hook returns a safe default when used outside ThemeProvider, which hides integration bugs—modify useTheme to guard against misuse by reading const ctx = React.useContext(ThemeContext) and, in development (e.g. process.env.NODE_ENV !== 'production'), throw a clear error (or at minimum console.warn) if ctx is missing or equals the default sentinel (inspect setTheme/no-op) so callers see an immediate failure; reference useTheme, ThemeContext, ThemeProvider and the default setTheme no-op when implementing the guard.
🤖 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 `@src/contexts/ThemeProvider.tsx`:
- Around line 42-55: The withTransitionsDisabled function risks leaking the
temporary <style> if apply() throws; wrap the call to apply() and the subsequent
getComputedStyle() in a try/finally so the style element created in
withTransitionsDisabled is always removed from document.head (use
document.head.contains(style) or guard removal) even when an exception occurs,
preserving the existing behavior of scheduling removal with setTimeout but
ensuring the immediate cleanup path in finally handles errors safely.
---
Outside diff comments:
In `@src/contexts/GlobalThemeProvider.tsx`:
- Around line 25-26: Replace the hardcoded hex skeleton colors in
GlobalThemeProvider (the baseColor/highlightColor ternaries using isLight) with
CSS variable references so they exactly match the design tokens and avoid drift;
set light baseColor to rgb(var(--nb-gray-900)), light highlightColor to
rgb(var(--nb-gray-940)), dark baseColor to rgb(var(--nb-gray-920)) and dark
highlightColor to rgb(var(--nb-gray-850)) so the baseColor/highlightColor
branches use those rgb(var(--nb-gray-...)) vars instead of the current hex
literals.
---
Nitpick comments:
In `@src/contexts/GlobalThemeProvider.tsx`:
- Around line 14-18: Update the top comment in GlobalThemeProvider.tsx to remove
the inaccurate "before mount" claim and state that the skeleton loader palette
follows the current resolvedTheme on first render; mention that resolvedTheme
(from useTheme/getStoredTheme or system preference) determines the initial
palette rather than an explicit dark default. Reference the component name
GlobalThemeProvider and the resolvedTheme/getStoredTheme sources so the comment
accurately reflects behavior.
In `@src/contexts/ThemeProvider.tsx`:
- Line 98: The useTheme hook returns a safe default when used outside
ThemeProvider, which hides integration bugs—modify useTheme to guard against
misuse by reading const ctx = React.useContext(ThemeContext) and, in development
(e.g. process.env.NODE_ENV !== 'production'), throw a clear error (or at minimum
console.warn) if ctx is missing or equals the default sentinel (inspect
setTheme/no-op) so callers see an immediate failure; reference useTheme,
ThemeContext, ThemeProvider and the default setTheme no-op when implementing the
guard.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 263b2c6b-9257-41b1-91eb-b62f8c1d2aa0
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
package.jsonsrc/app/(dashboard)/control-center/page.tsxsrc/components/ui/DarkModeToggle.tsxsrc/contexts/GlobalThemeProvider.tsxsrc/contexts/ThemeProvider.tsxsrc/layouts/AppLayout.tsxsrc/modules/control-center/edges/DirectionIn.tsxsrc/modules/control-center/edges/SimpleConnection.tsxsrc/modules/control-center/nodes/SelectGroupNode.tsxsrc/modules/control-center/nodes/SelectPeerNode.tsxsrc/modules/control-center/nodes/SelectUserNode.tsxsrc/modules/remote-access/ssh/Terminal.tsx
💤 Files with no reviewable changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (9)
- src/modules/control-center/edges/DirectionIn.tsx
- src/modules/control-center/nodes/SelectGroupNode.tsx
- src/app/(dashboard)/control-center/page.tsx
- src/modules/control-center/nodes/SelectPeerNode.tsx
- src/modules/control-center/nodes/SelectUserNode.tsx
- src/layouts/AppLayout.tsx
- src/modules/control-center/edges/SimpleConnection.tsx
- src/components/ui/DarkModeToggle.tsx
- src/modules/remote-access/ssh/Terminal.tsx
TechHutTV
force-pushed
the
ui-accessibility
branch
from
dcf236c to
a8275e5
Compare
TechHutTV
changed the title
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
1 participant
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
Adds an initial light theme to the dashboard, which is currently dark-only. The core idea is to make the
nb-graycolor scale theme-aware: instead of hard-coded hex values, the palette is driven by CSS variables that swap between a light ramp (:root) and the original dark ramp (.dark). Because existingnb-gray-*utility classes resolve through those variables, most of the UI follows the active theme without per-component changes.A working theme switcher (Light / Dark / System) is included, along with the supporting bits needed to make it feel right — system-preference resolution, themed loading skeletons, and a pre-paint guard to avoid a flash of the wrong theme on load.
Dark mode is intentionally unchanged — every fix adds a light-mode value (or a
dark:override) without altering existing dark-mode appearance.Changes
Theming core
nb-graytheme-aware via CSS variables (tailwind.config.ts+globals.css), preserving opacity modifiers (e.g.bg-nb-gray-900/80).:rootand the original dark ramp under.dark.Theme switching
Shared-component contrast pass
Systematic light-mode contrast/visibility fixes in shared components, so the fix propagates everywhere they're used:
BadgeandSquareIcon— all colored variants (red, green, sky, purple, yellow, netbird, …) made theme-aware (light pastel background + dark text) instead of dark-only boxes.Callout,Notification(toast) — light text that was invisible on light surfaces now switches per theme; success/error toast icons render with proper contrast.ToggleSwitch— enabled state is brand orange in light mode (was black).Tabs,Modal(footer + content borders),Steps— stalenb-gray-100-as-light dividers (which invert to near-black) replaced with correct light tokens.Paragraph, avatar/initials circles, the Settings Danger Zone, and empty-state list fillers — darkened/recolored for readable light-mode contrast.Control Center (React Flow)
colorModefrom the active theme instead of a hardcoded"dark".Assets
Issue ticket number and link
Documentation
Select exactly one:
Issue ticket number and link
#412 netbirdio/netbird#2918
Summary by CodeRabbit
New Features
Style
Bug Fixes