Mini Game v2: Dependency bump#34
Merged
gianzamboni merged 13 commits intomini-game-v2from Mar 1, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR refactors 3D components to use direct ref props instead of Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Dockerfile (1)
17-22: Consider running the container as a non-root user.The container currently runs as root, which is flagged by static analysis (Trivy DS-0002). Running as a non-root user is a security best practice that limits the impact of potential container escapes.
🛡️ Proposed fix to add a non-root user
FROM node:20.19.0-alpine COPY ./package.json package-lock.json /app/ COPY --from=production-dependencies-env /app/node_modules /app/node_modules COPY --from=build-env /app/build /app/build WORKDIR /app +USER node CMD ["npm", "run", "start"]Note: The
nodeuser is pre-created in the official Node.js Alpine images. Ensure the/appdirectory and its contents have appropriate permissions for thenodeuser.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 17 - 22, Change the Dockerfile to run the container as a non-root user by ensuring /app and its contents are owned by the non-root user (the pre-created node user in the Node.js Alpine image) and switching to that user before CMD; specifically, adjust the COPY/creation steps so /app is writable by node (e.g., set ownership of /app and copied files to node or use an equivalent chown/--chown approach), keep WORKDIR /app, and add a USER node before the CMD ["npm","run","start"] so the application runs with non-root privileges.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Dockerfile`:
- Around line 17-22: Change the Dockerfile to run the container as a non-root
user by ensuring /app and its contents are owned by the non-root user (the
pre-created node user in the Node.js Alpine image) and switching to that user
before CMD; specifically, adjust the COPY/creation steps so /app is writable by
node (e.g., set ownership of /app and copied files to node or use an equivalent
chown/--chown approach), keep WORKDIR /app, and add a USER node before the CMD
["npm","run","start"] so the application runs with non-root privileges.
gianzamboni
force-pushed
the
mini-game-v2-dependency-bump
branch
from
fa5c0a3 to
4b0f8c1
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/routes/home/styles.module.css (1)
29-31: Consider removing the redundant media query.The
gap: var(--spacing-lg)at 1200px now matches the value at 728px. If no other properties need the 1200px breakpoint for.home-container, this media query block could be removed to reduce duplication.♻️ Proposed simplification
.home-container { padding: var(--spacing-lg); gap: var(--spacing-md); `@media` (min-width: 728px) { gap: var(--spacing-lg); } - - `@media` (min-width: 1200px) { - gap: var(--spacing-lg); - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/home/styles.module.css` around lines 29 - 31, Remove the redundant 1200px media query for .home-container since gap: var(--spacing-lg) is already applied at 728px; locate the rule targeting .home-container inside the `@media` (min-width: 1200px) block and delete that block (or at minimum remove the gap declaration) so duplication is eliminated and only the 728px breakpoint controls the gap value.Dockerfile (1)
17-22: Consider adding a non-root user for improved security.The container currently runs as root. Adding a non-root user reduces the attack surface if the container is compromised.
🛡️ Proposed fix to run as non-root
FROM node:20.19.0-alpine COPY ./package.json package-lock.json /app/ COPY --from=production-dependencies-env /app/node_modules /app/node_modules COPY --from=build-env /app/build /app/build WORKDIR /app +USER node CMD ["npm", "run", "start"]Note: The
nodeuser is pre-created in the official Node Alpine images. Ensure file permissions allow thenodeuser to read the application files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 17 - 22, The Dockerfile currently runs as root; update it to switch to a non-root user (use the existing node user in the official Node Alpine image) after copying files and setting WORKDIR so the process runs without root privileges; ensure application files and /app ownership/permissions are adjusted (chown to node:node or chmod as needed) so the node user can access /app and then add USER node before the CMD ["npm","run","start"] to run the container as that user.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/routes/home/styles.module.css`:
- Around line 29-31: Remove the redundant 1200px media query for .home-container
since gap: var(--spacing-lg) is already applied at 728px; locate the rule
targeting .home-container inside the `@media` (min-width: 1200px) block and delete
that block (or at minimum remove the gap declaration) so duplication is
eliminated and only the 728px breakpoint controls the gap value.
In `@Dockerfile`:
- Around line 17-22: The Dockerfile currently runs as root; update it to switch
to a non-root user (use the existing node user in the official Node Alpine
image) after copying files and setting WORKDIR so the process runs without root
privileges; ensure application files and /app ownership/permissions are adjusted
(chown to node:node or chmod as needed) so the node user can access /app and
then add USER node before the CMD ["npm","run","start"] to run the container as
that user.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.coderabbit.yaml.nvmrcDockerfileapp/app.cssapp/routes/demos/layout.tsxapp/routes/home/index.tsxapp/routes/home/styles.module.cssapp/sketched-components/leva-panel/bottom-panel.tsxapp/sketched-components/leva-panel/styles.module.cssapp/utils/hooks/use-demo-params.tspackage.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app/sketched-components/title/h1.tsx (1)
9-10:styles.titleapplied to both parentdivand childh1.The
.titleclass is applied to both elements (line 9 and line 10). Since font properties in.title(font-weight, font-family, font-size) are inherited, applying them to the outerdivshould suffice. Consider removing it from the innerh1to avoid redundancy, unless there's a specific styling reason for both.💡 Suggested simplification
<div className={`relative ${styles['title']}`}> - <h1 className={styles.title}> + <h1> {children} </h1>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/sketched-components/title/h1.tsx` around lines 9 - 10, The .title class is redundantly applied to both the outer div and the inner h1 in the H1 component; remove the className={styles.title} from the inner h1 (leave className={`relative ${styles['title']}`} on the parent div) so font properties are only set on the container, and confirm there’s no separate h1-specific styling or behavior in the H1 component that requires the class on the h1 element..nvmrc (1)
1-1: Consider specifying the minimum version for consistency.The
.nvmrcspecifies20, butpackage.jsonrequires^20.19.0. If nvm resolves to an older 20.x version (e.g., from a stale cache), it may not satisfy the engines constraint. Consider using20.19.0orlts/ironto ensure alignment.💡 Suggested fix
-20 +20.19.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.nvmrc at line 1, The .nvmrc currently pins "20" which can resolve to an older 20.x not satisfying package.json's engines (^20.19.0); update .nvmrc to a specific minimum matching the package.json (e.g., replace "20" with "20.19.0") or use a stable alias like "lts/iron" to ensure nvm selects a compatible Node version.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.cursor/rules/project-conventions.mdc:
- Line 11: Update the doc line to clarify import origins: state that mesh refs
should be typed as Ref<Mesh> with Ref imported from 'react' and Mesh imported
from 'three' (e.g., use Ref from React and Mesh from three), and keep the
existing recommendation for `@react-three/fiber` and `@react-three/drei` plus
declarative elements like <mesh> and <meshStandardMaterial>; reference the
symbols Ref and Mesh so readers know which package each comes from.
In `@app/sketched-components/leva-panel/index.tsx`:
- Line 45: The className template in the JSX div (`<div
className={`${styles["panel-container"]} ${className}`}>`) can render the
literal "undefined" when className is not provided; change the composition to a
null-safe join (e.g. combine styles["panel-container"] and className into an
array, filter(Boolean) and join(' ')) or use a small helper/utility like
classnames to only include defined classes so "undefined" is never emitted.
In `@app/sketched-components/leva-panel/settings-button/index.tsx`:
- Around line 15-17: The Settings button is icon-only and needs an accessible
name: update the JSX where SketchyButton is rendered (the element containing
SettingsIcon, using props className and onClick) to include an aria-label (e.g.
aria-label="Open settings" or "Settings") and optionally a title with the same
text; ensure the aria-label is applied directly to the SketchyButton component
so assistive tech receives the accessible name.
---
Nitpick comments:
In @.nvmrc:
- Line 1: The .nvmrc currently pins "20" which can resolve to an older 20.x not
satisfying package.json's engines (^20.19.0); update .nvmrc to a specific
minimum matching the package.json (e.g., replace "20" with "20.19.0") or use a
stable alias like "lts/iron" to ensure nvm selects a compatible Node version.
In `@app/sketched-components/title/h1.tsx`:
- Around line 9-10: The .title class is redundantly applied to both the outer
div and the inner h1 in the H1 component; remove the className={styles.title}
from the inner h1 (leave className={`relative ${styles['title']}`} on the parent
div) so font properties are only set on the container, and confirm there’s no
separate h1-specific styling or behavior in the H1 component that requires the
class on the h1 element.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
.coderabbit.yaml.cursor/rules/project-conventions.mdc.nvmrcDockerfileapp/3d/cube.tsxapp/3d/sphere.tsxapp/app.cssapp/routes/demos/02-drei-app/main-scene.tsxapp/routes/demos/02-drei-app/styles.module.cssapp/routes/demos/03-debug-ui/main-scene.tsxapp/routes/demos/layout.tsxapp/routes/demos/styles.module.cssapp/routes/home/index.tsxapp/routes/home/styles.module.cssapp/sketched-components/button/styles.module.cssapp/sketched-components/card/index.tsxapp/sketched-components/card/styles.module.cssapp/sketched-components/common.module.cssapp/sketched-components/label/label.tsxapp/sketched-components/label/styles.module.cssapp/sketched-components/leva-panel/bottom-panel/index.tsxapp/sketched-components/leva-panel/bottom-panel/styles.module.cssapp/sketched-components/leva-panel/index.tsxapp/sketched-components/leva-panel/settings-button/index.tsxapp/sketched-components/leva-panel/settings-button/styles.module.cssapp/sketched-components/leva-panel/sketchy-leva-panel/index.tsxapp/sketched-components/leva-panel/sketchy-leva-panel/leva-theme.tsapp/sketched-components/leva-panel/styles.module.cssapp/sketched-components/link/styles.module.cssapp/sketched-components/title/h1.tsxapp/sketched-components/title/styles.module.cssapp/utils/hooks/use-demo-params.tspackage.json
💤 Files with no reviewable changes (4)
- app/sketched-components/leva-panel/styles.module.css
- app/routes/demos/02-drei-app/styles.module.css
- Dockerfile
- app/sketched-components/common.module.css
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/sketched-components/leva-panel/bottom-panel/index.tsx (1)
48-50:⚠️ Potential issue | 🟠 MajorReference to undefined CSS class
.opened.Line 49 references
styles["opened"]but this class is not defined instyles.module.css. This will injectundefinedinto the className string. The fix should be applied in the CSS file (already flagged there).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/sketched-components/leva-panel/bottom-panel/index.tsx` around lines 48 - 50, The JSX in BottomPanel uses styles["opened"] (in the div with className combining commonStyles["animated"] and isAtBottom ? styles["closed"] : styles["opened"]) but the `.opened` class is missing from the CSS module; add a matching `.opened` rule to the component's styles.module.css with the intended styles (mirror or complement the existing `.closed` behavior) so styles["opened"] is defined and the className no longer injects undefined.
🧹 Nitpick comments (3)
app/sketched-components/leva-panel/settings-button/index.tsx (1)
8-17: Expose toggle state witharia-pressedand a dynamic label.Since this button toggles panel visibility, a static
"Open settings panel"label can become misleading after opening.♿ Suggested improvement
type SettingsButtonProps = Readonly<{ onClick: () => void; className?: string; + isOpen: boolean; }>; -export default function SettingsButton({ onClick, className }: SettingsButtonProps) { +export default function SettingsButton({ onClick, className, isOpen }: SettingsButtonProps) { + const label = isOpen ? "Close settings panel" : "Open settings panel"; return ( - <SketchyButton className={`${styles["settings-button"]} ${className ?? ""}`} onClick={onClick} aria-label="Open settings panel" title="Open settings panel"> + <SketchyButton + className={`${styles["settings-button"]} ${className ?? ""}`} + onClick={onClick} + aria-label={label} + title={label} + aria-pressed={isOpen} + > <SettingsIcon className={styles["settings-icon"]} /> </SketchyButton> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/sketched-components/leva-panel/settings-button/index.tsx` around lines 8 - 17, The SettingsButton currently always uses a static label and doesn't expose toggle state; change the component signature (SettingsButton) to accept a boolean prop (e.g., isOpen or isPressed) and use it to set aria-pressed (aria-pressed={isOpen}) and a dynamic accessible label/title (e.g., "Open settings panel" when false and "Close settings panel" when true). Update the component to derive the title and aria-label from that prop and keep existing onClick and className behavior, and then update any call sites to pass the new boolean prop reflecting the panel's visibility..nvmrc (1)
1-1: Pin.nvmrcto an explicit patch version that satisfiesengines.node.Currently
.nvmrcspecifies only20, which is resolved by nvm to the latest available v20.x version. While this likely satisfies the constraint^20.19.0 || >=22.12.0in practice, explicitly pinning the patch version improves clarity and ensures consistency across different nvm versions and developer environments.🔧 Proposed fix
-20 +20.19.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.nvmrc at line 1, Update .nvmrc to pin an explicit Node patch version that satisfies the package.json engines.node constraint (e.g., replace "20" with "20.19.0" or whatever exact 20.x patch you choose that meets ^20.19.0 || >=22.12.0); ensure the chosen patch matches the engines.node entry in package.json so all developers use the same resolved Node version.app/sketched-components/leva-panel/index.tsx (1)
18-21: Consider documenting the@ts-ignorewith a more specific suppression.The comment explains why the unused
_storevariable is needed, but using@ts-ignoresuppresses all TypeScript errors. Consider using@ts-expect-errorwith a specific explanation, which will error if the suppression becomes unnecessary.📝 Proposed refinement
// Used as a triger for a levaStore changes; Not used directly though. // If removed, this component won't work. - // `@ts-ignore` + // `@ts-expect-error` -- Variable unused but subscription triggers re-render const _store = levaStore.useStore();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/sketched-components/leva-panel/index.tsx` around lines 18 - 21, Replace the broad "@ts-ignore" used above the _store declaration with a narrower suppression: change it to "@ts-expect-error" and add a short comment explaining why the suppression is required (e.g., keeps component subscribed to levaStore via levaStore.useStore() despite the variable being unused) so TypeScript will surface an error if the suppression is no longer necessary; update the comment that documents the purpose of the _store binding (referencing _store and levaStore.useStore) to include that it's intentionally unused but required to trigger leva store updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/sketched-components/button/styles.module.css`:
- Around line 1-6: Add a positioned container to restore anchoring for the
sketch overlays: update the .sketchy-button rule in styles.module.css to include
position: relative so SketchyBorder and SketchyShadow can anchor to the button,
and add overflow: hidden (or overflow: visible if you want overlays to extend)
to prevent stray traces from bleeding outside the control; this change targets
the .sketchy-button CSS class used by the SketchyBorder/SketchyShadow overlays.
In `@app/sketched-components/leva-panel/bottom-panel/styles.module.css`:
- Around line 34-42: The CSS module is missing the .opened class referenced as
styles["opened"] in bottom-panel/index.tsx; add a .opened rule to this CSS
(alongside .closed and .noNumberInput) that defines the open state (e.g., no
translateY or translateY(0) and any matching transition/visibility styles used
by the component) so styles["opened"] resolves to a valid class name and the
panel opens correctly.
In `@app/sketched-components/title/h1.tsx`:
- Around line 9-10: The outer <div> and inner <h1> are both using
styles['title'], causing heading font styles to be applied twice; update the JSX
so only the <h1> uses styles.title and remove styles['title'] from the container
(or replace it with a distinct container class such as styles.titleContainer if
layout-specific rules are needed), and update the CSS module to add the new
container rule if you introduce styles.titleContainer; touch the JSX in the
component where className={`relative ${styles['title']}`} and the <h1
className={styles.title}> to make this change.
In `@app/sketched-components/title/styles.module.css`:
- Around line 1-3: The .sketchy-trace CSS rule (selector .sketchy-trace) is
missing positioning coordinates; update that rule to include top: 0 and left: 0
so the absolutely positioned SVG overlays the title correctly (i.e., keep
position: absolute and add top: 0; left: 0; to the .sketchy-trace declaration).
---
Outside diff comments:
In `@app/sketched-components/leva-panel/bottom-panel/index.tsx`:
- Around line 48-50: The JSX in BottomPanel uses styles["opened"] (in the div
with className combining commonStyles["animated"] and isAtBottom ?
styles["closed"] : styles["opened"]) but the `.opened` class is missing from the
CSS module; add a matching `.opened` rule to the component's styles.module.css
with the intended styles (mirror or complement the existing `.closed` behavior)
so styles["opened"] is defined and the className no longer injects undefined.
---
Nitpick comments:
In @.nvmrc:
- Line 1: Update .nvmrc to pin an explicit Node patch version that satisfies the
package.json engines.node constraint (e.g., replace "20" with "20.19.0" or
whatever exact 20.x patch you choose that meets ^20.19.0 || >=22.12.0); ensure
the chosen patch matches the engines.node entry in package.json so all
developers use the same resolved Node version.
In `@app/sketched-components/leva-panel/index.tsx`:
- Around line 18-21: Replace the broad "@ts-ignore" used above the _store
declaration with a narrower suppression: change it to "@ts-expect-error" and add
a short comment explaining why the suppression is required (e.g., keeps
component subscribed to levaStore via levaStore.useStore() despite the variable
being unused) so TypeScript will surface an error if the suppression is no
longer necessary; update the comment that documents the purpose of the _store
binding (referencing _store and levaStore.useStore) to include that it's
intentionally unused but required to trigger leva store updates.
In `@app/sketched-components/leva-panel/settings-button/index.tsx`:
- Around line 8-17: The SettingsButton currently always uses a static label and
doesn't expose toggle state; change the component signature (SettingsButton) to
accept a boolean prop (e.g., isOpen or isPressed) and use it to set aria-pressed
(aria-pressed={isOpen}) and a dynamic accessible label/title (e.g., "Open
settings panel" when false and "Close settings panel" when true). Update the
component to derive the title and aria-label from that prop and keep existing
onClick and className behavior, and then update any call sites to pass the new
boolean prop reflecting the panel's visibility.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
.coderabbit.yaml.cursor/rules/project-conventions.mdc.nvmrcDockerfileapp/3d/cube.tsxapp/3d/sphere.tsxapp/app.cssapp/routes/demos/02-drei-app/main-scene.tsxapp/routes/demos/02-drei-app/styles.module.cssapp/routes/demos/03-debug-ui/main-scene.tsxapp/routes/demos/layout.tsxapp/routes/demos/styles.module.cssapp/routes/home/index.tsxapp/routes/home/styles.module.cssapp/sketched-components/button/styles.module.cssapp/sketched-components/card/index.tsxapp/sketched-components/card/styles.module.cssapp/sketched-components/common.module.cssapp/sketched-components/label/label.tsxapp/sketched-components/label/styles.module.cssapp/sketched-components/leva-panel/bottom-panel/index.tsxapp/sketched-components/leva-panel/bottom-panel/styles.module.cssapp/sketched-components/leva-panel/index.tsxapp/sketched-components/leva-panel/settings-button/index.tsxapp/sketched-components/leva-panel/settings-button/styles.module.cssapp/sketched-components/leva-panel/sketchy-leva-panel/index.tsxapp/sketched-components/leva-panel/sketchy-leva-panel/leva-theme.tsapp/sketched-components/leva-panel/styles.module.cssapp/sketched-components/link/styles.module.cssapp/sketched-components/title/h1.tsxapp/sketched-components/title/styles.module.cssapp/utils/hooks/use-demo-params.tspackage.json
💤 Files with no reviewable changes (4)
- app/sketched-components/leva-panel/styles.module.css
- app/routes/demos/02-drei-app/styles.module.css
- app/sketched-components/common.module.css
- Dockerfile
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/routes/home/index.tsx`:
- Line 25: The external LinkedIn anchor in the JSX (the <a
href="https://www.linkedin.com/in/gianfranco-zamboni/" target="_blank"> element
in app/routes/home/index.tsx) is missing rel attributes; update that anchor to
include rel="noopener noreferrer" so the target="_blank" link has proper tab
isolation and prevents access to window.opener.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
.coderabbit.yaml.cursor/rules/project-conventions.mdc.nvmrcDockerfileapp/3d/cube.tsxapp/3d/sphere.tsxapp/app.cssapp/routes/demos/02-drei-app/main-scene.tsxapp/routes/demos/02-drei-app/styles.module.cssapp/routes/demos/03-debug-ui/main-scene.tsxapp/routes/demos/layout.tsxapp/routes/demos/styles.module.cssapp/routes/home/index.tsxapp/routes/home/styles.module.cssapp/sketched-components/button/styles.module.cssapp/sketched-components/card/index.tsxapp/sketched-components/card/styles.module.cssapp/sketched-components/common.module.cssapp/sketched-components/label/label.tsxapp/sketched-components/label/styles.module.cssapp/sketched-components/leva-panel/bottom-panel/index.tsxapp/sketched-components/leva-panel/bottom-panel/styles.module.cssapp/sketched-components/leva-panel/index.tsxapp/sketched-components/leva-panel/settings-button/index.tsxapp/sketched-components/leva-panel/settings-button/styles.module.cssapp/sketched-components/leva-panel/sketchy-leva-panel/index.tsxapp/sketched-components/leva-panel/sketchy-leva-panel/leva-theme.tsapp/sketched-components/leva-panel/styles.module.cssapp/sketched-components/link/styles.module.cssapp/sketched-components/title/h1.tsxapp/sketched-components/title/styles.module.cssapp/utils/hooks/use-demo-params.tspackage.json
💤 Files with no reviewable changes (4)
- app/routes/demos/02-drei-app/styles.module.css
- app/sketched-components/leva-panel/styles.module.css
- app/sketched-components/common.module.css
- Dockerfile
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/sketched-components/link/styles.module.css (1)
19-23: Keep minimum interactive hit area while reducing icon widths.The smaller icon sizes at Line 19 and Line 23 are fine visually, but please ensure the clickable element still meets a minimum touch target (commonly 44x44 CSS px).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/sketched-components/link/styles.module.css` around lines 19 - 23, The reduced icon widths use var(--icon-size-sm) and var(--icon-size-md) but can produce too-small hit targets; update the CSS rule(s) that set width: var(--icon-size-sm) and width: var(--icon-size-md) to enforce a minimum interactive area by adding a min-width: 44px and min-height: 44px (and ensure display: inline-flex; align-items: center; justify-content: center to keep the icon centered), or alternatively add padding so the visual icon remains the desired size while the clickable element is at least 44x44 CSS px..nvmrc (1)
1-1: Pin.nvmrcto the engine floor for consistency.The
.nvmrcfile currently specifies20(major-only), whilepackage.jsondeclares^20.19.0 || >=22.12.0. This creates a drift risk: a developer using.nvmrccould install Node 20.0.0 or 20.18.0, butpackage.jsonrequires at least 20.19.0, causing potential engine mismatch surprises. Pin to20.19.0to match the declared minimum.Proposed change
-20 +20.19.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.nvmrc at line 1, Update the .nvmrc value from the major-only "20" to the minimum engine version declared in package.json by pinning it to "20.19.0" so local nvm usage matches the project's engine requirement; edit the .nvmrc file to contain the exact version string "20.19.0".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.nvmrc:
- Line 1: Update the .nvmrc value from the major-only "20" to the minimum engine
version declared in package.json by pinning it to "20.19.0" so local nvm usage
matches the project's engine requirement; edit the .nvmrc file to contain the
exact version string "20.19.0".
In `@app/sketched-components/link/styles.module.css`:
- Around line 19-23: The reduced icon widths use var(--icon-size-sm) and
var(--icon-size-md) but can produce too-small hit targets; update the CSS
rule(s) that set width: var(--icon-size-sm) and width: var(--icon-size-md) to
enforce a minimum interactive area by adding a min-width: 44px and min-height:
44px (and ensure display: inline-flex; align-items: center; justify-content:
center to keep the icon centered), or alternatively add padding so the visual
icon remains the desired size while the clickable element is at least 44x44 CSS
px.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
.coderabbit.yaml.cursor/rules/project-conventions.mdc.nvmrcDockerfileapp/3d/cube.tsxapp/3d/sphere.tsxapp/app.cssapp/routes/demos/02-drei-app/main-scene.tsxapp/routes/demos/02-drei-app/styles.module.cssapp/routes/demos/03-debug-ui/main-scene.tsxapp/routes/demos/layout.tsxapp/routes/demos/styles.module.cssapp/routes/home/index.tsxapp/routes/home/styles.module.cssapp/sketched-components/button/styles.module.cssapp/sketched-components/card/index.tsxapp/sketched-components/card/styles.module.cssapp/sketched-components/common.module.cssapp/sketched-components/label/label.tsxapp/sketched-components/label/styles.module.cssapp/sketched-components/leva-panel/bottom-panel/index.tsxapp/sketched-components/leva-panel/bottom-panel/styles.module.cssapp/sketched-components/leva-panel/index.tsxapp/sketched-components/leva-panel/settings-button/index.tsxapp/sketched-components/leva-panel/settings-button/styles.module.cssapp/sketched-components/leva-panel/sketchy-leva-panel/index.tsxapp/sketched-components/leva-panel/sketchy-leva-panel/leva-theme.tsapp/sketched-components/leva-panel/styles.module.cssapp/sketched-components/link/styles.module.cssapp/sketched-components/title/h1.tsxapp/sketched-components/title/styles.module.cssapp/utils/hooks/use-demo-params.tspackage.json
💤 Files with no reviewable changes (4)
- app/sketched-components/common.module.css
- app/routes/demos/02-drei-app/styles.module.css
- Dockerfile
- app/sketched-components/leva-panel/styles.module.css
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/sketched-components/leva-panel/bottom-panel/index.tsx (1)
48-50:⚠️ Potential issue | 🔴 Critical
styles["opened"]references a missing CSS class.This line uses
styles["opened"]whenisAtBottomis false, but the.openedclass is not defined instyles.module.css. This will applyundefinedas the class name. Either add the.openedclass to the CSS module or simplify the logic to only apply theclosedclass conditionally.🐛 Alternative fix if no explicit open state styling is needed
- className={`${commonStyles["animated"]} ${isAtBottom ? styles["closed"] : styles["opened"]}`} + className={`${commonStyles["animated"]} ${isAtBottom ? styles["closed"] : ""}`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/sketched-components/leva-panel/bottom-panel/index.tsx` around lines 48 - 50, The component is applying styles["opened"] which doesn't exist in the CSS module; either add an .opened class to the CSS module or change the JSX in the bottom-panel component to only apply styles["closed"] conditionally; update the div that uses isAtBottom, commonStyles["animated"], styles["closed"], and styles["opened"] so that when isAtBottom is false you do not reference styles["opened"] (e.g., render `${commonStyles["animated"]} ${!isAtBottom ? styles["closed"] : ''}`) or add a corresponding .opened rule to the CSS module to match the existing reference.
🧹 Nitpick comments (3)
.cursor/rules/project-conventions.mdc (1)
8-8: Consider explicitly noting React 19+ in the ref guidance for clarity.The repository targets React 19.2.4, so line 8's guidance on ref-as-prop and avoiding
forwardRefis appropriate. However, adding an explicit React 19+ qualifier in the document itself (e.g., "React 19+: Use ref as a normal prop...") would improve clarity for future readers and make the version assumption explicit, especially if conventions are shared across projects.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.cursor/rules/project-conventions.mdc at line 8, Update the rule text to explicitly reference React 19+ so readers know the ref-as-prop guidance applies to modern React; change the sentence that currently begins "**React**: Use functional components..." to something like "**React 19+**: Use functional components (we target React 19.2.4): use `ref` as a normal prop (include `ref?: Ref<T>` in props, destructure `{ ref, ...props }` in the signature and pass it through) and keep the note to not use `forwardRef`—this makes the version assumption explicit while preserving the existing instructions.app/sketched-components/leva-panel/bottom-panel/styles.module.css (1)
38-42: Stylelint warning for:global()is a false positive.The
:global()pseudo-class is valid CSS Modules syntax used to escape module scoping and target global selectors. The Stylelint ruleselector-pseudo-class-no-unknownneeds configuration to recognize this. Consider adding:globalto theignorePseudoClassesoption in your Stylelint config if you want to suppress this warning.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/sketched-components/leva-panel/bottom-panel/styles.module.css` around lines 38 - 42, The Stylelint warning for the :global() pseudo-class is a false positive; update the Stylelint configuration to add ":global" to the selector-pseudo-class-no-unknown rule's ignorePseudoClasses option so CSS Modules escapes like .noNumberInput :global(input[leva-type="number"]) are allowed; specifically, modify your Stylelint config to include ":global" in ignorePseudoClasses (or add it to the equivalent overrides) so the selector-pseudo-class-no-unknown rule no longer flags the :global pseudo-class.app/sketched-components/leva-panel/bottom-panel/index.tsx (1)
38-41: Inconsistent className handling between root div and inner div.The root div (lines 38-41) conditionally appends only the
closedclass when!isOpen, defaulting to no additional class when open. However, the inner div (line 49) explicitly referencesstyles["opened"]. Consider aligning these patterns for consistency—either both should use an explicitopenedclass, or both should omit styling when in the open state.Also applies to: 48-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/sketched-components/leva-panel/bottom-panel/index.tsx` around lines 38 - 41, The root className builds `${styles["bottom-panel"]} ${commonStyles["animated"]}` and only appends styles["closed"] when !isOpen, while the inner div explicitly uses styles["opened"]; make them consistent by updating the root class construction to append styles["opened"] when isOpen and styles["closed"] when not (so className includes either styles["opened"] or styles["closed"]), and ensure the inner div uses the same predicate (remove hardcoded styles["opened"] if you prefer the root to control state classes) — adjust references to className and the inner div's class usage of styles["opened"]/styles["closed"] accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@app/sketched-components/leva-panel/bottom-panel/index.tsx`:
- Around line 48-50: The component is applying styles["opened"] which doesn't
exist in the CSS module; either add an .opened class to the CSS module or change
the JSX in the bottom-panel component to only apply styles["closed"]
conditionally; update the div that uses isAtBottom, commonStyles["animated"],
styles["closed"], and styles["opened"] so that when isAtBottom is false you do
not reference styles["opened"] (e.g., render `${commonStyles["animated"]}
${!isAtBottom ? styles["closed"] : ''}`) or add a corresponding .opened rule to
the CSS module to match the existing reference.
---
Nitpick comments:
In @.cursor/rules/project-conventions.mdc:
- Line 8: Update the rule text to explicitly reference React 19+ so readers know
the ref-as-prop guidance applies to modern React; change the sentence that
currently begins "**React**: Use functional components..." to something like
"**React 19+**: Use functional components (we target React 19.2.4): use `ref` as
a normal prop (include `ref?: Ref<T>` in props, destructure `{ ref, ...props }`
in the signature and pass it through) and keep the note to not use
`forwardRef`—this makes the version assumption explicit while preserving the
existing instructions.
In `@app/sketched-components/leva-panel/bottom-panel/index.tsx`:
- Around line 38-41: The root className builds `${styles["bottom-panel"]}
${commonStyles["animated"]}` and only appends styles["closed"] when !isOpen,
while the inner div explicitly uses styles["opened"]; make them consistent by
updating the root class construction to append styles["opened"] when isOpen and
styles["closed"] when not (so className includes either styles["opened"] or
styles["closed"]), and ensure the inner div uses the same predicate (remove
hardcoded styles["opened"] if you prefer the root to control state classes) —
adjust references to className and the inner div's class usage of
styles["opened"]/styles["closed"] accordingly.
In `@app/sketched-components/leva-panel/bottom-panel/styles.module.css`:
- Around line 38-42: The Stylelint warning for the :global() pseudo-class is a
false positive; update the Stylelint configuration to add ":global" to the
selector-pseudo-class-no-unknown rule's ignorePseudoClasses option so CSS
Modules escapes like .noNumberInput :global(input[leva-type="number"]) are
allowed; specifically, modify your Stylelint config to include ":global" in
ignorePseudoClasses (or add it to the equivalent overrides) so the
selector-pseudo-class-no-unknown rule no longer flags the :global pseudo-class.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
.coderabbit.yaml.cursor/rules/project-conventions.mdc.nvmrcDockerfileapp/3d/cube.tsxapp/3d/sphere.tsxapp/app.cssapp/routes/demos/02-drei-app/main-scene.tsxapp/routes/demos/02-drei-app/styles.module.cssapp/routes/demos/03-debug-ui/main-scene.tsxapp/routes/demos/layout.tsxapp/routes/demos/styles.module.cssapp/routes/home/index.tsxapp/routes/home/styles.module.cssapp/sketched-components/button/styles.module.cssapp/sketched-components/card/index.tsxapp/sketched-components/card/styles.module.cssapp/sketched-components/common.module.cssapp/sketched-components/label/label.tsxapp/sketched-components/label/styles.module.cssapp/sketched-components/leva-panel/bottom-panel/index.tsxapp/sketched-components/leva-panel/bottom-panel/styles.module.cssapp/sketched-components/leva-panel/index.tsxapp/sketched-components/leva-panel/settings-button/index.tsxapp/sketched-components/leva-panel/settings-button/styles.module.cssapp/sketched-components/leva-panel/sketchy-leva-panel/index.tsxapp/sketched-components/leva-panel/sketchy-leva-panel/leva-theme.tsapp/sketched-components/leva-panel/styles.module.cssapp/sketched-components/link/styles.module.cssapp/sketched-components/title/h1.tsxapp/sketched-components/title/styles.module.cssapp/utils/hooks/use-demo-params.tspackage.json
💤 Files with no reviewable changes (4)
- app/routes/demos/02-drei-app/styles.module.css
- app/sketched-components/leva-panel/styles.module.css
- app/sketched-components/common.module.css
- Dockerfile
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/sketched-components/leva-panel/bottom-panel/index.tsx (1)
47-47: Move staticz-indexstyling into CSS module.
zIndex: -1is static; keeping it instyles.module.csswill simplify maintenance and reduce inline style overrides.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/sketched-components/leva-panel/bottom-panel/index.tsx` at line 47, The inline static style style={{ zIndex: `-1` }} in bottom-panel/index.tsx should be moved into the component's CSS module: add a rule (e.g., .below or .panelZIndex) with z-index: -1 in the corresponding styles.module.css, remove the style prop from the JSX (the exact token style={{ zIndex: `-1` }}), and add the new CSS class to the element's className (merging with existing classes or the imported styles object used in this file). Ensure you update any className concatenation or class composition in the BottomPanel component so the new module class is applied instead of the inline style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/sketched-components/leva-panel/bottom-panel/index.tsx`:
- Line 47: The inline static style style={{ zIndex: `-1` }} in
bottom-panel/index.tsx should be moved into the component's CSS module: add a
rule (e.g., .below or .panelZIndex) with z-index: -1 in the corresponding
styles.module.css, remove the style prop from the JSX (the exact token style={{
zIndex: `-1` }}), and add the new CSS class to the element's className (merging
with existing classes or the imported styles object used in this file). Ensure
you update any className concatenation or class composition in the BottomPanel
component so the new module class is applied instead of the inline style.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.cursor/rules/project-conventions.mdc.nvmrcapp/sketched-components/leva-panel/bottom-panel/index.tsxapp/sketched-components/leva-panel/index.tsxapp/sketched-components/title/h1.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- app/sketched-components/title/h1.tsx
- .nvmrc
- .cursor/rules/project-conventions.mdc
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/routes/home/index.tsx (2)
37-37:⚠️ Potential issue | 🟡 MinorGrammatical error: "This are" should be "These are".
✏️ Proposed fix
- description="This are the demos made for the Three JS Journey Course before reaching the React Three Fiber Chapter." + description="These are the demos made for the Three JS Journey Course before reaching the React Three Fiber Chapter."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/home/index.tsx` at line 37, The description prop in app/routes/home/index.tsx currently reads "This are the demos..." and has a grammatical error; update the string passed to the description prop (the description="...") to "These are the demos made for the Three JS Journey Course before reaching the React Three Fiber Chapter." so subject-verb agreement is correct.
32-40:⚠️ Potential issue | 🟡 MinorAdd
rel="noopener noreferrer"to theSketchedCardcomponent for external links.The component sets
target="_blank"when thetargetBlankprop is passed but does not include therelattribute. This is a security oversight. When opening links in a new tab, therel="noopener noreferrer"attribute should always be set to prevent the opened page from accessingwindow.opener. Update theLinkelement inapp/sketched-components/card/index.tsxto conditionally applyrel="noopener noreferrer"whentargetBlankis true.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/home/index.tsx` around lines 32 - 40, The SketchedCard component is setting targetBlank but not adding rel, which is a security risk; update the SketchedCard render (the Link element inside the sketched card component) to conditionally add rel="noopener noreferrer" whenever the targetBlank prop is true (i.e., when rendering <Link ... target="_blank"> include rel="noopener noreferrer"). Ensure the prop name targetBlank is used to decide adding the rel attribute so external links opened in a new tab include rel="noopener noreferrer".
🧹 Nitpick comments (1)
app/sketched-components/leva-panel/bottom-panel/index.tsx (1)
45-48: Consider using a number forzIndex.The
zIndexstyle value is passed as a string. While this works, using a number is more idiomatic for numeric CSS properties in React.✨ Suggested change
<div className={`${commonStyles["animated"]} ${isContextPanelSlidDown ? styles["closed"] : ""}`} - style={{ zIndex: `-1` }} + style={{ zIndex: -1 }} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/sketched-components/leva-panel/bottom-panel/index.tsx` around lines 45 - 48, The inline style on the bottom-panel div uses a string zIndex value; update the div in app/sketched-components/leva-panel/bottom-panel/index.tsx so the style uses a numeric zIndex (e.g., change style={{ zIndex: `-1` }} to a numeric -1) to be idiomatic in React while leaving the surrounding logic (commonStyles["animated"], styles["closed"], isContextPanelSlidDown) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@app/routes/home/index.tsx`:
- Line 37: The description prop in app/routes/home/index.tsx currently reads
"This are the demos..." and has a grammatical error; update the string passed to
the description prop (the description="...") to "These are the demos made for
the Three JS Journey Course before reaching the React Three Fiber Chapter." so
subject-verb agreement is correct.
- Around line 32-40: The SketchedCard component is setting targetBlank but not
adding rel, which is a security risk; update the SketchedCard render (the Link
element inside the sketched card component) to conditionally add rel="noopener
noreferrer" whenever the targetBlank prop is true (i.e., when rendering <Link
... target="_blank"> include rel="noopener noreferrer"). Ensure the prop name
targetBlank is used to decide adding the rel attribute so external links opened
in a new tab include rel="noopener noreferrer".
---
Nitpick comments:
In `@app/sketched-components/leva-panel/bottom-panel/index.tsx`:
- Around line 45-48: The inline style on the bottom-panel div uses a string
zIndex value; update the div in
app/sketched-components/leva-panel/bottom-panel/index.tsx so the style uses a
numeric zIndex (e.g., change style={{ zIndex: `-1` }} to a numeric -1) to be
idiomatic in React while leaving the surrounding logic
(commonStyles["animated"], styles["closed"], isContextPanelSlidDown) unchanged.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.cursor/rules/project-conventions.mdc.nvmrcapp/routes/home/index.tsxapp/sketched-components/leva-panel/bottom-panel/index.tsxapp/sketched-components/leva-panel/index.tsxapp/sketched-components/title/h1.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- .nvmrc
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 by CodeRabbit
New Features
Style
Chores