Fix environment variable configuration section in self-host #901
Conversation
📝 WalkthroughWalkthroughThe pull request improves environment variable editor UI with standardized truncation styling via a new INPUT_TRUNCATE constant, replaces ScrollArea-based layout with a simpler scrollable container, and enhances key/value display responsiveness. Additionally, a useRef-based initialization guard is added to the editor hook to prevent re-initialization on subsequent renders. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @view/components/ui/env-variables-editor.tsx:
- Around line 145-159: The span currently sets title={variable.value}
unconditionally, which exposes secrets; modify the JSX in the span that renders
variable.value (the element using variable.isSecret, isRevealed, and maskValue)
so the title attribute is only present when the value is not secret or when it
is revealed (e.g., conditionally include title={variable.value} only if
!variable.isSecret || isRevealed); also consider removing the title entirely for
masked values to prevent tooltip leaks and optionally simplify the className
template literal formatting for readability.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (62)
api/templates/deploy-airsonic-advanced.yamlapi/templates/deploy-alist.yamlapi/templates/deploy-altus.yamlapi/templates/deploy-ardour.yamlapi/templates/deploy-audacity.yamlapi/templates/deploy-bambustudio.yamlapi/templates/deploy-blender.yamlapi/templates/deploy-boinc.yamlapi/templates/deploy-brave.yamlapi/templates/deploy-calligra.yamlapi/templates/deploy-chrome.yamlapi/templates/deploy-chromium.yamlapi/templates/deploy-cura.yamlapi/templates/deploy-darktable.yamlapi/templates/deploy-digikam.yamlapi/templates/deploy-dolphin.yamlapi/templates/deploy-doublecommander.yamlapi/templates/deploy-duckstation.yamlapi/templates/deploy-faster-whisper.yamlapi/templates/deploy-ferdium.yamlapi/templates/deploy-ffmpeg.yamlapi/templates/deploy-filezilla.yamlapi/templates/deploy-firefox.yamlapi/templates/deploy-foldingathome.yamlapi/templates/deploy-freecad.yamlapi/templates/deploy-gimp.yamlapi/templates/deploy-gzdoom.yamlapi/templates/deploy-hishtory-server.yamlapi/templates/deploy-htpcmanager.yamlapi/templates/deploy-inkscape.yamlapi/templates/deploy-jackett.yamlapi/templates/deploy-joplin.yamlapi/templates/deploy-kali-linux.yamlapi/templates/deploy-kasm.yamlapi/templates/deploy-kavita.yamlapi/templates/deploy-kdenlive.yamlapi/templates/deploy-keepassxc.yamlapi/templates/deploy-kicad.yamlapi/templates/deploy-krita.yamlapi/templates/deploy-lazylibrarian.yamlapi/templates/deploy-librewolf.yamlapi/templates/deploy-limnoria.yamlapi/templates/deploy-lollypop.yamlapi/templates/deploy-luanti.yamlapi/templates/deploy-mame.yamlapi/templates/deploy-mediaelch.yamlapi/templates/deploy-medusa.yamlapi/templates/deploy-minisatip.yamlapi/templates/deploy-monica.yamlapi/templates/deploy-msedge.yamlapi/templates/deploy-mstream.yamlapi/templates/deploy-mullvad-browser.yamlapi/templates/deploy-myspeed.yamlapi/templates/deploy-netbootxyz.yamlapi/templates/deploy-opengist.yamlapi/templates/deploy-redisinsight.yamlapi/templates/deploy-slash.yamlapi/templates/deploy-stirling-pdf.yamlapi/templates/deploy-taskmd.yamlapi/templates/deploy-url-to-png.yamlview/components/ui/env-variables-editor.tsxview/hooks/use-env-variables-editor.ts
💤 Files with no reviewable changes (60)
- api/templates/deploy-ffmpeg.yaml
- api/templates/deploy-lazylibrarian.yaml
- api/templates/deploy-url-to-png.yaml
- api/templates/deploy-calligra.yaml
- api/templates/deploy-chrome.yaml
- api/templates/deploy-redisinsight.yaml
- api/templates/deploy-brave.yaml
- api/templates/deploy-digikam.yaml
- api/templates/deploy-netbootxyz.yaml
- api/templates/deploy-airsonic-advanced.yaml
- api/templates/deploy-kasm.yaml
- api/templates/deploy-firefox.yaml
- api/templates/deploy-gzdoom.yaml
- api/templates/deploy-mstream.yaml
- api/templates/deploy-altus.yaml
- api/templates/deploy-myspeed.yaml
- api/templates/deploy-monica.yaml
- api/templates/deploy-mame.yaml
- api/templates/deploy-medusa.yaml
- api/templates/deploy-lollypop.yaml
- api/templates/deploy-jackett.yaml
- api/templates/deploy-kdenlive.yaml
- api/templates/deploy-filezilla.yaml
- api/templates/deploy-minisatip.yaml
- api/templates/deploy-dolphin.yaml
- api/templates/deploy-kicad.yaml
- api/templates/deploy-mullvad-browser.yaml
- api/templates/deploy-duckstation.yaml
- api/templates/deploy-audacity.yaml
- api/templates/deploy-limnoria.yaml
- api/templates/deploy-kali-linux.yaml
- api/templates/deploy-faster-whisper.yaml
- api/templates/deploy-slash.yaml
- api/templates/deploy-darktable.yaml
- api/templates/deploy-taskmd.yaml
- api/templates/deploy-stirling-pdf.yaml
- api/templates/deploy-opengist.yaml
- api/templates/deploy-bambustudio.yaml
- api/templates/deploy-freecad.yaml
- api/templates/deploy-librewolf.yaml
- api/templates/deploy-mediaelch.yaml
- api/templates/deploy-chromium.yaml
- api/templates/deploy-doublecommander.yaml
- api/templates/deploy-kavita.yaml
- api/templates/deploy-ardour.yaml
- api/templates/deploy-alist.yaml
- api/templates/deploy-krita.yaml
- api/templates/deploy-blender.yaml
- api/templates/deploy-inkscape.yaml
- api/templates/deploy-ferdium.yaml
- api/templates/deploy-gimp.yaml
- api/templates/deploy-htpcmanager.yaml
- api/templates/deploy-joplin.yaml
- api/templates/deploy-cura.yaml
- api/templates/deploy-foldingathome.yaml
- api/templates/deploy-msedge.yaml
- api/templates/deploy-keepassxc.yaml
- api/templates/deploy-boinc.yaml
- api/templates/deploy-hishtory-server.yaml
- api/templates/deploy-luanti.yaml
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
**/*.{ts,tsx}: Before writing any new logic, search the codebase for existing implementations inview/hooks/,view/lib/utils.ts,view/components/ui/, andview/redux/services/before creating custom elements
Extract repeated patterns into custom hooks or shared components
Use early returns and flat structure instead of nested conditions for improved code readability
Always use typed hooksuseAppDispatchanduseAppSelectorfrom@/redux/hooksinstead of untypeduseDispatchanduseSelector
Never useanytype; always use explicit types for function parameters and return values
Remove unused imports immediately and delete commented-out code
Keep files focused and minimal by removing unused variables and functions
In comments, explain WHY code works a certain way, not WHAT it does
Never disable lint rules without strong justification
Files:
view/components/ui/env-variables-editor.tsxview/hooks/use-env-variables-editor.ts
view/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
view/**/*.{tsx,ts}: Each component file should export one main component
Never write plain HTML for interactive elements; always use shadcn components like Button, Card, Badge, Skeleton
Use thecn()utility function for conditional Tailwind classes
No hardcoded user-facing strings; always use i18n with theuseTranslationhook
Handle loading states by checkingisLoadingfrom RTK Query and returning skeleton loaders or loading components
Handle error states by checking error values from RTK Query and displaying error messages using i18n
Show empty state UI when data is empty using i18n messages
Always provide skeleton loaders for async content instead of blank/white space
Use semantic HTML elements for accessibility
Provide proper ARIA labels where needed for accessibility
Memoize expensive computations withuseMemo
Prevent unnecessary re-renders withuseCallback
Use React Query's caching effectively to avoid redundant API calls
Lazy load heavy components when appropriate
Ensure responsive design works on all screen sizes
Files:
view/components/ui/env-variables-editor.tsxview/hooks/use-env-variables-editor.ts
view/components/ui/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
Do not modify components in
view/components/ui/(shadcn base components)
Files:
view/components/ui/env-variables-editor.tsx
view/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
view/**/*.{ts,tsx}: Respect ESLint rules configured inview/eslint.config.mjs
Use Prettier for consistent formatting and fix all lint errors before committing
Files:
view/components/ui/env-variables-editor.tsxview/hooks/use-env-variables-editor.ts
view/hooks/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
view/hooks/**/*.ts: Use generic patterns for typed hooks to handle type-safe data manipulation
Use JSDoc comments for complex hooks and functions to explain their purpose and parameters
Files:
view/hooks/use-env-variables-editor.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: raghavyuva
Repo: raghavyuva/nixopus PR: 0
File: :0-0
Timestamp: 2025-09-14T14:31:19.830Z
Learning: The update functionality in Nixopus has a critical issue where environment variables are not properly exported to containers during updates. This causes health checks in the compose file to gradually hit Postgres, eventually killing it and causing it to enter recovery mode, leading to unexpected termination and cleaning of production database tables - a severe data loss issue.
Learnt from: raghavyuva
Repo: raghavyuva/nixopus PR: 0
File: :0-0
Timestamp: 2025-09-14T14:31:19.830Z
Learning: The Nixopus update functionality has a critical flaw: the update process in cli/app/commands/update/run.py calls BaseDockerService for "pull" and "up" operations without passing the env_file parameter. This causes containers to start without environment variables, leading to health check failures. Specifically, the Postgres health check "pg_isready -U ${USERNAME} -d ${DB_NAME}" fails when USERNAME and DB_NAME environment variables are missing, eventually causing Postgres to enter recovery mode and potentially lose production data.
📚 Learning: 2025-12-27T03:02:43.092Z
Learnt from: CR
Repo: raghavyuva/nixopus PR: 0
File: .cursor/rules/frontend.mdc:0-0
Timestamp: 2025-12-27T03:02:43.092Z
Learning: Applies to **/*.{ts,tsx} : Keep files focused and minimal by removing unused variables and functions
Applied to files:
view/components/ui/env-variables-editor.tsx
📚 Learning: 2025-12-27T03:02:43.092Z
Learnt from: CR
Repo: raghavyuva/nixopus PR: 0
File: .cursor/rules/frontend.mdc:0-0
Timestamp: 2025-12-27T03:02:43.092Z
Learning: Applies to view/**/*.{tsx,ts} : Never write plain HTML for interactive elements; always use shadcn components like Button, Card, Badge, Skeleton
Applied to files:
view/components/ui/env-variables-editor.tsx
📚 Learning: 2025-12-27T03:02:43.092Z
Learnt from: CR
Repo: raghavyuva/nixopus PR: 0
File: .cursor/rules/frontend.mdc:0-0
Timestamp: 2025-12-27T03:02:43.092Z
Learning: Applies to view/**/*.{tsx,ts} : Prevent unnecessary re-renders with `useCallback`
Applied to files:
view/hooks/use-env-variables-editor.ts
🧬 Code graph analysis (1)
view/components/ui/env-variables-editor.tsx (1)
view/lib/utils.ts (1)
cn(4-6)
🔇 Additional comments (6)
view/hooks/use-env-variables-editor.ts (2)
3-3: LGTM: Import addition for useRef.The addition of
useRefto the imports is appropriate for the new initialization guard pattern.
51-68: Add a comment explaining the one-time initialization behavior.The initialization guard intentionally prevents re-initialization on prop changes, which prevents unwanted resets while users edit variables. However, declaring
defaultValuesin the dependency array while ignoring subsequent changes is misleading. This pattern needs clarification to avoid surprising future maintainers.Suggest one of:
- Add a JSDoc comment explaining this is intentional one-time initialization
- Remove
defaultValuesfrom the dependency array and add an inline comment explaining why- Add a comment above the effect:
// Initialize once from defaultValues; ignore updates to prevent resetting user editsview/components/ui/env-variables-editor.tsx (4)
72-72: LGTM: Truncation constant for DRY principle.The
INPUT_TRUNCATEconstant effectively centralizes truncation styling and ensures consistent text overflow behavior across all input fields in the editor.
99-109: LGTM: Consistent truncation in edit mode.The application of
INPUT_TRUNCATEto both edit key and value inputs ensures consistent text overflow handling during editing.
361-385: Clarify: "Removal of scrolling" vs. overflow-y-auto.The PR objectives mention "removal of scrolling to allow vertical flow," but line 361 still includes
overflow-y-auto, which enables vertical scrolling. This appears to be a replacement of theScrollAreacomponent with native CSS scrolling rather than complete removal of scrolling behavior.Please clarify the intended behavior. If the goal was to:
- Remove custom ScrollArea component: The change is correct
- Remove all scrolling: Consider removing
overflow-y-autoand using a different layout approach- Improve scrolling behavior: Consider documenting what issues the ScrollArea component was causing
Additionally, verify that removing the
ScrollAreacomponent doesn't impact:
- Accessibility features (keyboard navigation, screen reader support)
- Cross-browser scrolling behavior
- Custom scrollbar styling
Based on learnings: The learnings mention critical environment variable export issues in Nixopus. While this change doesn't directly affect the export functionality, ensure that the UI changes don't inadvertently prevent users from properly viewing or editing environment variables that are critical for container health checks.
389-412: LGTM: Consistent truncation for new variable inputs.The new variable input fields properly apply the
INPUT_TRUNCATEconstant and use thecn()utility for conditional styling. The addition ofmin-w-0to the container enables proper text truncation in flex layouts.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
view/hooks/use-env-variables-editor.ts (1)
53-68: Simplify logic and add explanatory comment.The initialization guard works correctly but the else-if block is unnecessarily verbose. Per coding guidelines, prefer early returns and flat structure over nested conditions.
Additionally, add a comment explaining WHY this guard exists (per guidelines: "explain WHY code works a certain way, not WHAT it does"). Based on the PR objectives, this prevents a UI quirk where the form would reset on re-renders.
♻️ Simplified implementation
useEffect(() => { - if (!hasInitialized.current && defaultValues && Object.keys(defaultValues).length > 0) { + // Guard prevents re-initialization on re-renders, fixing form reset quirk + if (hasInitialized.current) return; + + if (defaultValues && Object.keys(defaultValues).length > 0) { const vars = Object.entries(defaultValues).map(([key, value]) => ({ key, value, isSecret: false })); setVariables(vars); - hasInitialized.current = true; - } else if ( - !hasInitialized.current && - (!defaultValues || Object.keys(defaultValues).length === 0) - ) { - hasInitialized.current = true; } + hasInitialized.current = true; }, [defaultValues]);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
view/components/ui/env-variables-editor.tsxview/hooks/use-env-variables-editor.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- view/components/ui/env-variables-editor.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
**/*.{ts,tsx}: Before writing any new logic, search the codebase for existing implementations inview/hooks/,view/lib/utils.ts,view/components/ui/, andview/redux/services/before creating custom elements
Extract repeated patterns into custom hooks or shared components
Use early returns and flat structure instead of nested conditions for improved code readability
Always use typed hooksuseAppDispatchanduseAppSelectorfrom@/redux/hooksinstead of untypeduseDispatchanduseSelector
Never useanytype; always use explicit types for function parameters and return values
Remove unused imports immediately and delete commented-out code
Keep files focused and minimal by removing unused variables and functions
In comments, explain WHY code works a certain way, not WHAT it does
Never disable lint rules without strong justification
Files:
view/hooks/use-env-variables-editor.ts
view/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
view/**/*.{tsx,ts}: Each component file should export one main component
Never write plain HTML for interactive elements; always use shadcn components like Button, Card, Badge, Skeleton
Use thecn()utility function for conditional Tailwind classes
No hardcoded user-facing strings; always use i18n with theuseTranslationhook
Handle loading states by checkingisLoadingfrom RTK Query and returning skeleton loaders or loading components
Handle error states by checking error values from RTK Query and displaying error messages using i18n
Show empty state UI when data is empty using i18n messages
Always provide skeleton loaders for async content instead of blank/white space
Use semantic HTML elements for accessibility
Provide proper ARIA labels where needed for accessibility
Memoize expensive computations withuseMemo
Prevent unnecessary re-renders withuseCallback
Use React Query's caching effectively to avoid redundant API calls
Lazy load heavy components when appropriate
Ensure responsive design works on all screen sizes
Files:
view/hooks/use-env-variables-editor.ts
view/hooks/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
view/hooks/**/*.ts: Use generic patterns for typed hooks to handle type-safe data manipulation
Use JSDoc comments for complex hooks and functions to explain their purpose and parameters
Files:
view/hooks/use-env-variables-editor.ts
view/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
view/**/*.{ts,tsx}: Respect ESLint rules configured inview/eslint.config.mjs
Use Prettier for consistent formatting and fix all lint errors before committing
Files:
view/hooks/use-env-variables-editor.ts
🧠 Learnings (1)
📚 Learning: 2025-12-27T03:02:43.092Z
Learnt from: CR
Repo: raghavyuva/nixopus PR: 0
File: .cursor/rules/frontend.mdc:0-0
Timestamp: 2025-12-27T03:02:43.092Z
Learning: Applies to view/**/*.{tsx,ts} : Prevent unnecessary re-renders with `useCallback`
Applied to files:
view/hooks/use-env-variables-editor.ts
🔇 Additional comments (2)
view/hooks/use-env-variables-editor.ts (2)
3-3: LGTM! useRef import is necessary and properly used.The import addition is appropriate for the new initialization guard pattern.
51-51: LGTM! Ref declaration follows React best practices.The
hasInitializedref appropriately tracks initialization state across renders.
2 participants
Issue
Scope of Change
Select all applicable areas impacted by this PR:
Screenshot / Video / GIF (if applicable)
Developer Checklist
To be completed by the developer who raised the PR.
Reviewer Checklist
To be completed by the reviewer before merge.
Summary by CodeRabbit
Release Notes
Style
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.