Conversation
- also fix some SSR issues
📝 WalkthroughWalkthroughSwitched project tooling and docs from npm to pnpm; added a responsive Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Browser
participant DOM as document.documentElement\n(data-theme)
participant Session as sessionStorage
participant MutationObs as MutationObserver
participant ThemeSync as ThemeSync
participant Iframe as Iframe(s)
User->>DOM: set/query theme (query param / user action)
DOM->>ThemeSync: ThemeSync reads data-theme
ThemeSync->>Session: persist normalized theme
ThemeSync->>Iframe: postMessage "theme-ready"
Note over MutationObs,ThemeSync: Observe data-theme and storage changes
User->>DOM: change data-theme attribute
DOM->>MutationObs: mutation triggers
MutationObs->>ThemeSync: syncThemeState()
ThemeSync->>Session: update sessionStorage
ThemeSync->>Iframe: postMessage "theme-changed"
Iframe->>ThemeSync: postMessage "SET_THEME"
ThemeSync->>DOM: apply setCurrentTheme (update data-theme)
DOM->>MutationObs: trigger (cycle continues)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
unraid-docs | fc1817d | Commit Preview URL Branch Preview URL |
Mar 20 2026, 09:52 PM |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/pr-lint.yml (1)
13-24:⚠️ Potential issue | 🟡 MinorFix step ordering: pnpm must be available before
actions/setup-noderuns caching.
actions/setup-node@v6withcache: pnpmrequires pnpm to be available on PATH when the action executes. Currently,corepack enableruns aftersetup-node, causing the cache step to fail.Move
corepack enablebeforesetup-node, or usepnpm/action-setupbeforesetup-node(the recommended approach per setup-node docs).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-lint.yml around lines 13 - 24, The workflow steps run in the wrong order causing the setup-node cache to fail because pnpm isn't on PATH; reorder the steps so "Enable Corepack" (the run: corepack enable step) executes before the actions/setup-node@v6 step, or replace the corepack step with the recommended pnpm/action-setup step and place that before actions/setup-node@v6, then keep the "Install dependencies" (pnpm install --frozen-lockfile) step after setup-node; update the job step order and references to actions/setup-node@v6, corepack enable, pnpm/action-setup, and pnpm install accordingly.
🧹 Nitpick comments (2)
src/theme/Layout/ThemeSync.tsx (1)
31-48: Consider using the modernStorageEventconstructor instead of deprecated API.
document.createEvent()andinitStorageEvent()are deprecated. The modern approach uses theStorageEventconstructor, which is supported in all current browsers and provides cleaner code.♻️ Suggested refactor using modern API
function dispatchStorageChangeEvent(key: string, oldValue: string | null, newValue: string | null): void { - if (typeof document === "undefined" || typeof window === "undefined") { + if (typeof window === "undefined") { return; } - const event = document.createEvent("StorageEvent"); - event.initStorageEvent( - "storage", - false, - false, - key, - oldValue, - newValue, - window.location.href, - window.localStorage, - ); + const event = new StorageEvent("storage", { + key, + oldValue, + newValue, + url: window.location.href, + storageArea: window.localStorage, + }); window.dispatchEvent(event); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/theme/Layout/ThemeSync.tsx` around lines 31 - 48, The dispatchStorageChangeEvent function uses the deprecated document.createEvent/initStorageEvent; replace that with the modern StorageEvent constructor: construct a new StorageEvent("storage", { key, oldValue, newValue, url: window.location.href, storageArea: window.localStorage, bubbles: false, cancelable: false }) and then call window.dispatchEvent on it; keep the existing environment guards (typeof document/window) and other parameters unchanged so the behavior remains the same.src/theme/Layout/Provider/index.tsx (1)
6-14: Add TypeScript types for the component props.The
childrenparameter lacks type annotation. For consistency with TypeScript best practices and to match the typed approach in other files (e.g.,ThemeSync.tsxuses explicitReactElement | null), consider adding proper typing.✏️ Suggested type annotation
+import type { ReactNode } from "react"; import React from "react"; import LayoutProvider from "@theme-original/Layout/Provider"; import { ThemeSync } from "../ThemeSync"; import { IframeNavigation } from "../IframeNavigation"; -export default function LayoutProviderWrapper({ children }) { +export default function LayoutProviderWrapper({ children }: { children: ReactNode }) { return ( <LayoutProvider> <ThemeSync /> <IframeNavigation /> {children} </LayoutProvider> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/theme/Layout/Provider/index.tsx` around lines 6 - 14, The LayoutProviderWrapper component is missing TypeScript props types; add an explicit props interface (e.g., type Props = { children?: React.ReactNode } or use children: React.ReactElement | null to match ThemeSync) and annotate the function signature (LayoutProviderWrapper({ children }: Props): JSX.Element or React.ReactElement | null) so the children prop is typed and the component return type is declared; update imports to include React types if needed and ensure the function uses the new Props type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@i18n/zh/docusaurus-plugin-content-docs/current/unraid-os/troubleshooting/licensing-faq.mdx`:
- Line 53: The iframe's title attribute on this Chinese (zh) MDX page is
currently English; update the iframe's title value in licensing-faq.mdx to a
Chinese/localized string (e.g., translate "Loom walkthrough" to Chinese) so
screen readers receive locale-consistent text — locate the iframe element in
this file and replace the title attribute with the localized Chinese text and
verify it matches the page locale and aria/screen-reader expectations.
In `@src/css/custom.css`:
- Around line 14-15: The Stylelint value-keyword-case error comes from unquoted,
non-generic font family identifiers in the fallback list; update the two custom
properties (--ifm-font-family-base and --ifm-heading-font-family) so that
multi-word or PascalCase family names are quoted (e.g., "BlinkMacSystemFont")
and ensure generic keywords remain lowercase (system-ui, -apple-system,
sans-serif); in short, quote any non-generic font names and keep the generic
tokens lowercase in both --ifm-font-family-base and --ifm-heading-font-family
declarations.
---
Outside diff comments:
In @.github/workflows/pr-lint.yml:
- Around line 13-24: The workflow steps run in the wrong order causing the
setup-node cache to fail because pnpm isn't on PATH; reorder the steps so
"Enable Corepack" (the run: corepack enable step) executes before the
actions/setup-node@v6 step, or replace the corepack step with the recommended
pnpm/action-setup step and place that before actions/setup-node@v6, then keep
the "Install dependencies" (pnpm install --frozen-lockfile) step after
setup-node; update the job step order and references to actions/setup-node@v6,
corepack enable, pnpm/action-setup, and pnpm install accordingly.
---
Nitpick comments:
In `@src/theme/Layout/Provider/index.tsx`:
- Around line 6-14: The LayoutProviderWrapper component is missing TypeScript
props types; add an explicit props interface (e.g., type Props = { children?:
React.ReactNode } or use children: React.ReactElement | null to match ThemeSync)
and annotate the function signature (LayoutProviderWrapper({ children }: Props):
JSX.Element or React.ReactElement | null) so the children prop is typed and the
component return type is declared; update imports to include React types if
needed and ensure the function uses the new Props type.
In `@src/theme/Layout/ThemeSync.tsx`:
- Around line 31-48: The dispatchStorageChangeEvent function uses the deprecated
document.createEvent/initStorageEvent; replace that with the modern StorageEvent
constructor: construct a new StorageEvent("storage", { key, oldValue, newValue,
url: window.location.href, storageArea: window.localStorage, bubbles: false,
cancelable: false }) and then call window.dispatchEvent on it; keep the existing
environment guards (typeof document/window) and other parameters unchanged so
the behavior remains the same.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 19e6ce38-cfe3-4397-82ed-6108be3d8856
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
.github/workflows/pr-lint.yml.gitignoreREADME.mddocs/unraid-os/getting-started/set-up-unraid/internal-boot-faq.mdxi18n/de/docusaurus-plugin-content-docs/current/unraid-os/troubleshooting/licensing-faq.mdxi18n/es/docusaurus-plugin-content-docs/current/unraid-os/troubleshooting/licensing-faq.mdxi18n/fr/docusaurus-plugin-content-docs/current/unraid-os/troubleshooting/licensing-faq.mdxi18n/zh/docusaurus-plugin-content-docs/current/unraid-os/troubleshooting/licensing-faq.mdxpackage.jsonpnpm-workspace.yamlsrc/components/ResponsiveEmbed/index.tsxsrc/components/ResponsiveEmbed/styles.module.csssrc/css/custom.csssrc/theme/Layout/Provider/index.tsxsrc/theme/Layout/ThemeSync.tsxsrc/theme/Layout/index.tsx
| </div> | ||
| <ResponsiveEmbed | ||
| src="https://www.loom.com/embed/3ceb40440240474aaa80a0b7e3e69cb2" | ||
| title="Loom walkthrough: Unraid licensing FAQ" |
There was a problem hiding this comment.
Localize the iframe title text for the zh page.
The current title is English, which is inconsistent with the page locale and screen-reader language flow.
Suggested change
- title="Loom walkthrough: Unraid licensing FAQ"
+ title="Loom 演示:Unraid 许可证常见问题解答"📝 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.
| title="Loom walkthrough: Unraid licensing FAQ" | |
| title="Loom 演示:Unraid 许可证常见问题解答" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@i18n/zh/docusaurus-plugin-content-docs/current/unraid-os/troubleshooting/licensing-faq.mdx`
at line 53, The iframe's title attribute on this Chinese (zh) MDX page is
currently English; update the iframe's title value in licensing-faq.mdx to a
Chinese/localized string (e.g., translate "Loom walkthrough" to Chinese) so
screen readers receive locale-consistent text — locate the iframe element in
this file and replace the title attribute with the localized Chinese text and
verify it matches the page locale and aria/screen-reader expectations.
src/css/custom.css
Outdated
| --ifm-font-family-base: "Figtree Variable", system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif; | ||
| --ifm-heading-font-family: "Figtree Variable", system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif; |
There was a problem hiding this comment.
Fix the Stylelint error on the fallback font token.
This currently fails value-keyword-case on both declarations and can block lint-gated checks.
Suggested change
- --ifm-font-family-base: "Figtree Variable", system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif;
- --ifm-heading-font-family: "Figtree Variable", system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif;
+ --ifm-font-family-base: "Figtree Variable", system-ui, -apple-system, "Segoe UI", sans-serif;
+ --ifm-heading-font-family: "Figtree Variable", system-ui, -apple-system, "Segoe UI", sans-serif;🧰 Tools
🪛 Stylelint (17.4.0)
[error] 14-14: Expected "BlinkMacSystemFont" to be "blinkmacsystemfont" (value-keyword-case)
(value-keyword-case)
[error] 15-15: Expected "BlinkMacSystemFont" to be "blinkmacsystemfont" (value-keyword-case)
(value-keyword-case)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/css/custom.css` around lines 14 - 15, The Stylelint value-keyword-case
error comes from unquoted, non-generic font family identifiers in the fallback
list; update the two custom properties (--ifm-font-family-base and
--ifm-heading-font-family) so that multi-word or PascalCase family names are
quoted (e.g., "BlinkMacSystemFont") and ensure generic keywords remain lowercase
(system-ui, -apple-system, sans-serif); in short, quote any non-generic font
names and keep the generic tokens lowercase in both --ifm-font-family-base and
--ifm-heading-font-family declarations.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/css/custom.css (1)
1-2: Consider string notation for the Typekit import to satisfy Stylelint.Stylelint's
import-notationrule expects string notation. If this blocks CI, update the import:-@import url("https://use.typekit.net/ymm4lqb.css"); +@import "https://use.typekit.net/ymm4lqb.css";Both are functionally equivalent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/css/custom.css` around lines 1 - 2, Stylelint wants string notation for imports: replace the Typekit line `@import` url("https://use.typekit.net/ymm4lqb.css"); with string notation `@import` "https://use.typekit.net/ymm4lqb.css"; (leave the `@import` "@fontsource-variable/atkinson-hyperlegible-next"; line as-is) so the rule `import-notation` is satisfied.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/css/custom.css`:
- Around line 1-2: Stylelint wants string notation for imports: replace the
Typekit line `@import` url("https://use.typekit.net/ymm4lqb.css"); with string
notation `@import` "https://use.typekit.net/ymm4lqb.css"; (leave the `@import`
"@fontsource-variable/atkinson-hyperlegible-next"; line as-is) so the rule
`import-notation` is satisfied.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0cfa92d4-11c3-4de5-9b68-6e538b125a06
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
.github/workflows/pr-lint.ymlpackage.jsonsrc/css/custom.css
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/pr-lint.yml
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/css/custom.css (1)
15-16:⚠️ Potential issue | 🟠 MajorResolve remaining
value-keyword-caseviolations in font fallbacks and text-rendering values.Line 16 repeats the previously flagged fallback-font casing issue, and Lines 44/88 introduce the same rule violation for
optimizeLegibility.Suggested fix
- --unraid-brand-font-family: "nudista-web", -apple-system, system-ui, "Segoe UI", Roboto, Helvetica, Arial, sans-serif; - --ifm-font-family-base: "Atkinson Hyperlegible Next Variable", system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif; + --unraid-brand-font-family: "nudista-web", -apple-system, system-ui, "Segoe UI", "Roboto", "Helvetica", "Arial", sans-serif; + --ifm-font-family-base: "Atkinson Hyperlegible Next Variable", system-ui, -apple-system, "BlinkMacSystemFont", "Segoe UI", sans-serif; @@ - text-rendering: optimizeLegibility; + text-rendering: optimizelegibility; @@ - text-rendering: optimizeLegibility; + text-rendering: optimizelegibility;Also applies to: 44-44, 88-88
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/css/custom.css` around lines 15 - 16, Normalize the value keyword casing: update the font-family fallbacks in --unraid-brand-font-family and --ifm-font-family-base so all generic/system keywords are lowercase (e.g., system-ui, -apple-system, sans-serif) and adjust any text-rendering declarations (the text-rendering property occurrences) to use the lowercase keyword variant expected by the linter (e.g., optimizelegibility if your lint rule enforces lowercase). Ensure only the generic keywords are lowercased and quoted font names keep their original capitalization/quotes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/css/custom.css`:
- Line 1: Replace the `@import` url(...) statement in custom.css with string
import notation to satisfy the Stylelint import-notation rule: locate the
`@import` line (the statement referencing "https://use.typekit.net/ymm4lqb.css")
and change it to use a quoted string form (e.g., `@import`
"https://use.typekit.net/ymm4lqb.css";) so the linter accepts the import.
---
Duplicate comments:
In `@src/css/custom.css`:
- Around line 15-16: Normalize the value keyword casing: update the font-family
fallbacks in --unraid-brand-font-family and --ifm-font-family-base so all
generic/system keywords are lowercase (e.g., system-ui, -apple-system,
sans-serif) and adjust any text-rendering declarations (the text-rendering
property occurrences) to use the lowercase keyword variant expected by the
linter (e.g., optimizelegibility if your lint rule enforces lowercase). Ensure
only the generic keywords are lowercased and quoted font names keep their
original capitalization/quotes.
Before Submitting This PR, Please Ensure You Have Completed The Following:
Summary by CodeRabbit
New Features
Documentation
Chores