Adds a "dev connected" banner to the top of the Run and Runs list pages #1855
Conversation
|
WalkthroughThis pull request introduces new components and refactors existing ones to improve how the development server's connection status is displayed. New components ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SideMenu
participant ConnectionIcon
participant DevDisconnectedBanner
User->>SideMenu: Interacts with SideMenu
SideMenu->>ConnectionIcon: Request connection status
ConnectionIcon-->>SideMenu: Render appropriate icon
SideMenu->>DevDisconnectedBanner: Check connection status
DevDisconnectedBanner-->>SideMenu: Display banner if disconnected
sequenceDiagram
participant User
participant PageComponent
participant useDevPresence
participant DevDisconnectedBanner
User->>PageComponent: Loads page
PageComponent->>useDevPresence: Retrieve connection status
useDevPresence-->>PageComponent: Return isConnected flag
PageComponent->>DevDisconnectedBanner: Conditional render check (env = DEVELOPMENT)
DevDisconnectedBanner-->>PageComponent: Display connection status banner
Possibly related PRs
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/webapp/app/components/DevPresence.tsx (1)
107-158: Centralized connection dialog logic.
Using theDialogfor a visual alert on server connectivity is user-friendly and well-organized. The fallback text is clear (“Checking connection...”). You may want to add a small test case for undefined vs. true vs. false states to ensure the dialog content is correct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/webapp/app/components/DevPresence.tsx(2 hunks)apps/webapp/app/components/navigation/SideMenu.tsx(3 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx(2 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/route.tsx(2 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (1)
apps/webapp/app/components/DevPresence.tsx (1)
DevPresenceBanner(160-191)
apps/webapp/app/components/DevPresence.tsx (2)
apps/webapp/app/assets/icons/ConnectionIcons.tsx (3)
CheckingConnectionIcon(50-73)ConnectedIcon(1-28)DisconnectedIcon(30-48)apps/webapp/app/utils/pathBuilder.ts (1)
docsPath(417-419)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
🔇 Additional comments (13)
apps/webapp/app/components/DevPresence.tsx (4)
1-7: Imports look consistent and well-structured.
No issues with addingAnimatePresenceandmotionfromframer-motion, as well as the icon imports.
12-19: Proper imports for path building and images.
These references are used correctly, and having separate images for connected vs. disconnected states is a clear design.
95-105: Straightforward and readable connection icon logic.
The component handles the undefined vs. boolean states cleanly. Consider logging, or gracefully handling unexpected values ifisConnectedever falls out of expected states.
160-190: Banner component integrated with framer-motion.
The fade-in/out transition, specifically forDEVELOPMENTenvironment, is well-structured. It’s concise and keeps the UI elements separated from logic.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/route.tsx (3)
11-11: Newly added imports for DevPresence.
This import ensures availability of the dev-connected banner. Everything appears correct.
165-166: Accessing isConnected via useDevPresence.
Good usage of the hook for retrieving the server connection status. No issues spotted.
172-184: Conditional dev banner with fade animation.
This fade effect is straightforward, improving the user experience with minimal overhead. Nicely done.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (3)
24-24: Expanded framer-motion import.
Allowing usage ofAnimatePresencefor transitions is consistent with other code sections.
30-30: Importing DevPresence functionality.
Clean separation of dev presence logic in its own module. This keeps route code focused on displaying relevant banners.
198-210: Extended fade animation for the runs detail page.
Matching the existing pattern from other pages, providing a unified user experience. Looks good.apps/webapp/app/components/navigation/SideMenu.tsx (3)
23-25: New icon imports for runs and waitpoints.
These icons are used consistently with the rest of the menu. No concerns here.
60-63: DevPresence imports in SideMenu.
The approach of having the inline connectivity check is simplified and removes duplication from older code. Great structure.
156-183: Inline DevConnection usage under DEVELOPMENT.
This is a neat way to show or hide connectivity status within the environment selector. The tooltip logic is handled properly, and the messaging is intuitive. Consider a fallback scenario for unexpected states ofisConnected, though it should be fine as typed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/webapp/app/components/DevPresence.tsx (1)
96-126: Unnecessary dependency in useEffectThe
environment.typeis included in the dependency array but isn't used inside the effect function.- }, [environment.type, project.engine, logCount, isConnected]); + }, [project.engine, logCount, isConnected]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/webapp/app/components/DevPresence.tsx(2 hunks)apps/webapp/app/components/navigation/SideMenu.tsx(5 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx(3 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/route.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs._index/route.tsx
- apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx
🧰 Additional context used
🧬 Code Definitions (1)
apps/webapp/app/components/DevPresence.tsx (2)
apps/webapp/app/assets/icons/ConnectionIcons.tsx (3)
CheckingConnectionIcon(50-73)ConnectedIcon(1-28)DisconnectedIcon(30-48)apps/webapp/app/utils/pathBuilder.ts (1)
docsPath(417-419)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
apps/webapp/app/components/navigation/SideMenu.tsx (4)
58-59: Good integration of the DevPresence componentsThe import of the new components from the DevPresence module looks good and aligns with the PR objective of refactoring the connection functionality into a dedicated module.
83-83: Appropriate type extensionAdding the
engineproperty to theSideMenuProjecttype is necessary to support the new connection status logic that differentiates between V1 and V2 engines.
107-107: Clean hook usageGood use of the new
useDevPresencehook to access the connection state.
156-182: Well-implemented conditional connection status displayThis implementation correctly:
- Only shows the connection status for development environments with V2 engines
- Uses the ConnectionIcon component to visually represent the connection state
- Provides helpful tooltips that change based on connection status
- Integrates the DevPresencePanel for detailed information
apps/webapp/app/components/DevPresence.tsx (3)
128-137: Well-implemented connection icon componentThe
ConnectionIconcomponent properly handles all three possible states (checking, connected, disconnected) and renders the appropriate icon with consistent sizing.
139-181: Comprehensive connection status panelThe
DevPresencePaneleffectively:
- Shows the appropriate header based on connection state
- Displays relevant imagery for the connection status
- Provides clear instructions when disconnected
- Links to documentation for additional help
This is a good user experience enhancement for developers trying to understand connection issues.
183-210: Good implementation of the banner componentThe
DevDisconnectedBannereffectively implements the PR's main objective of adding a banner when disconnected. The animation provides a smooth user experience, and the conditional rendering ensures it only appears when needed. Good integration with theDevPresencePanelcomponent for providing more detailed information.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/webapp/app/components/DevPresence.tsx (3)
96-137: Consider removing unused dependency from useEffect.The
useCrossEngineIsConnectedhook correctly implements different connection detection logic based on the project engine type. However, there's an unnecessary dependency in the useEffect.- }, [environment.type, project.engine, logCount, isConnected, isCompleted]); + }, [project.engine, logCount, isConnected, isCompleted]);The
environment.typevariable is included in the dependency array but isn't used within the effect body, potentially causing unnecessary effect executions.
125-133: Consider adding debounce to prevent connection status flicker.The current implementation uses a timeout to determine connection status for V1 engines with low log counts. This approach could lead to UI flicker if the log count hovers around the threshold.
Consider implementing a debounce mechanism to make the connection status more stable:
+ const [debouncedLogCount, setDebouncedLogCount] = useState(logCount); + + useEffect(() => { + const timer = setTimeout(() => { + setDebouncedLogCount(logCount); + }, 1000); + return () => clearTimeout(timer); + }, [logCount]); if (project.engine === "V1") { if (isCompleted) { setCrossEngineIsConnected(true); return; } - if (logCount <= 1) { + if (debouncedLogCount <= 1) { const timer = setTimeout(() => { setCrossEngineIsConnected(false); }, 5000); return () => clearTimeout(timer); } else { setCrossEngineIsConnected(true); } }
150-192: Extract repeated text to constants.The
DevPresencePanelcomponent contains duplicated text strings that could be extracted to constants for better maintainability.export function DevPresencePanel({ isConnected }: { isConnected: boolean | undefined }) { + const getHeaderText = () => { + if (isConnected === undefined) return "Checking connection..."; + return isConnected ? "Your dev server is connected" : "Your dev server is not connected"; + }; + + const getStatusText = () => { + if (isConnected === undefined) return "Checking connection..."; + return isConnected + ? "Your local dev server is connected to Trigger.dev" + : "Your local dev server is not connected to Trigger.dev"; + }; return ( <DialogContent> <DialogHeader> - {isConnected === undefined - ? "Checking connection..." - : isConnected - ? "Your dev server is connected" - : "Your dev server is not connected"} + {getHeaderText()} </DialogHeader> <div className="mt-2 flex flex-col gap-3 px-2"> <div className="flex flex-col items-center justify-center gap-6 px-6 py-10"> <img src={isConnected === true ? connectedImage : disconnectedImage} alt={isConnected === true ? "Connected" : "Disconnected"} width={282} height={45} /> <Paragraph variant="small" className={isConnected ? "text-success" : "text-error"}> - {isConnected === undefined - ? "Checking connection..." - : isConnected - ? "Your local dev server is connected to Trigger.dev" - : "Your local dev server is not connected to Trigger.dev"} + {getStatusText()} </Paragraph> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/webapp/app/components/DevPresence.tsx(2 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx(3 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
apps/webapp/app/components/DevPresence.tsx (2)
apps/webapp/app/assets/icons/ConnectionIcons.tsx (3)
CheckingConnectionIcon(50-73)ConnectedIcon(1-28)DisconnectedIcon(30-48)apps/webapp/app/utils/pathBuilder.ts (1)
docsPath(417-419)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
🔇 Additional comments (7)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx (4)
24-24: Import update looks good.The import has been expanded to include
AnimatePresencealong withmotionfrom framer-motion, which is needed for the animation effects in the new components.
30-34: Good addition of required components.These imports bring in the necessary components and hooks for handling the development server connection status. The organization of imports follows the project's conventions.
191-194: Well-structured connection detection logic.The implementation correctly uses the
useCrossEngineIsConnectedhook to determine if the development server is connected, providing relevant context from the current run (log count and completion status).
206-206: Good conditional rendering of the banner.The banner is appropriately shown only in development environments, ensuring production users don't see irrelevant connection warnings. The implementation correctly passes the connection status to the banner component.
apps/webapp/app/components/DevPresence.tsx (3)
1-21: Good organization of imports.The imports are well-structured and properly grouped. The code imports necessary components from framer-motion, React core, and internal application components that will be needed for the connection status UI.
139-148: Clean implementation of ConnectionIcon component.The component properly handles all three possible states (undefined, true, false) and renders the appropriate icon for each case. The use of conditional rendering is clean and effective.
194-222: Well-implemented banner with proper animation.The
DevDisconnectedBannercomponent is well-structured with appropriate use ofAnimatePresencefor smooth transitions. It correctly appears only whenisConnectedis false, and the dialog is properly implemented with a clear trigger mechanism.
Adds a "Your local dev server is not connected to Trigger.dev" banner message to the top of the Run and Runs list pages
Also moves the functionality to the DevPresence.tsx file and makes the DevConnection component reusable.
Summary by CodeRabbit
New Features
Refactor