Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughMoves environment-variable management out of advanced settings into a new dedicated Env Vars area: adds an Env Vars page with multiple UI components and a SlidePanel primitive, updates backend shapes to use Changes
Sequence Diagram(s)mermaid UI->>Slide: open add panel Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 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📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ogzhanolguncu
requested review from
Flo4604,
MichaelUnkey,
chronark,
imeyer and
mcstepp
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/apps/dashboard/lib/collections/deploy/env-vars.ts (1)
55-62:⚠️ Potential issue | 🔴 CriticalSeparate masked display values from persistable secrets.
Line 61 now hydrates
EnvVar.valuefrom the list payload, butweb/apps/dashboard/lib/trpc/routers/deploy/env-vars/list.ts:64-71now hardcodes that field to"••••••••". Line 106 still round-tripsmodified.valueintodeploy.envVar.update, so an edit that changes only the key/type or other metadata can overwrite the real secret with the mask. Please model masked values separately or make unchanged secrets impossible to send back through this update path. As per coding guidelines, "Make illegal states unrepresentable by modeling domain with discriminated unions and parsing inputs at boundaries into typed structures".Also applies to: 102-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/apps/dashboard/lib/collections/deploy/env-vars.ts` around lines 55 - 62, The code is currently round-tripping a masked display string into the persistable secret (EnvVar.value) causing masked "••••••••" to overwrite real secrets; update the domain and update path so displayed masked values are modeled separately (e.g., add a display/maskedValue or isMasked flag on the EnvVar DTO returned by web/apps/dashboard/lib/collections/deploy/env-vars.ts and in trpc output), and change the update flow (deploy.envVar.update handler and the UI code that sends modified.value) to never send the masked display back as the persisted value—either ignore value when the client indicates isMasked or detect the mask sentinel and treat it as "unchanged". Ensure parsing at the TRPC boundary converts incoming payloads into a discriminated shape so masked display values are not accepted as valid secrets and only real, non-masked values reach deploy.envVar.update.web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/use-drop-zone.ts (1)
20-30:⚠️ Potential issue | 🟡 MinorSkip blank keys at the parse boundary.
Lines 20-30 currently accept
=valueand produce{ key: "", value }, which then gets imported as a blank-key row instead of being rejected as invalid input.As per coding guidelines, "Make illegal states unrepresentable by modeling domain with discriminated unions and parsing inputs at boundaries into typed structures".🔧 Minimal fix
const key = trimmed.slice(0, eqIndex).trim(); let value = trimmed.slice(eqIndex + 1).trim(); + if (key === "") { + return null; + } if (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/use-drop-zone.ts around lines 20 - 30, The parser in use-drop-zone.ts currently accepts lines like "=value" and returns { key: "", value }, so change the boundary to reject blank keys: after computing const key = trimmed.slice(0, eqIndex).trim(), if key === "" then do not return a {key,value} pair (return null/undefined or a discriminated invalid result) so callers skip/importers won't create blank-key rows; update the parsing function (the block that computes key/value and returns { key, value }) to return an explicit failure for empty keys and adjust any caller handling to ignore those failures.
🧹 Nitpick comments (1)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-vars-toolbar.tsx (1)
17-19: Drop the type assertion in the sort guard.This works, but the
as readonly string[]cast is avoidable.some()keeps the same behavior without punching through the type boundary.Suggested fix
function isSortOption(value: string): value is SortOption { - return (SORT_OPTIONS as readonly string[]).includes(value); + return SORT_OPTIONS.some((option) => option === value); }As per coding guidelines,
**/*.{ts,tsx}: Never compromise type safety: Noany, no!(non-null assertion), noas Type.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-vars-toolbar.tsx around lines 17 - 19, The type guard is using a type assertion on SORT_OPTIONS; change isSortOption to avoid the cast by using a runtime predicate like SORT_OPTIONS.some(opt => opt === value) instead of (SORT_OPTIONS as readonly string[]).includes(value) so the function isSortOption(value: string): value is SortOption retains type safety and no `as` assertion is used; update the implementation to compare via .some against SORT_OPTIONS.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-var-edit-row.tsx:
- Around line 78-88: The edit form updates description locally but the TRPC
mutation and collection onUpdate don't send it to the server; update the
envVar.update mutation input schema (add a description?: string | null to the
input for envVarId, key, value, type) and modify the mutation handler to persist
description, then update the collection's onUpdate handler (the code that calls
envVar.update) to pass values.description (or null) along with envVarId, key,
value, type so description changes are sent and saved; alternatively remove the
description field from the edit form if you prefer not to support it.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-var-value-cell.tsx:
- Around line 43-50: In handleToggleReveal, when hiding the value (the branch
where visible is true), also clear the plaintext state so the secret is revoked
immediately; call the decrypted value setter (setDecryptedValue or whichever
state updater holds decryptedValue) to reset it (e.g., to empty string or
undefined) in addition to setVisible(false) and
clearTimeout(hideTimeoutRef.current), so manual hide removes the decrypted
secret from component state.
- Around line 71-83: The handleCopy handler currently calls
navigator.clipboard.writeText without awaiting it and always shows success; make
handleCopy async (or return the promise) and await
navigator.clipboard.writeText(decryptedValue) inside a try/catch, and only on
success call setCopied(true), toast.success("Copied to clipboard"), clear/set
copyTimeoutRef.current and call startAutoHide; in the catch branch show an error
(e.g. toast.error) and do not set copied; ensure you reference handleCopy,
copyTimeoutRef, setCopied, toast.success/toast.error and startAutoHide when
applying the change.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/schema.ts:
- Around line 3-7: The key field in envVarEntrySchema accepts whitespace-only
names; update the schema so the key is trimmed before validation and required to
be non-empty: change the key validator in envVarEntrySchema to trim the input
(e.g., use z.string().transform or a refine that applies s.trim()) and then
enforce min(1) or a trimmed-length check so keys like " " fail and
duplicate/conflict checks operate on the trimmed value.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/use-drop-zone.ts:
- Around line 54-69: The import currently drops in-progress rows because
existing is computed with getValues("envVars").filter((row) => row.key !== ""),
so preserve rows with empty keys that contain user input by changing how
existing is derived: compute existingKeys from only rows with non-empty key, but
build existing as all current envVars where either key !== "" OR (key === "" and
(value or description or other editable fields are non-empty or there's a
focused/dirty flag)); then compute newRows = [...deduped.values()].filter(row =>
!existingKeys.has(row.key)) as before and call reset with envVars: [...existing,
...newRows.map(row => ({...row, description: ""}))]; reference
functions/variables: getValues, reset, envVars, deduped, existingKeys, newRows
to locate and implement the change.
In `@web/apps/dashboard/components/ui/switch.tsx`:
- Around line 16-24: The small-switch consumers still use the old 32/14 geometry
so their padding and translate values no longer line up with the updated shared
Switch sizes; update the consumer components (the Protection switch component in
protection-switch.tsx and the EnvVarSecretSwitch in env-var-secret-switch.tsx)
to use the new base geometry: set the track to 40x20 with p-0.5 and the thumb to
20x20, and ensure the checked translate uses translate-x-4 (or remove custom
sizing so they inherit the shared Switch classes and SwitchPrimitives.Thumb
behavior); adjust any custom p-[1px] or translate-x-3 values accordingly so the
thumb lands exactly on the rail when checked.
In `@web/internal/ui/src/components/slide-panel.tsx`:
- Around line 67-88: The panel's children remain mounted and focusable when
isOpen is false; update the slide panel to remove it from the accessibility/tab
order during the closed state by (a) adding aria-hidden={ !isOpen } and the
inert attribute (or setting pointer-events: none and user-select: none) on the
root div when closed to hide it from AT and block interaction, and (b) implement
a small mounted/visible state that delays unmount until the exit animation
completes (listen for transitionend or use a timer) so you unmount children
after the fade/translate finishes; reference the root div using isOpen,
children, widthClassName, className and topOffset to implement these changes.
- Around line 26-35: SlidePanelRoot currently calls createPortal/document.body
during render which breaks in Next.js App Router prerendering; add a mounted
state guard in SlidePanelRoot (e.g., const [mounted, setMounted] =
useState(false) and setMounted(true) in useEffect) and return null until mounted
is true, then call createPortal so document.body is only accessed on the client
after mount; update any other portal usage in this component to use the same
mounted guard.
---
Outside diff comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/use-drop-zone.ts:
- Around line 20-30: The parser in use-drop-zone.ts currently accepts lines like
"=value" and returns { key: "", value }, so change the boundary to reject blank
keys: after computing const key = trimmed.slice(0, eqIndex).trim(), if key ===
"" then do not return a {key,value} pair (return null/undefined or a
discriminated invalid result) so callers skip/importers won't create blank-key
rows; update the parsing function (the block that computes key/value and returns
{ key, value }) to return an explicit failure for empty keys and adjust any
caller handling to ignore those failures.
In `@web/apps/dashboard/lib/collections/deploy/env-vars.ts`:
- Around line 55-62: The code is currently round-tripping a masked display
string into the persistable secret (EnvVar.value) causing masked "••••••••" to
overwrite real secrets; update the domain and update path so displayed masked
values are modeled separately (e.g., add a display/maskedValue or isMasked flag
on the EnvVar DTO returned by
web/apps/dashboard/lib/collections/deploy/env-vars.ts and in trpc output), and
change the update flow (deploy.envVar.update handler and the UI code that sends
modified.value) to never send the masked display back as the persisted
value—either ignore value when the client indicates isMasked or detect the mask
sentinel and treat it as "unchanged". Ensure parsing at the TRPC boundary
converts incoming payloads into a discriminated shape so masked display values
are not accepted as valid secrets and only real, non-masked values reach
deploy.envVar.update.
---
Nitpick comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-vars-toolbar.tsx:
- Around line 17-19: The type guard is using a type assertion on SORT_OPTIONS;
change isSortOption to avoid the cast by using a runtime predicate like
SORT_OPTIONS.some(opt => opt === value) instead of (SORT_OPTIONS as readonly
string[]).includes(value) so the function isSortOption(value: string): value is
SortOption retains type safety and no `as` assertion is used; update the
implementation to compare via .some against SORT_OPTIONS.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f9939ede-008b-4ee3-918b-d4531d361428
📒 Files selected for processing (39)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/details/env-variables-section/components/env-var-secret-switch.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/add-env-var-expandable.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-var-action-menu.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-var-edit-row.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-var-name-cell.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-var-row.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-var-value-cell.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-vars-empty.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-vars-header.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-vars-list.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-vars-skeleton.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-vars-toolbar.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/highlight-match.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/schema.tsweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/use-drop-zone.tsweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/page.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/layout-provider.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/navigations/create-deployment-button.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/navigations/project-navigation.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/navigations/use-breadcrumb-config.tsweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/advanced-settings/env-vars/env-var-row.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/advanced-settings/env-vars/index.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/advanced-settings/env-vars/schema.tsweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/advanced-settings/env-vars/use-decrypted-values.tsweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/advanced-settings/env-vars/utils.tsweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/deployment-settings.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/pending-redeploy-banner.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/layout.tsxweb/apps/dashboard/components/navbar-popover.tsxweb/apps/dashboard/components/navigation/sidebar/navigation-configs.tsxweb/apps/dashboard/components/ui/switch.tsxweb/apps/dashboard/lib/collections/deploy/env-vars.tsweb/apps/dashboard/lib/collections/deploy/environment-settings.tsweb/apps/dashboard/lib/trpc/routers/deploy/env-vars/list.tsweb/internal/icons/src/icons/brackets-square-dots.tsxweb/internal/icons/src/icons/note-3.tsxweb/internal/icons/src/index.tsweb/internal/ui/src/components/slide-panel.tsxweb/internal/ui/src/index.ts
💤 Files with no reviewable changes (6)
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/deployment-settings.tsx
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/advanced-settings/env-vars/schema.ts
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/advanced-settings/env-vars/use-decrypted-values.ts
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/advanced-settings/env-vars/env-var-row.tsx
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/advanced-settings/env-vars/index.tsx
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/settings/components/advanced-settings/env-vars/utils.ts
...pp)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-var-edit-row.tsx
Show resolved
Hide resolved
...)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-var-value-cell.tsx
Show resolved
Hide resolved
...)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-var-value-cell.tsx
Show resolved
Hide resolved
...oard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/schema.ts
Show resolved
Hide resolved
...p/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/use-drop-zone.ts
Outdated
Show resolved
Hide resolved
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
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/use-drop-zone.ts:
- Around line 98-103: The importFile callback currently calls await file.text()
without handling possible failures; wrap the file.text() call inside a try/catch
in the importFile useCallback (the function named importFile that depends on
importText) and on error surface a user-visible failure (e.g., call an existing
error handler or show a toast/notification and optionally log the error via
console/processLogger); on success continue to call importText(await
file.text()) as before. Ensure the catch passes the error context (file
name/type) to the error reporting mechanism so users get feedback when file.read
fails.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b5600fa3-252c-492c-a9d6-4da38eeb792e
📒 Files selected for processing (8)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/details/env-variables-section/components/env-var-secret-switch.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-var-value-cell.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/schema.tsweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/use-drop-zone.tsweb/apps/dashboard/components/dashboard/metadata/protection-switch.tsxweb/apps/dashboard/lib/collections/deploy/env-vars.tsweb/apps/dashboard/lib/trpc/routers/deploy/env-vars/update.tsweb/internal/ui/src/components/slide-panel.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/details/env-variables-section/components/env-var-secret-switch.tsx
- web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/schema.ts
Comment on lines
+98
to
+103
| const importFile = useCallback( | ||
| async (file: File) => { | ||
| importText(await file.text()); | ||
| }, | ||
| [importText], | ||
| ); |
Contributor
There was a problem hiding this comment.
Add error handling for file read failures.
file.text() can throw (e.g., corrupted file, read permission denied). Without a try-catch, the user receives no feedback on failure.
🛡️ Proposed fix
const importFile = useCallback(
async (file: File) => {
- importText(await file.text());
+ try {
+ importText(await file.text());
+ } catch {
+ toast.error("Failed to read file");
+ }
},
[importText],
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/use-drop-zone.ts
around lines 98 - 103, The importFile callback currently calls await file.text()
without handling possible failures; wrap the file.text() call inside a try/catch
in the importFile useCallback (the function named importFile that depends on
importText) and on error surface a user-visible failure (e.g., call an existing
error handler or show a toast/notification and optionally log the error via
console/processLogger); on success continue to call importText(await
file.text()) as before. Ensure the catch passes the error context (file
name/type) to the error reporting mechanism so users get feedback when file.read
fails.
chronark
approved these changes
Mar 30, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-var-value-cell.tsx (1)
47-51:⚠️ Potential issue | 🟡 MinorReset copy state when manually hiding the secret.
Manual hide clears plaintext, but
copiedandcopyTimeoutRefare left intact. If the value is re-revealed quickly, tooltip state can incorrectly stay as “Copied!”.Suggested patch
if (visible) { setVisible(false); setDecryptedValue(undefined); + setCopied(false); + clearTimeout(copyTimeoutRef.current); clearTimeout(hideTimeoutRef.current); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-var-value-cell.tsx around lines 47 - 51, When manually hiding the secret in the env-var-value-cell component (the branch that runs when visible is true and calls setVisible(false), setDecryptedValue(undefined), clearTimeout(hideTimeoutRef.current)), also reset the copy state by calling setCopied(false) and clearTimeout(copyTimeoutRef.current) so the “Copied!” tooltip does not persist if the value is quickly re-revealed; update the same hide branch to clear the copy timeout and reset copied to false.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/add-env-var-expandable.tsx:
- Around line 187-192: The DoubleChevronRight icon uses the Tailwind group-hover
utility but its parent SlidePanel.Close trigger lacks the required group class;
update the SlidePanel.Close element (the parent with className="mt-0.5
inline-flex ...") to include "group" (e.g., append "group" to its className) so
the group-hover:text-gray-12 on DoubleChevronRight takes effect, preserving the
existing classes and transitions.
- Around line 79-88: The effect persistFormState currently runs on mount and
calls saveCurrentValues when isOpen is false; change it to run only on
open/close transitions by skipping the initial mount: add a ref (e.g.,
hasMountedRef) initialized false, then inside the useEffect for persistFormState
check if hasMountedRef.current is false — if so set it true and return early;
otherwise perform the existing logic that calls loadSavedValues() when isOpen
becomes true and saveCurrentValues() when it becomes false. Ensure the ref and
checks reference the same effect that uses isOpen, loadSavedValues, and
saveCurrentValues so initial render does not invoke saveCurrentValues().
In `@web/internal/ui/src/components/slide-panel.tsx`:
- Around line 67-90: The panel root DIV in the SlidePanel component (the element
using isOpen, side, widthClassName, className, children) needs explicit modal
semantics: add role="dialog" and aria-modal="true" and provide a stable
accessible name—prefer aria-labelledby pointing to the panel title/header ID if
a header element exists, otherwise add and surface an aria-label prop on the
SlidePanel component and apply it to the root DIV; ensure the chosen label prop
or header ID is documented/required so assistive tech can announce the modal.
---
Duplicate comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-var-value-cell.tsx:
- Around line 47-51: When manually hiding the secret in the env-var-value-cell
component (the branch that runs when visible is true and calls
setVisible(false), setDecryptedValue(undefined),
clearTimeout(hideTimeoutRef.current)), also reset the copy state by calling
setCopied(false) and clearTimeout(copyTimeoutRef.current) so the “Copied!”
tooltip does not persist if the value is quickly re-revealed; update the same
hide branch to clear the copy timeout and reset copied to false.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 958af591-0a74-4dee-86a8-6737a04c4b33
📒 Files selected for processing (3)
web/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/add-env-var-expandable.tsxweb/apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/env-var-value-cell.tsxweb/internal/ui/src/components/slide-panel.tsx
Comment on lines
+79
to
+88
| useEffect( | ||
| function persistFormState() { | ||
| if (isOpen) { | ||
| loadSavedValues(); | ||
| } else { | ||
| saveCurrentValues(); | ||
| } | ||
| }, | ||
| [isOpen, loadSavedValues, saveCurrentValues], | ||
| ); |
Contributor
There was a problem hiding this comment.
Persist/load should run on open↔close transitions, not initial mount.
At Line 83-85, the initial render with isOpen === false calls saveCurrentValues() immediately, which can overwrite an existing saved draft before the first open.
Suggested minimal patch
+ const prevIsOpenRef = useRef(isOpen);
+
useEffect(
function persistFormState() {
- if (isOpen) {
+ if (isOpen && !prevIsOpenRef.current) {
loadSavedValues();
- } else {
+ } else if (!isOpen && prevIsOpenRef.current) {
saveCurrentValues();
}
+ prevIsOpenRef.current = isOpen;
},
[isOpen, loadSavedValues, saveCurrentValues],
);📝 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.
Suggested change
| useEffect( | |
| function persistFormState() { | |
| if (isOpen) { | |
| loadSavedValues(); | |
| } else { | |
| saveCurrentValues(); | |
| } | |
| }, | |
| [isOpen, loadSavedValues, saveCurrentValues], | |
| ); | |
| const prevIsOpenRef = useRef(isOpen); | |
| useEffect( | |
| function persistFormState() { | |
| if (isOpen && !prevIsOpenRef.current) { | |
| loadSavedValues(); | |
| } else if (!isOpen && prevIsOpenRef.current) { | |
| saveCurrentValues(); | |
| } | |
| prevIsOpenRef.current = isOpen; | |
| }, | |
| [isOpen, loadSavedValues, saveCurrentValues], | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/add-env-var-expandable.tsx
around lines 79 - 88, The effect persistFormState currently runs on mount and
calls saveCurrentValues when isOpen is false; change it to run only on
open/close transitions by skipping the initial mount: add a ref (e.g.,
hasMountedRef) initialized false, then inside the useEffect for persistFormState
check if hasMountedRef.current is false — if so set it true and return early;
otherwise perform the existing logic that calls loadSavedValues() when isOpen
becomes true and saveCurrentValues() when it becomes false. Ensure the ref and
checks reference the same effect that uses isOpen, loadSavedValues, and
saveCurrentValues so initial render does not invoke saveCurrentValues().
Comment on lines
+187
to
+192
| className="mt-0.5 inline-flex items-center justify-center size-9 rounded-md hover:bg-grayA-3 transition-colors cursor-pointer" | ||
| > | ||
| <DoubleChevronRight | ||
| iconSize="lg-medium" | ||
| className="text-gray-10 transition-transform duration-300 ease-out group-hover:text-gray-12" | ||
| /> |
Contributor
There was a problem hiding this comment.
group-hover styles won’t apply without a group parent class.
At Line 191, the icon uses group-hover:text-gray-12, but the SlidePanel.Close trigger at Line 187 is missing group, so the hover color transition never activates.
Suggested minimal patch
<SlidePanel.Close
aria-label="Close panel"
- className="mt-0.5 inline-flex items-center justify-center size-9 rounded-md hover:bg-grayA-3 transition-colors cursor-pointer"
+ className="group mt-0.5 inline-flex items-center justify-center size-9 rounded-md hover:bg-grayA-3 transition-colors cursor-pointer"
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/projects/[projectId]/(overview)/env-vars/components/add-env-var-expandable.tsx
around lines 187 - 192, The DoubleChevronRight icon uses the Tailwind
group-hover utility but its parent SlidePanel.Close trigger lacks the required
group class; update the SlidePanel.Close element (the parent with
className="mt-0.5 inline-flex ...") to include "group" (e.g., append "group" to
its className) so the group-hover:text-gray-12 on DoubleChevronRight takes
effect, preserving the existing classes and transitions.
Comment on lines
+67
to
+90
| <div | ||
| aria-hidden={!isOpen} | ||
| inert={!isOpen || undefined} | ||
| className={cn( | ||
| "fixed dark:bg-black bg-white border border-gray-4 rounded-xl overflow-hidden z-51", | ||
| "transition-[transform,opacity] duration-300 ease-out", | ||
| "shadow-md", | ||
| side === "right" ? "right-3" : "left-3", | ||
| isOpen | ||
| ? "translate-x-0 opacity-100" | ||
| : side === "right" | ||
| ? "translate-x-full opacity-0" | ||
| : "-translate-x-full opacity-0", | ||
| widthClassName, | ||
| className, | ||
| )} | ||
| style={{ | ||
| top: `${topOffset + 12}px`, | ||
| height: `calc(100vh - ${topOffset + 24}px)`, | ||
| willChange: isOpen ? "transform, opacity" : "auto", | ||
| }} | ||
| > | ||
| <div className="h-full flex flex-col">{children}</div> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Add explicit modal semantics to the panel root.
At Line 67, the panel behaves like a modal but is missing role="dialog" and aria-modal="true" (and a stable accessible name), so assistive tech won’t consistently announce modal context.
Suggested minimal patch
type SlidePanelRootProps = {
children: React.ReactNode;
isOpen: boolean;
onClose: () => void;
+ "aria-label"?: string;
side?: "left" | "right";
topOffset?: number;
widthClassName?: string;
className?: string;
};
@@
const SlidePanelRoot = ({
children,
isOpen,
onClose,
+ "aria-label": ariaLabel = "Slide panel",
side = "right",
topOffset = 0,
widthClassName = "w-175",
className,
}: SlidePanelRootProps) => {
@@
<div
+ role="dialog"
+ aria-modal="true"
+ aria-label={ariaLabel}
aria-hidden={!isOpen}
inert={!isOpen || undefined}
className={cn(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/internal/ui/src/components/slide-panel.tsx` around lines 67 - 90, The
panel root DIV in the SlidePanel component (the element using isOpen, side,
widthClassName, className, children) needs explicit modal semantics: add
role="dialog" and aria-modal="true" and provide a stable accessible name—prefer
aria-labelledby pointing to the panel title/header ID if a header element
exists, otherwise add and surface an aria-label prop on the SlidePanel component
and apply it to the root DIV; ensure the chosen label prop or header ID is
documented/required so assistive tech can announce the modal.
perkinsjr
approved these changes
Mar 30, 2026
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
3 participants
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.
What does this PR do?
This PR:
How should this be tested?