feat(ui): sidebar navigation redesign and global style polish#768
feat(ui): sidebar navigation redesign and global style polish#768Serokz wants to merge 6 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRefactors the web UI: introduces a ChangesWeb UI Visual Refresh
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@web/src/index.jsx`:
- Around line 60-66: Guard the localStorage operations to prevent crashes when
storage is unavailable or contains malformed data. Wrap the
JSON.parse(localStorage.getItem('gm_nav_collapsed')) call in the useState
initialization with a try-catch block, defaulting to false if any error occurs
(storage unavailable, parse failure, etc.). Similarly, wrap the
localStorage.setItem call in the useEffect hook with a try-catch block to safely
handle any storage write errors without crashing the app. This ensures the nav
collapse state can gracefully degrade when storage operations fail.
In `@web/src/style.css`:
- Line 423: The stylelint configuration extends stylelint-config-standard-scss
which has the at-rule-no-unknown rule enabled by default, causing Tailwind v4
directives like `@theme`, `@source`, and `@custom-variant` to be flagged as unknown
at-rules and block CI builds. Update the at-rule-no-unknown rule in the
configuration to include an ignoreAtRules array that lists the Tailwind v4
at-rules (theme, source, custom-variant, apply, utility, variant), or
alternatively replace the current extended configuration with
stylelint-config-tailwindcss which provides built-in support for Tailwind v4
syntax.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e546be75-de64-49e6-b419-168c687c61d8
📒 Files selected for processing (10)
web/index.htmlweb/src/components/Card.jsxweb/src/components/Chart.jsxweb/src/components/GmLogoIcon.jsxweb/src/components/Navigation.jsxweb/src/components/Tooltip.jsxweb/src/components/VisualizerUploadModal.jsxweb/src/index.jsxweb/src/pages/Home/ModeTab.jsxweb/src/style.css
| const [navCollapsed, setNavCollapsed] = useState( | ||
| JSON.parse(localStorage.getItem('gm_nav_collapsed') || 'false'), | ||
| ); | ||
|
|
||
| useEffect(() => { | ||
| const storage = globalThis.window?.localStorage; | ||
| if (!storage) return; | ||
|
|
||
| try { | ||
| storage.setItem(DESKTOP_NAV_COLLAPSED_STORAGE_KEY, String(navCollapsed)); | ||
| } catch { | ||
| // Ignore storage write failures so the navigation still works in restricted browsers. | ||
| } | ||
| localStorage.setItem('gm_nav_collapsed', JSON.stringify(navCollapsed)); | ||
| }, [navCollapsed]); |
There was a problem hiding this comment.
Guard persisted nav-state reads/writes to prevent boot-time crashes.
Direct JSON.parse(localStorage.getItem(...)) and unguarded localStorage.setItem(...) can throw and take down the app when storage is unavailable or value is malformed.
Suggested fix
export default function App() {
- const [navCollapsed, setNavCollapsed] = useState(
- JSON.parse(localStorage.getItem('gm_nav_collapsed') || 'false'),
- );
+ const [navCollapsed, setNavCollapsed] = useState(() => {
+ try {
+ const raw = globalThis.window?.localStorage?.getItem('gm_nav_collapsed');
+ return raw ? JSON.parse(raw) === true : false;
+ } catch {
+ return false;
+ }
+ });
useEffect(() => {
- localStorage.setItem('gm_nav_collapsed', JSON.stringify(navCollapsed));
+ try {
+ globalThis.window?.localStorage?.setItem(
+ 'gm_nav_collapsed',
+ JSON.stringify(navCollapsed),
+ );
+ } catch {}
}, [navCollapsed]);📝 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 [navCollapsed, setNavCollapsed] = useState( | |
| JSON.parse(localStorage.getItem('gm_nav_collapsed') || 'false'), | |
| ); | |
| useEffect(() => { | |
| const storage = globalThis.window?.localStorage; | |
| if (!storage) return; | |
| try { | |
| storage.setItem(DESKTOP_NAV_COLLAPSED_STORAGE_KEY, String(navCollapsed)); | |
| } catch { | |
| // Ignore storage write failures so the navigation still works in restricted browsers. | |
| } | |
| localStorage.setItem('gm_nav_collapsed', JSON.stringify(navCollapsed)); | |
| }, [navCollapsed]); | |
| const [navCollapsed, setNavCollapsed] = useState(() => { | |
| try { | |
| const raw = globalThis.window?.localStorage?.getItem('gm_nav_collapsed'); | |
| return raw ? JSON.parse(raw) === true : false; | |
| } catch { | |
| return false; | |
| } | |
| }); | |
| useEffect(() => { | |
| try { | |
| globalThis.window?.localStorage?.setItem( | |
| 'gm_nav_collapsed', | |
| JSON.stringify(navCollapsed), | |
| ); | |
| } catch {} | |
| }, [navCollapsed]); |
🤖 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 `@web/src/index.jsx` around lines 60 - 66, Guard the localStorage operations to
prevent crashes when storage is unavailable or contains malformed data. Wrap the
JSON.parse(localStorage.getItem('gm_nav_collapsed')) call in the useState
initialization with a try-catch block, defaulting to false if any error occurs
(storage unavailable, parse failure, etc.). Similarly, wrap the
localStorage.setItem call in the useEffect hook with a try-catch block to safely
handle any storage write errors without crashing the app. This ensures the nav
collapse state can gracefully degrade when storage operations fail.
|
|
||
| @theme { | ||
| --font-logo: 'Montserrat', sans-serif; | ||
| --font-logo: 'Montserrat', system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, sans-serif; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify Tailwind v4 at-rule usage in CSS
rg -n "`@custom-variant`|`@plugin`|`@theme`|`@source`|--font-logo" web/src/style.css
# Inspect stylelint config files/rules for unknown at-rule handling
fd -i "stylelint" -t f
fd -i ".stylelintrc" -t f
rg -n "at-rule-no-unknown|scss/at-rule-no-unknown|ignoreAtRules|custom-variant|tailwind" --iglob "*stylelint*" --iglob ".stylelintrc*"Repository: jniebuhr/gaggimate
Length of output: 407
🏁 Script executed:
# Check package.json for stylelint and Tailwind configuration
cat package.json | head -100
# Search for all stylelint config files more broadly
find . -type f -name "*stylelint*" -o -name ".stylelintrc*" 2>/dev/null | head -20
# Check for stylelint config in common locations
test -f .stylelintrc && echo "Found .stylelintrc" || echo "No .stylelintrc"
test -f .stylelintrc.json && echo "Found .stylelintrc.json" || echo "No .stylelintrc.json"
test -f stylelint.config.js && echo "Found stylelint.config.js" || echo "No stylelint.config.js"
test -f stylelint.config.mjs && echo "Found stylelint.config.mjs" || echo "No stylelint.config.mjs"
# Check if Tailwind is installed and what version
rg '"tailwindcss"' package.json -A 1Repository: jniebuhr/gaggimate
Length of output: 285
🏁 Script executed:
# Find package.json
fd "package.json" -t f
# Read stylelint config
cat .stylelintrc.json
# Once we find package.json, check Tailwind version
fd "package.json" -t f -x head -50 {} \;Repository: jniebuhr/gaggimate
Length of output: 2674
🏁 Script executed:
# Check if there's a configuration for ignoreAtRules or at-rule-no-unknown in stylelint config
cat .stylelintrc.json | grep -E "at-rule|ignore|custom-variant|theme|source"
# Check if stylelint-config-standard-scss is mentioned elsewhere
rg "stylelint-config-standard-scss" -t json
# Search web/src/style.css for the actual lines to verify context
sed -n '420,430p' web/src/style.css
sed -n '467,471p' web/src/style.css
# Check if there's a tailwind.config file
fd "tailwind.config" -t fRepository: jniebuhr/gaggimate
Length of output: 564
🏁 Script executed:
# Check tailwind.config.js for Tailwind v4 configuration
cat web/tailwind.config.js
# Check if there are actual lint errors by looking at CI config or lint output
fd ".github" -t d
fd "eslintignore|stylelintignore" -t f
# Search documentation comments or issues about Tailwind v4 at-rules
rg -i "custom-variant|`@theme`|`@source`" web/src/style.css -B 2 -A 2Repository: jniebuhr/gaggimate
Length of output: 438
🌐 Web query:
Tailwind CSS v4 @custom-variant @theme @source stylelint configuration ignoreAtRules
💡 Result:
In Tailwind CSS v4, directives such as @theme, @source, and @custom-variant are native CSS-driven features [1][2]. To prevent Stylelint from flagging these as unknown at-rules, you can use the following approaches: Recommended Approach: Use a dedicated Tailwind-aware Stylelint configuration. Projects like stylelint-config-tailwindcss are designed for v4+ and automatically configure Stylelint to recognize Tailwind’s directives and functions [3][4][5]. These configurations integrate into Stylelint's languageOptions, ensuring that directives like @theme, @source, and @custom-variant are treated as valid syntax rather than errors [3][4]. Manual Configuration: If you prefer to configure Stylelint manually, you must add these directives to the ignoreAtRules option within the at-rule-no-unknown rule (or the equivalent SCSS rule if you are using stylelint-scss) [6][7][8]. Example configuration: { "rules": { "at-rule-no-unknown": [ true, { "ignoreAtRules": [ "theme", "source", "custom-variant", "utility", "apply", "variant" ] } ] } } Additionally, if you are using nesting in your CSS (e.g., inside @utility), you may need to add the same directives to the ignoreAtRules option for the nesting-selector-no-missing-scoping-root rule to avoid scoping errors [9]. { "rules": { "nesting-selector-no-missing-scoping-root": [ true, { "ignoreAtRules": ["utility", "custom-variant"] } ] } } Ensure that your Stylelint version is up-to-date (v16.17+ is typically recommended for full compatibility with these configurations) [3][4][5].
Citations:
- 1: https://tailwindcss.com/docs/functions-and-directives
- 2: https://github.com/tailwindlabs/tailwindcss.com/blob/main/src/docs/functions-and-directives.mdx
- 3: https://github.com/dreamsicle-io/stylelint-config-tailwindcss
- 4: https://github.com/studiometa/stylelint-config-tailwindcss
- 5: https://github.com/t0byman/stylelint-config-tailwindcss
- 6: https://www.w3tutorials.net/blog/unexpected-unknown-at-rule-tailwind-scss-at-rule-no-unknown/
- 7: https://tailkits.com/blog/how-to-fix-unknown-at-rule-error-in-tailwind-css/
- 8: https://stackoverflow.com/questions/72161637/unexpected-unknown-at-rule-tailwind-scss-at-rule-no-unknown
- 9: Add
ignoreAtRules: []tonesting-selector-no-missing-scoping-rootstylelint/stylelint#8743
Configure stylelint to recognize Tailwind v4 at-rules (@theme, @source, @custom-variant).
The .stylelintrc.json extends stylelint-config-standard-scss, which has at-rule-no-unknown enabled by default. This will flag Tailwind v4's native directives as unknown at-rules and block CI. Add an ignoreAtRules list to the at-rule-no-unknown rule:
Fix for .stylelintrc.json
{
"extends": ["stylelint-config-standard-scss"],
"rules": {
"at-rule-no-unknown": [true, { "ignoreAtRules": ["theme", "source", "custom-variant", "apply", "utility", "variant"] }],
// ... existing rules ...
}
}Alternatively, use stylelint-config-tailwindcss for built-in Tailwind v4 support.
🧰 Tools
🪛 Stylelint (17.12.0)
[error] 423-423: Expected "BlinkMacSystemFont" to be "blinkmacsystemfont" (value-keyword-case)
(value-keyword-case)
[error] 423-423: Expected "Roboto" to be "roboto" (value-keyword-case)
(value-keyword-case)
🤖 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 `@web/src/style.css` at line 423, The stylelint configuration extends
stylelint-config-standard-scss which has the at-rule-no-unknown rule enabled by
default, causing Tailwind v4 directives like `@theme`, `@source`, and
`@custom-variant` to be flagged as unknown at-rules and block CI builds. Update
the at-rule-no-unknown rule in the configuration to include an ignoreAtRules
array that lists the Tailwind v4 at-rules (theme, source, custom-variant, apply,
utility, variant), or alternatively replace the current extended configuration
with stylelint-config-tailwindcss which provides built-in support for Tailwind
v4 syntax.
Source: Linters/SAST tools
…Montserrat stylesheet
|
2 participants
Summary
This PR modernizes the sidebar navigation layout and polishes global CSS aesthetics, introducing clean interactive transitions, improved color tokens, and font-loading optimizations.
What's Changed
web/src/components/Navigation.jsx): Reworked navigation to always-mounted elements with clean slide transitions, and embedded custom SVG logo (GmLogoIcon.jsx). The link structure is kept identical tomasterto ensure backward compatibility and prevent dead links.web/src/style.css,web/index.html): Polished visual variables (color tokens, focus rings, progress bar animations) and deferred Google Fonts loading inindex.htmlto eliminate render-blocking CSS imports.Card.jsx,Chart.jsx, etc.): Added a customizablesurfaceprop toCard.jsxand added aResizeObserverto charts to dynamically handle size changes (e.g. within accordion structures).This is part of a series of independent, story-driven PRs split from the larger frontend modernization effort.
Summary by CodeRabbit
New Features
Improvements
Style