feature(webapp): Show a v3 deprecation notice in the side bar#3090
feature(webapp): Show a v3 deprecation notice in the side bar#3090
Conversation
|
WalkthroughSideMenu now imports additional heroicons and Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/webapp/app/components/navigation/SideMenu.tsx (3)
48-49: Nit: Group heroicon imports together.
ExclamationTriangleIconfrom@heroicons/react/24/outlineis separated from the other@heroicons/react/20/solidimports at lines 1–24. Moving it adjacent to those keeps the import block scannable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/navigation/SideMenu.tsx` around lines 48 - 49, The ExclamationTriangleIcon import is separated from the other heroicons; move the import of ExclamationTriangleIcon so it sits next to the other `@heroicons/react` imports (the existing 20/solid imports) to keep the import block grouped and scannable—update the import statements at the top of SideMenu.tsx so all heroicon symbols (including ExclamationTriangleIcon) are adjacent.
628-633: Eliminate duplicateuseIncidentStatuspolling.Both
IncidentStatusPanel(line 602) andV3DeprecationPanel(line 629) independently calluseIncidentStatus(), creating two separate 60-second polling intervals. Lift the hook call to theSideMenuparent component and passhasIncidentas a prop to both consumers to avoid the redundant network requests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/navigation/SideMenu.tsx` around lines 628 - 633, Move the useIncidentStatus() call out of the child components and into the SideMenu component so polling runs once: remove the useIncidentStatus() calls from V3DeprecationPanel and IncidentStatusPanel, add a single const { hasIncident } = useIncidentStatus() in SideMenu, and pass hasIncident down as a prop to both V3DeprecationPanel and IncidentStatusPanel (update their param types to accept hasIncident: boolean) so they use the passed value instead of invoking useIncidentStatus() themselves.
603-603: Add clarity to the engine version mapping with a named constant or inline comment.The
isV3={project.engine === "V1"}mapping is correct—engine "V1" represents v3 projects and "V2" represents v4 projects—but the engine/product version mismatch can be confusing. Consider extracting a helper constant likeconst isV3Project = project.engine === "V1"near the top of the component or adding a brief inline comment to self-document this relationship.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/navigation/SideMenu.tsx` at line 603, The V3 deprecation prop is confusing because it passes isV3={project.engine === "V1"}; extract a clearly named boolean (e.g., const isV3Project = project.engine === "V1") near the top of the SideMenu component and use isV3={isV3Project} when rendering V3DeprecationPanel (or, if you prefer not to add a variable, add a one-line inline comment next to the prop explaining that engine "V1" maps to v3 projects) to make the engine-to-product-version mapping explicit and self-documenting.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/components/navigation/SideMenu.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/components/navigation/SideMenu.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/components/navigation/SideMenu.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/components/navigation/SideMenu.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/components/navigation/SideMenu.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/components/navigation/SideMenu.tsx
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/components/navigation/SideMenu.tsx
🧠 Learnings (2)
📚 Learning: 2026-02-03T18:27:49.039Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:553-555
Timestamp: 2026-02-03T18:27:49.039Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (like the Edit button with PencilSquareIcon) intentionally have no text labels - only icons are shown in the TableCellMenu. This is a deliberate UI design pattern for compact icon-only menu items.
Applied to files:
apps/webapp/app/components/navigation/SideMenu.tsx
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Applied to files:
apps/webapp/app/components/navigation/SideMenu.tsx
🧬 Code graph analysis (1)
apps/webapp/app/components/navigation/SideMenu.tsx (1)
apps/webapp/app/routes/resources.incidents.tsx (1)
useIncidentStatus(41-71)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (25)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: typecheck / typecheck
🔇 Additional comments (3)
apps/webapp/app/components/navigation/SideMenu.tsx (3)
635-677: Clean collapse/expand implementation.The dual
motion.divapproach with inverse height/opacity animations is a good pattern and stays consistent with the rest of the side menu's collapsible behavior. The popover for collapsed state provides a nice UX for showing the full deprecation content when the sidebar is narrow.
680-704: Deprecation content looks good.Clear messaging, appropriate visual treatment with amber theming, and a well-placed CTA. Implementation is clean.
693-701: No action required. The migration docs URL is correct and properly configured.The URL
https://trigger.dev/docs/migrating-from-v3is verified against the codebase documentation configuration:
- The docs file
docs/migrating-from-v3.mdxexistsdocs/docs.jsonconfirms the route slug"migrating-from-v3"with redirect mapping- The URL pattern is consistent with other documentation links across the codebase
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/webapp/app/components/navigation/SideMenu.tsx`:
- Around line 48-49: The ExclamationTriangleIcon import is separated from the
other heroicons; move the import of ExclamationTriangleIcon so it sits next to
the other `@heroicons/react` imports (the existing 20/solid imports) to keep the
import block grouped and scannable—update the import statements at the top of
SideMenu.tsx so all heroicon symbols (including ExclamationTriangleIcon) are
adjacent.
- Around line 628-633: Move the useIncidentStatus() call out of the child
components and into the SideMenu component so polling runs once: remove the
useIncidentStatus() calls from V3DeprecationPanel and IncidentStatusPanel, add a
single const { hasIncident } = useIncidentStatus() in SideMenu, and pass
hasIncident down as a prop to both V3DeprecationPanel and IncidentStatusPanel
(update their param types to accept hasIncident: boolean) so they use the passed
value instead of invoking useIncidentStatus() themselves.
- Line 603: The V3 deprecation prop is confusing because it passes
isV3={project.engine === "V1"}; extract a clearly named boolean (e.g., const
isV3Project = project.engine === "V1") near the top of the SideMenu component
and use isV3={isV3Project} when rendering V3DeprecationPanel (or, if you prefer
not to add a variable, add a one-line inline comment next to the prop explaining
that engine "V1" maps to v3 projects) to make the engine-to-product-version
mapping explicit and self-documenting.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/webapp/app/components/navigation/SideMenu.tsx (2)
706-709: Extract hardcoded deprecation dates as named constants.
"1 April 2026"and"1 July 2026"are buried in a JSX string. If the timeline shifts, these require a string-search to find. Named constants at module scope make them easy to locate and update in one place.♻️ Proposed refactor
+const V3_DEPLOY_STOP_DATE = "1 April 2026"; +const V3_SHUTDOWN_DATE = "1 July 2026"; function V3DeprecationContent() { return ( <div className="flex flex-col gap-2 rounded border border-amber-500/30 bg-amber-500/10 p-2 pt-1.5"> ... <Paragraph variant="extra-small/bright" className="text-amber-300"> - This is a v3 project. V3 deploys will stop working on 1 April 2026. Full shutdown is 1 July - 2026 where all v3 runs will stop executing. Migrate to v4 to avoid downtime. + This is a v3 project. V3 deploys will stop working on {V3_DEPLOY_STOP_DATE}. Full shutdown + is {V3_SHUTDOWN_DATE} where all v3 runs will stop executing. Migrate to v4 to avoid downtime. </Paragraph>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/navigation/SideMenu.tsx` around lines 706 - 709, Extract the hardcoded dates in SideMenu.tsx into module-level named constants (e.g., V3_DEPLOY_STOP_DATE and V3_FULL_SHUTDOWN_DATE) and replace the inline strings in the Paragraph JSX (the element using variant="extra-small/bright") with those constants so future timeline updates only require changing the constants at the top of the file.
639-695: Consider extracting a sharedCollapsibleSidePanelprimitive to eliminate structural duplication.
V3DeprecationPanelis structurally identical toIncidentStatusPanel: both wrap content in aPopover, animate between a full view and a collapsed icon trigger via twomotion.divs, and use the sameSimpleTooltip/PopoverTriggericon pattern. If a third panel is ever added, this triplicate boilerplate will be error-prone to maintain.A thin shared primitive (accepting
isCollapsed,collapsedIcon,tooltipContent, and achildrenrender slot) would eliminate the duplication.Also a minor nit: Line 690 has
className="w-52 !min-w-0 p-0"while the equivalent inIncidentStatusPanel(line 128 ofresources.incidents.tsx) is"!min-w-0 w-52 p-0"— inconsistent ordering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/navigation/SideMenu.tsx` around lines 639 - 695, V3DeprecationPanel duplicates the same Popover + two motion.div collapse/expand pattern used by IncidentStatusPanel; extract a CollapsibleSidePanel primitive that accepts props {isCollapsed, collapsedIcon, tooltipContent, children} (or similar) to render the Popover, the two motion.div states, the SimpleTooltip/PopoverTrigger icon, and the PopoverContent slot; then refactor V3DeprecationPanel and IncidentStatusPanel to render CollapsibleSidePanel with their specific collapsedIcon (ExclamationTriangleIcon), tooltipContent ("V3 deprecation warning"), and children (V3DeprecationContent or incident content); also normalize the PopoverContent className ordering (use consistent "!min-w-0 w-52 p-0") to match IncidentStatusPanel.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/routes/resources.incidents.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/routes/resources.incidents.tsxapps/webapp/app/components/navigation/SideMenu.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/routes/resources.incidents.tsxapps/webapp/app/components/navigation/SideMenu.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/routes/resources.incidents.tsxapps/webapp/app/components/navigation/SideMenu.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/routes/resources.incidents.tsxapps/webapp/app/components/navigation/SideMenu.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/routes/resources.incidents.tsxapps/webapp/app/components/navigation/SideMenu.tsx
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/routes/resources.incidents.tsxapps/webapp/app/components/navigation/SideMenu.tsx
🧠 Learnings (2)
📚 Learning: 2026-02-03T18:27:40.429Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:553-555
Timestamp: 2026-02-03T18:27:40.429Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (e.g., Edit with PencilSquareIcon) in the TableCellMenu are intentionally icon-only with no text labels as a compact UI pattern. This is a deliberate design choice for this route; preserve the icon-only behavior for consistency in this file.
Applied to files:
apps/webapp/app/routes/resources.incidents.tsx
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Applied to files:
apps/webapp/app/routes/resources.incidents.tsxapps/webapp/app/components/navigation/SideMenu.tsx
🧬 Code graph analysis (1)
apps/webapp/app/components/navigation/SideMenu.tsx (1)
apps/webapp/app/routes/resources.incidents.tsx (2)
useIncidentStatus(41-71)IncidentStatusPanel(73-133)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (25)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: sdk-compat / Deno Runtime
🔇 Additional comments (3)
apps/webapp/app/routes/resources.incidents.tsx (1)
73-83: LGTM — clean prop-lifting refactor.The signature change correctly externalizes data sourcing to the parent (
SideMenu), enablingV3DeprecationPanelinSideMenuto share the sameincidentStatusobject without a second fetch. The prop types match theuseIncidentStatus()return shape exactly.apps/webapp/app/components/navigation/SideMenu.tsx (2)
604-614: LGTM — prop threading and panel ordering are correct.
IncidentStatusPanelcorrectly receives all three props fromincidentStatus.V3DeprecationPanelis correctly gated byhasIncidentto yield to the incident panel when active. The mutual-exclusion logic (!isV3 || hasIncidentin the deprecation panel) matches the PR spec.
169-169: The engine mapping is correct; consider adding an inline comment for clarity.The variable
isV3Projectis correctly set whenengine === "V1"because V1 is the legacy engine for v3 SDK projects and V2 is the new managed engine for v4 SDK projects. This is confirmed by the upgrade paths throughout the codebase (createDeploymentBackgroundWorkerV3.server.ts, initializeDeployment.server.ts, etc.) which all upgrade projects from V1 to V2, and by DevPresence at line 345 which checksengine === "V2"for v4-specific features.The logic is sound but the naming is counter-intuitive—an inline comment explaining the mapping (e.g.,
// V1 = v3 SDK engine) would improve maintainability for future developers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/webapp/app/components/navigation/SideMenu.tsx`:
- Around line 706-709: Extract the hardcoded dates in SideMenu.tsx into
module-level named constants (e.g., V3_DEPLOY_STOP_DATE and
V3_FULL_SHUTDOWN_DATE) and replace the inline strings in the Paragraph JSX (the
element using variant="extra-small/bright") with those constants so future
timeline updates only require changing the constants at the top of the file.
- Around line 639-695: V3DeprecationPanel duplicates the same Popover + two
motion.div collapse/expand pattern used by IncidentStatusPanel; extract a
CollapsibleSidePanel primitive that accepts props {isCollapsed, collapsedIcon,
tooltipContent, children} (or similar) to render the Popover, the two motion.div
states, the SimpleTooltip/PopoverTrigger icon, and the PopoverContent slot; then
refactor V3DeprecationPanel and IncidentStatusPanel to render
CollapsibleSidePanel with their specific collapsedIcon
(ExclamationTriangleIcon), tooltipContent ("V3 deprecation warning"), and
children (V3DeprecationContent or incident content); also normalize the
PopoverContent className ordering (use consistent "!min-w-0 w-52 p-0") to match
IncidentStatusPanel.
ericallam
left a comment
ericallam
left a comment
There was a problem hiding this comment.
That "Migrate to v4 now" button makes it seem like clicking that button will migrate your project, which will lead to people not click on it because they'll be worried it will be a destructive action. Make it clear its a docs link I think
|
Good call @ericallam - i've changed it to this: 
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/webapp/app/components/navigation/SideMenu.tsx (1)
653-695: HiddenPopoverTriggerremains focusable when the sidebar is expanded.When
isCollapsedis false, the collapsed-statemotion.div(lines 668-689) animates toheight: 0/opacity: 0but thePopoverTriggerinside it stays in the DOM and is keyboard-focusable. A user tabbing through the sidebar could activate the popover and see the deprecation content duplicated in a popover alongside the inline panel.Consider conditionally rendering the collapsed block, or applying
aria-hiddenandtabIndex={-1}when not collapsed:Suggested approach
<motion.div initial={false} animate={{ height: isCollapsed ? "auto" : 0, opacity: isCollapsed ? 1 : 0, }} transition={{ duration: 0.15 }} - className="overflow-hidden" + className="overflow-hidden" + aria-hidden={!isCollapsed} + {...(!isCollapsed && { inert: "" })} >Alternatively, gate the entire collapsed block with
{isCollapsed && (...)}and remove the animation for that direction.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/navigation/SideMenu.tsx` around lines 653 - 695, The collapsed-state panel containing PopoverTrigger stays in the DOM and remains keyboard-focusable when isCollapsed is false; update the component so the collapsed block (the motion.div wrapping SimpleTooltip/PopoverTrigger) either is only rendered when isCollapsed (e.g., gate it with {isCollapsed && (...)} to remove the element entirely) or, if you keep animation, set accessibility attributes on the focusable element(s): add aria-hidden="true" and tabIndex={-1} to the PopoverTrigger (and any focusable children) when isCollapsed is false to prevent keyboard focus and duplicate popover activation; reference the motion.div, isCollapsed, PopoverTrigger and SimpleTooltip when making the change.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/components/navigation/SideMenu.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/components/navigation/SideMenu.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/components/navigation/SideMenu.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/components/navigation/SideMenu.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/components/navigation/SideMenu.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/components/navigation/SideMenu.tsx
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/components/navigation/SideMenu.tsx
🧠 Learnings (1)
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Applied to files:
apps/webapp/app/components/navigation/SideMenu.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (27)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: typecheck / typecheck
- GitHub Check: sdk-compat / Bun Runtime
🔇 Additional comments (3)
apps/webapp/app/components/navigation/SideMenu.tsx (3)
604-615: LGTM!Clean separation of concerns:
IncidentStatusPanelnow receives explicit props instead of deriving state internally, and theV3DeprecationPanelis correctly placed below it with proper conditional logic (isV3andhasIncident).
698-724: LGTM!Clear, well-structured deprecation content with appropriate urgency messaging and a direct link to the migration guide. The amber color scheme effectively communicates warning severity.
170-170: No action needed. Line 170 is correct:project.engine === "V1"correctly identifies v3 projects. The engine version scheme (V1 = v3 product, V2 = v4 product) is intentionally inverted from the product versioning, as confirmed by codebase patterns likerun.engine === "V1"being used to route to#getV3TaskRunContext.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/webapp/app/components/navigation/SideMenu.tsx`:
- Around line 653-695: The collapsed-state panel containing PopoverTrigger stays
in the DOM and remains keyboard-focusable when isCollapsed is false; update the
component so the collapsed block (the motion.div wrapping
SimpleTooltip/PopoverTrigger) either is only rendered when isCollapsed (e.g.,
gate it with {isCollapsed && (...)} to remove the element entirely) or, if you
keep animation, set accessibility attributes on the focusable element(s): add
aria-hidden="true" and tabIndex={-1} to the PopoverTrigger (and any focusable
children) when isCollapsed is false to prevent keyboard focus and duplicate
popover activation; reference the motion.div, isCollapsed, PopoverTrigger and
SimpleTooltip when making the change.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/webapp/app/components/navigation/SideMenu.tsx (1)
711-712: Consider extracting hardcoded deprecation dates to named constantsThe dates
"1 April 2026"and"1 July 2026"are embedded directly in UI copy. If dates slip or the message needs updating, it requires a text search through JSX. Extracting them makes future changes less error-prone.♻️ Proposed refactor
+const V3_DEPLOY_DEADLINE = "1 April 2026"; +const V3_SHUTDOWN_DEADLINE = "1 July 2026"; function V3DeprecationContent() { return ( ... <Paragraph variant="extra-small/bright" className="text-amber-300"> - This is a v3 project. V3 deploys will stop working on 1 April 2026. Full shutdown is 1 July - 2026 where all v3 runs will stop executing. Migrate to v4 to avoid downtime. + This is a v3 project. V3 deploys will stop working on {V3_DEPLOY_DEADLINE}. Full shutdown + is {V3_SHUTDOWN_DEADLINE} where all v3 runs will stop executing. Migrate to v4 to avoid + downtime. </Paragraph>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/navigation/SideMenu.tsx` around lines 711 - 712, The hardcoded dates in the SideMenu JSX ("1 April 2026" and "1 July 2026") should be extracted to named constants so future updates are easier; declare constants like V3_DEPRECATION_DATE and V3_FULL_SHUTDOWN_DATE (either at the top of the SideMenu.tsx module or in a shared constants file) and replace the literal strings in the component's render/JSX with those constants (e.g., use {V3_DEPRECATION_DATE} and {V3_FULL_SHUTDOWN_DATE} inside the message) ensuring any tests or i18n handling are updated accordingly.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/components/navigation/SideMenu.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/components/navigation/SideMenu.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/components/navigation/SideMenu.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/components/navigation/SideMenu.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/components/navigation/SideMenu.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/components/navigation/SideMenu.tsx
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/components/navigation/SideMenu.tsx
🧠 Learnings (1)
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Applied to files:
apps/webapp/app/components/navigation/SideMenu.tsx
🧬 Code graph analysis (1)
apps/webapp/app/components/navigation/SideMenu.tsx (1)
apps/webapp/app/routes/resources.incidents.tsx (2)
useIncidentStatus(41-71)IncidentStatusPanel(73-133)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (27)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: sdk-compat / Cloudflare Workers
🔇 Additional comments (1)
apps/webapp/app/components/navigation/SideMenu.tsx (1)
714-724: No action required —LinkButtonautomatically handlesrel="noopener noreferrer"The
LinkButtoncomponent delegates external URLs (starting with "http") to theExtLinkcomponent, which hardcodes bothtarget="_blank"andrel="noopener noreferrer"on the underlying<a>element. The migration guide link is already secure and follows best practices.Likely an incorrect or invalid review comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/webapp/app/components/navigation/SideMenu.tsx`:
- Around line 711-712: The hardcoded dates in the SideMenu JSX ("1 April 2026"
and "1 July 2026") should be extracted to named constants so future updates are
easier; declare constants like V3_DEPRECATION_DATE and V3_FULL_SHUTDOWN_DATE
(either at the top of the SideMenu.tsx module or in a shared constants file) and
replace the literal strings in the component's render/JSX with those constants
(e.g., use {V3_DEPRECATION_DATE} and {V3_FULL_SHUTDOWN_DATE} inside the message)
ensuring any tests or i18n handling are updated accordingly.
2 participants
CleanShot.2026-02-19.at.08.17.32.mp4