feat(ui): add tab layout for DAG definitions page#1503
Conversation
📝 WalkthroughWalkthroughAdds persistent multi-tab DAG management with a TabContext and Radix-based TabBar, measures left panel pixel width via PanelWidthContext/ResizeObserver, enables card view in the DAG list based on panel width, refactors modal animation/render lifecycle, adds DAG-not-found handling, preserves remoteNode in navigation, and updates styles/deps. Changes
Sequence DiagramsequenceDiagram
participant User
participant DAGTable
participant TabContext
participant LocalStorage
participant DAGDetailsPanel
User->>DAGTable: Click DAG row (fileName, title)
DAGTable->>TabContext: selectDAG(fileName, title)
activate TabContext
alt tab exists
TabContext->>TabContext: setActiveTab(existingId)
else new tab
TabContext->>TabContext: addTab(fileName, title)
TabContext->>TabContext: setActiveTab(newId)
end
TabContext->>LocalStorage: persist tabs & activeTabId
deactivate TabContext
TabContext-->>DAGDetailsPanel: activeTabId / fileName updated
DAGDetailsPanel->>DAGDetailsPanel: fetch DAG data
alt fetch success
DAGDetailsPanel-->>User: render details (visible)
else fetch error / 404
DAGDetailsPanel-->>User: render "DAG not found" UI (Close Tab)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-12-04T10:34:01.045ZApplied to files:
📚 Learning: 2025-12-04T10:34:01.045ZApplied to files:
⏰ 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). (1)
🔇 Additional comments (2)
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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
ui/src/styles/global.css (1)
412-412: Consider if slower animation durations align with UX goals.The
.slide-in-from-rightduration increased from 0.2s to 0.3s, and.fade-infrom 0.15s to 0.5s. While this may suit the Mac OS-inspired aesthetic, 0.5s can feel sluggish for frequent UI transitions. Consider whether these durations provide the desired user experience.Also applies to: 425-425
ui/src/components/TabBar.tsx (1)
34-43: Consider adding aria-label for enhanced screen reader support.While the
titleattribute provides basic accessibility, adding anaria-labelto the close button would improve screen reader announcements, especially for identifying which tab is being closed.🔎 Suggested enhancement
<button onClick={(e) => { e.stopPropagation(); closeTab(tab.id); }} className="tab-close" title="Close" + aria-label={`Close ${tab.title}`} > <X className="h-3 w-3" /> </button>ui/src/features/dags/components/dag-details/DAGDetailsPanel.tsx (1)
169-176: Consider showing stale data instead of full loading state.Per coding guidelines, full-page loading indicators that hide content should be avoided. This block shows a centered
LoadingIndicatorwhile waiting for initial data. Consider showing a skeleton or placeholder UI instead, especially sincelastValidDatapattern is already in place for subsequent loads.However, since this is only for initial load (
!displayData) and not during refreshes, it may be acceptable for this case.ui/src/pages/dags/index.tsx (1)
336-351: Consider edge case when all DAGs are already open in tabs.
handleAddTabfinds the first DAG not already open. If all DAGs are open, no action is taken (which is reasonable), but consider adding user feedback or disabling the add button in this case.Also, this function depends on
dagFileswhich updates asynchronously - if the list is still loading, the button may not work as expected.🔎 Suggested improvement
const handleAddTab = () => { // Find a DAG to open in the new tab (first one not already open) const openFileNames = new Set(tabs.map(t => t.fileName)); const availableDAG = dagFiles.find(d => !openFileNames.has(d.fileName)); if (availableDAG) { addTab(availableDAG.fileName, availableDAG.dag.name); + } else if (dagFiles.length > 0) { + // All DAGs already open - could show a toast or disable button } };ui/src/features/dags/components/dag-list/DAGTable.tsx (1)
1179-1180: Consider formatting the colgroup for readability.The column widths are all on a single line making them hard to read and maintain.
🔎 Suggested formatting
-{/* Column widths: Expand 5%, Name 37%, Status 10%, LastRun 18%, Schedule 20%, Actions 10% */} - <colgroup><col style={{ width: '5%' }} /><col style={{ width: '37%' }} /><col style={{ width: '10%' }} /><col style={{ width: '18%' }} /><col style={{ width: '20%' }} /><col style={{ width: '10%' }} /></colgroup> + {/* Column widths: Expand 5%, Name 37%, Status 10%, LastRun 18%, Schedule 20%, Actions 10% */} + <colgroup> + <col style={{ width: '5%' }} /> + <col style={{ width: '37%' }} /> + <col style={{ width: '10%' }} /> + <col style={{ width: '18%' }} /> + <col style={{ width: '20%' }} /> + <col style={{ width: '10%' }} /> + </colgroup>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ui/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
ui/index.htmlui/package.jsonui/src/components/SplitLayout.tsxui/src/components/TabBar.cssui/src/components/TabBar.tsxui/src/components/ui/table.tsxui/src/contexts/TabContext.tsxui/src/features/dag-runs/components/dag-run-details/DAGRunDetailsModal.tsxui/src/features/dags/components/dag-details/DAGDetailsPanel.tsxui/src/features/dags/components/dag-list/DAGTable.tsxui/src/pages/dags/dag/index.tsxui/src/pages/dags/index.tsxui/src/styles/global.css
🧰 Additional context used
📓 Path-based instructions (2)
ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (ui/CLAUDE.md)
ui/**/*.{ts,tsx,jsx,js}: Use developer-centric UI design with high information density, minimal whitespace, compact components, and no unnecessary decorations
Support both light and dark modes for all UI components using Tailwind CSS class pairs likedark:bg-slate-700
NEVER use full-page loading overlays or LoadingIndicator components that hide content - show stale data while fetching updates instead
Use compact modal design with small headers, minimal padding (p-2 or p-3), tight spacing, and support keyboard navigation (arrows, enter, escape)
Use small heights for form elements: select boxes h-7 or smaller, buttons h-7 or h-8, inputs with compact padding (py-0.5 or py-1)
Minimize row heights in tables and lists while maintaining readability, merge related columns, and always handle long text withwhitespace-normal break-words
Use consistent metadata styling withbg-slate-200 dark:bg-slate-700backgrounds and maintain text hierarchy with primary/secondary/muted text colors
Use flexbox-first layouts withmin-h-0andoverflow-hiddento prevent layout breaks, account for fixed elements when setting heights
Maintain keyboard navigation support in all interactive components with appropriate focus indicators and ARIA labels
Files:
ui/src/pages/dags/dag/index.tsxui/src/components/ui/table.tsxui/src/components/TabBar.tsxui/src/components/SplitLayout.tsxui/src/pages/dags/index.tsxui/src/features/dag-runs/components/dag-run-details/DAGRunDetailsModal.tsxui/src/features/dags/components/dag-details/DAGDetailsPanel.tsxui/src/contexts/TabContext.tsxui/src/features/dags/components/dag-list/DAGTable.tsx
ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
ui/**/*.{ts,tsx}: The React + TypeScript frontend resides inui/, with production bundles copied tointernal/service/frontend/assetsbymake ui
UI code follows ESLint + Prettier (2-space indent) and Tailwind utilities; name React components in PascalCase (JobList.tsx) and hooks withuse*(useJobs.ts)
Files:
ui/src/pages/dags/dag/index.tsxui/src/components/ui/table.tsxui/src/components/TabBar.tsxui/src/components/SplitLayout.tsxui/src/pages/dags/index.tsxui/src/features/dag-runs/components/dag-run-details/DAGRunDetailsModal.tsxui/src/features/dags/components/dag-details/DAGDetailsPanel.tsxui/src/contexts/TabContext.tsxui/src/features/dags/components/dag-list/DAGTable.tsx
🧠 Learnings (7)
📚 Learning: 2025-12-04T10:34:01.045Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: ui/CLAUDE.md:0-0
Timestamp: 2025-12-04T10:34:01.045Z
Learning: Applies to ui/**/*.{ts,tsx,jsx,js} : Use consistent metadata styling with `bg-slate-200 dark:bg-slate-700` backgrounds and maintain text hierarchy with primary/secondary/muted text colors
Applied to files:
ui/src/components/TabBar.cssui/src/components/ui/table.tsxui/src/pages/dags/index.tsxui/src/styles/global.cssui/src/features/dags/components/dag-list/DAGTable.tsx
📚 Learning: 2025-12-04T10:34:01.045Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: ui/CLAUDE.md:0-0
Timestamp: 2025-12-04T10:34:01.045Z
Learning: Applies to ui/**/*.{ts,tsx,jsx,js} : Use developer-centric UI design with high information density, minimal whitespace, compact components, and no unnecessary decorations
Applied to files:
ui/src/components/TabBar.cssui/src/components/ui/table.tsxui/src/components/SplitLayout.tsxui/src/pages/dags/index.tsxui/src/features/dags/components/dag-details/DAGDetailsPanel.tsx
📚 Learning: 2025-12-04T10:34:17.062Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T10:34:17.062Z
Learning: Applies to ui/**/*.{ts,tsx} : UI code follows ESLint + Prettier (2-space indent) and Tailwind utilities; name React components in PascalCase (`JobList.tsx`) and hooks with `use*` (`useJobs.ts`)
Applied to files:
ui/package.jsonui/src/pages/dags/index.tsx
📚 Learning: 2025-12-04T10:34:01.045Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: ui/CLAUDE.md:0-0
Timestamp: 2025-12-04T10:34:01.045Z
Learning: Applies to ui/**/*.{ts,tsx,jsx,js} : Minimize row heights in tables and lists while maintaining readability, merge related columns, and always handle long text with `whitespace-normal break-words`
Applied to files:
ui/package.jsonui/src/components/ui/table.tsxui/src/components/SplitLayout.tsxui/src/pages/dags/index.tsxui/src/features/dags/components/dag-details/DAGDetailsPanel.tsxui/src/features/dags/components/dag-list/DAGTable.tsx
📚 Learning: 2025-12-04T10:34:01.045Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: ui/CLAUDE.md:0-0
Timestamp: 2025-12-04T10:34:01.045Z
Learning: Applies to ui/**/*.{ts,tsx,jsx,js} : Use flexbox-first layouts with `min-h-0` and `overflow-hidden` to prevent layout breaks, account for fixed elements when setting heights
Applied to files:
ui/src/components/SplitLayout.tsxui/src/styles/global.cssui/src/features/dags/components/dag-details/DAGDetailsPanel.tsx
📚 Learning: 2025-12-04T10:34:01.045Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: ui/CLAUDE.md:0-0
Timestamp: 2025-12-04T10:34:01.045Z
Learning: Applies to ui/**/*.{ts,tsx,jsx,js} : Use compact modal design with small headers, minimal padding (p-2 or p-3), tight spacing, and support keyboard navigation (arrows, enter, escape)
Applied to files:
ui/src/features/dag-runs/components/dag-run-details/DAGRunDetailsModal.tsxui/src/features/dags/components/dag-details/DAGDetailsPanel.tsx
📚 Learning: 2025-12-04T10:34:01.045Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: ui/CLAUDE.md:0-0
Timestamp: 2025-12-04T10:34:01.045Z
Learning: Applies to ui/**/*.{ts,tsx,jsx,js} : Use small heights for form elements: select boxes h-7 or smaller, buttons h-7 or h-8, inputs with compact padding (py-0.5 or py-1)
Applied to files:
ui/src/features/dags/components/dag-details/DAGDetailsPanel.tsx
🧬 Code graph analysis (4)
ui/src/components/TabBar.tsx (1)
ui/src/contexts/TabContext.tsx (1)
useTabContext(24-30)
ui/src/pages/dags/index.tsx (1)
ui/src/contexts/TabContext.tsx (2)
useTabContext(24-30)TabProvider(46-164)
ui/src/features/dags/components/dag-details/DAGDetailsPanel.tsx (1)
ui/src/hooks/api.ts (1)
useQuery(28-28)
ui/src/features/dags/components/dag-list/DAGTable.tsx (2)
ui/src/api/v2/schema.ts (1)
components(889-1557)ui/src/components/SplitLayout.tsx (1)
PanelWidthContext(4-4)
⏰ 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). (1)
- GitHub Check: Build
🔇 Additional comments (26)
ui/index.html (2)
2-2: LGTM: Disabling auto-translation is appropriate.Adding
translate="no"prevents browsers and translation services from translating technical content like DAG names, configuration values, and code snippets.
38-38: Verify that the authentication mode change is intentional.The
authModehas been changed from'none'to'builtin', which enables authentication in this test/development configuration. This change appears unrelated to the PR's stated objective of adding tab layout for the DAG definitions page.Please confirm:
- Is this change intentional?
- Should this authentication mode change be part of this UI layout PR?
- Will this affect existing development/testing workflows?
ui/src/pages/dags/dag/index.tsx (1)
34-34: LGTM! Clean URL state management implementation.The remoteNode extraction from URL params with fallback logic is well-structured, and the
buildUrlhelper properly preserves the remoteNode across navigations with correct URI encoding. Dependency arrays are complete and correct.Also applies to: 57-66, 69-85
ui/src/components/ui/table.tsx (1)
12-15: LGTM! Visual refinement aligns with design guidelines.The border addition and variant change to "ghost" enhance table definition while maintaining the compact, minimal aesthetic specified in the coding guidelines.
ui/src/features/dag-runs/components/dag-run-details/DAGRunDetailsModal.tsx (1)
29-60: LGTM! Proper animation lifecycle implementation.The two-phase rendering approach (shouldRender/isVisible) with double-RAF for mount and 150ms timeout for unmount correctly implements smooth CSS transitions. The timeout duration matches the CSS transition duration (line 174).
ui/src/contexts/TabContext.tsx (3)
42-44: Tab ID generation is acceptable for this use case.The combination of
Date.now()andMath.random()provides sufficient uniqueness for UI tab IDs. Collision risk is negligible in practice.
47-80: LGTM! Robust localStorage persistence with proper error handling.The try/catch blocks appropriately handle JSON parsing errors, and validation ensures the activeTabId exists in the tabs array. Silent failures are acceptable for localStorage access (e.g., private browsing modes).
121-140: Smart DAG selection logic that minimizes tab proliferation.The priority order (existing tab → reuse active tab → create new) prevents duplicate tabs while maintaining intuitive behavior for single-click navigation.
ui/src/styles/global.css (1)
8-13: LGTM! Full-height layout setup aligns with flexbox guidelines.The
overflow: hiddenandheight: 100%on html/body establish a proper full-height container system, consistent with the "flexbox-first layouts" coding guideline.ui/src/components/TabBar.css (1)
1-168: LGTM! Well-structured Mac OS-inspired tab styling.The CSS effectively creates a 3D beveled tab appearance using semantic color tokens (supporting dark mode), maintains compact sizing (12px font, minimal padding) per guidelines, and includes appropriate hover/active states.
ui/src/components/TabBar.tsx (1)
11-58: LGTM! Clean TabBar implementation with proper event handling.The component correctly uses Radix UI Tabs primitives for built-in keyboard navigation and accessibility. The
stopPropagationon close buttons (line 36) properly prevents tab switching when closing. Early return for empty tabs prevents unnecessary rendering.ui/package.json (1)
81-81: No action needed. @radix-ui/react-tabs v1.1.13 is the latest stable version as of December 2025. The dependency is current and appropriately specified with the caret (^) range.ui/src/features/dags/components/dag-details/DAGDetailsPanel.tsx (3)
39-71: LGTM - Well-structured error state management.The
notFoundstate pattern is clean: it correctly sets on error, clears on valid data, and resets whenfileNamechanges. Disabling polling whennotFoundistrueprevents unnecessary network requests. The two separate effects clearly separate concerns.
157-167: LGTM - Clean error state UI.The not-found state provides clear feedback with a simple message and actionable close button. The button appropriately uses
size="sm"for compact design per coding guidelines.
194-195: LGTM - Layout improvements.The updated padding (
pl-2 pt-2) and margin (mb-2) provide appropriate spacing for the panel header area. The flexbox layout withflex-shrink-0on the header prevents layout issues.ui/src/components/SplitLayout.tsx (3)
3-4: LGTM - Clean context for panel width sharing.The
PanelWidthContextexport pattern is appropriate for allowing descendants to access the measured panel width without prop drilling.
48-66: LGTM - Proper ResizeObserver implementation with one minor consideration.The ResizeObserver pattern is correctly implemented with:
- Null check before setup
- Initial measurement call
- Proper cleanup via
disconnect()One consideration: If
leftPanelRef.currentisnullinitially and becomes available later (unlikely in this component's lifecycle), the effect won't re-run due to the empty dependency array. However, since the ref is attached to a div that's always rendered, this shouldn't be an issue in practice.
127-135: LGTM - Responsive layout with CSS variables.The responsive width approach using CSS variables (
--left-width) combined with Tailwind'smd:breakpoint is elegant. Wrapping children withPanelWidthContext.Providercorrectly exposes the measured pixel width to descendants likeDAGTable.ui/src/pages/dags/index.tsx (3)
40-51: LGTM - Clean tab context integration.The component correctly integrates with the new
TabContext, extractingselectDAGandgetActiveFileNamefor tab-aware DAG management. The rename toDAGsContentappropriately reflects that it's now the inner content component.
353-365: LGTM - Right panel with tab bar integration.The right panel correctly renders the
TabBarwhen tabs exist, with proper flex layout for the content area. Theoverflow-hiddenon the inner div follows flexbox-first layout guidelines.
377-384: LGTM - Clean provider wrapper pattern.The wrapper component correctly provides
TabProvidercontext toDAGsContent, maintaining clean separation between context setup and component logic.ui/src/features/dags/components/dag-list/DAGTable.tsx (5)
78-80: LGTM - Reasonable threshold for card view.The 700px threshold is appropriate for switching between table and card views when the panel becomes too narrow for comfortable table column display.
82-198: LGTM - Well-designed reusable DAGCard component.The
DAGCardcomponent is well-structured with:
- Clean props interface
- Proper event handling (Cmd/Ctrl+click for new tab)
- Appropriate visual hierarchy with compact spacing
- Good use of
stopPropagationfor interactive elements- Accessibility consideration with cursor styling and hover states
The component follows coding guidelines for compact design with small text sizes (
text-[9px],text-[10px]) and minimal padding.
1075-1078: LGTM - Smart responsive card view toggle.Using
PanelWidthContextto determine when to switch to card view is an elegant solution. The null check ensures the table view is used as default when the context isn't available.
1009-1033: LGTM - Keyboard navigation properly updated.The keyboard navigation correctly extracts
titlealongsidefileNameand passes both toonSelectDAG. This maintains consistency with the updated function signature.
1302-1400: LGTM - Card view implementation using shared DAGCard component.The card view correctly uses the new
DAGCardcomponent for both grouped and standalone DAGs. The conditional rendering withuseCardViewproperly toggles between table and card layouts based on available width.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ui/src/styles/global.css
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: ui/CLAUDE.md:0-0
Timestamp: 2025-12-04T10:34:01.045Z
Learning: Applies to ui/**/*.{ts,tsx,jsx,js} : Minimize row heights in tables and lists while maintaining readability, merge related columns, and always handle long text with `whitespace-normal break-words`
📚 Learning: 2025-12-04T10:34:01.045Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: ui/CLAUDE.md:0-0
Timestamp: 2025-12-04T10:34:01.045Z
Learning: Applies to ui/**/*.{ts,tsx,jsx,js} : Use consistent metadata styling with `bg-slate-200 dark:bg-slate-700` backgrounds and maintain text hierarchy with primary/secondary/muted text colors
Applied to files:
ui/src/styles/global.css
📚 Learning: 2025-12-04T10:34:01.045Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: ui/CLAUDE.md:0-0
Timestamp: 2025-12-04T10:34:01.045Z
Learning: Applies to ui/**/*.{ts,tsx,jsx,js} : Use flexbox-first layouts with `min-h-0` and `overflow-hidden` to prevent layout breaks, account for fixed elements when setting heights
Applied to files:
ui/src/styles/global.css
🪛 Biome (2.1.2)
ui/src/styles/global.css
[error] 462-462: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
background is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
⏰ 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). (1)
- GitHub Check: Build
🔇 Additional comments (3)
ui/src/styles/global.css (3)
8-13: LGTM! Layout constraints support the tab-based UI.Adding
overflow: hiddenandheight: 100%to bothhtmlandbodyis the correct approach for a full-height application layout with internally scrollable regions. This change aligns with the tab layout feature and prevents unwanted document-level scrolling.Based on learnings, this follows the flexbox-first layout pattern with proper overflow constraints.
85-338: LGTM! Formatting improvements enhance readability.The line-wrapping adjustments for color tokens and rgba() declarations improve code readability without changing any functional behavior.
448-448: Insufficient evidence to verify the animation timing change.The current code shows
.fade-in { animation: fadeIn 0.5s ease-in-out; }, but the claimed change from 0.15s to 0.5s cannot be confirmed from available git history. Additionally, the custom.fade-inclass appears to have minimal direct usage in the codebase—most animations are implemented using Tailwind'sfade-in-0/fade-out-0classes withduration-100(100ms), not the custom.fade-inclass. Without verifying which animations actually use this timing and whether the change was intentional, the impact on perceived responsiveness cannot be assessed.
Summary by CodeRabbit
New Features
Bug Fixes
UI/UX Improvements
✏️ Tip: You can customize this high-level summary in your review settings.