fix(ui): refactor small UI issues#1504
Conversation
📝 WalkthroughWalkthroughIntroduces unsaved changes detection and management for DAG editing via a new React context. Integrates warnings in DAG actions and tracks changes in the editor. Additionally refactors log pagination and line numbering in execution logs to handle trailing newlines and ensure consistent line counts across view modes. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant DAGEditor as DAG Editor
participant DAGSpec as DAGSpec Component
participant Context as UnsavedChanges Context
participant DAGActions as DAG Actions
participant Modal as Confirmation Modal
User->>DAGEditor: Edit DAG configuration
DAGEditor->>DAGSpec: Update value
DAGSpec->>DAGSpec: Compare with fetched data
DAGSpec->>Context: setHasUnsavedChanges(true)
DAGSpec->>DAGEditor: Disable save (changes exist)
User->>DAGActions: Click Enqueue
DAGActions->>Context: Check hasUnsavedChanges
alt Unsaved changes exist
Context-->>DAGActions: true
DAGActions->>Modal: Open "Unsaved Changes" modal
Modal-->>User: Warn about unsaved edits
User->>Modal: Confirm proceed
Modal->>DAGActions: Continue to Enqueue
else No unsaved changes
Context-->>DAGActions: false
DAGActions->>DAGActions: Open Enqueue modal
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (8)
🧰 Additional context used📓 Path-based instructions (2)ui/**/*.{ts,tsx,jsx,js}📄 CodeRabbit inference engine (ui/CLAUDE.md)
Files:
ui/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (3)📚 Learning: 2025-12-04T10:34:01.045ZApplied to files:
📚 Learning: 2025-12-04T10:34:01.045ZApplied to files:
📚 Learning: 2025-12-04T10:34:01.045ZApplied to files:
🧬 Code graph analysis (3)ui/src/features/dags/components/dag-editor/DAGSpec.tsx (1)
ui/src/features/dags/components/common/DAGActions.tsx (1)
ui/src/features/dags/components/dag-details/DAGDetailsPanel.tsx (3)
⏰ 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 (13)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/src/features/dags/components/dag-execution/ExecutionLog.tsx (1)
378-400: Inconsistency: Page navigation condition usestotalLinesinstead ofeffectiveTotalLines.This differs from
StepLog.tsx(line 386) which useseffectiveTotalLinesfor the same condition. For consistency, consider updating this line:🔎 Suggested fix
{/* Page navigation controls */} - {viewMode === 'page' && totalLines > 0 && ( + {viewMode === 'page' && effectiveTotalLines > 0 && ( <div className="flex items-center gap-2 mt-2">
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
ui/src/contexts/UnsavedChangesContext.tsxui/src/features/dags/components/common/DAGActions.tsxui/src/features/dags/components/dag-details/DAGDetailsPanel.tsxui/src/features/dags/components/dag-editor/DAGSpec.tsxui/src/features/dags/components/dag-execution/ExecutionLog.tsxui/src/features/dags/components/dag-execution/StepLog.tsxui/src/pages/dags/dag/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/features/dags/components/dag-editor/DAGSpec.tsxui/src/contexts/UnsavedChangesContext.tsxui/src/features/dags/components/dag-execution/ExecutionLog.tsxui/src/features/dags/components/dag-execution/StepLog.tsxui/src/features/dags/components/common/DAGActions.tsxui/src/pages/dags/dag/index.tsxui/src/features/dags/components/dag-details/DAGDetailsPanel.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/features/dags/components/dag-editor/DAGSpec.tsxui/src/contexts/UnsavedChangesContext.tsxui/src/features/dags/components/dag-execution/ExecutionLog.tsxui/src/features/dags/components/dag-execution/StepLog.tsxui/src/features/dags/components/common/DAGActions.tsxui/src/pages/dags/dag/index.tsxui/src/features/dags/components/dag-details/DAGDetailsPanel.tsx
🧠 Learnings (3)
📚 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/src/features/dags/components/dag-execution/ExecutionLog.tsxui/src/features/dags/components/dag-execution/StepLog.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 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 developer-centric UI design with high information density, minimal whitespace, compact components, and no unnecessary decorations
Applied to files:
ui/src/features/dags/components/common/DAGActions.tsx
🧬 Code graph analysis (3)
ui/src/features/dags/components/dag-editor/DAGSpec.tsx (1)
ui/src/contexts/UnsavedChangesContext.tsx (1)
useUnsavedChanges(10-17)
ui/src/features/dags/components/common/DAGActions.tsx (1)
ui/src/contexts/UnsavedChangesContext.tsx (1)
useUnsavedChanges(10-17)
ui/src/features/dags/components/dag-details/DAGDetailsPanel.tsx (3)
ui/src/contexts/UnsavedChangesContext.tsx (1)
UnsavedChangesProvider(19-31)ui/src/features/dags/contexts/DAGContext.ts (1)
DAGContext(3-9)ui/src/features/dags/contexts/RootDAGRunContext.ts (1)
RootDAGRunContext(9-16)
⏰ 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 (13)
ui/src/styles/global.css (1)
459-470: LGTM! Good UX improvement for log text selection.The selection styling ensures visible text selection on dark backgrounds. The Mozilla-prefixed selector provides broader browser compatibility.
ui/src/features/dags/components/dag-editor/DAGSpec.tsx (2)
128-143: LGTM! Clean implementation of unsaved changes tracking.The change detection logic correctly compares editor state against fetched data, and the cleanup effect properly resets state on unmount to prevent stale context values.
394-402: LGTM! Save button correctly reflects unsaved state.Disabling the save button when there are no changes provides clear visual feedback to users.
ui/src/features/dags/components/dag-details/DAGDetailsPanel.tsx (1)
180-242: LGTM! Proper provider integration.The
UnsavedChangesProvidercorrectly wraps the panel content, enabling descendant components (likeDAGSpec) to track and report unsaved changes.ui/src/features/dags/components/common/DAGActions.tsx (2)
111-117: LGTM! Good UX for unsaved changes warning.The conditional flow correctly intercepts the enqueue action when unsaved changes exist, preventing users from accidentally running outdated DAG configurations.
547-571: LGTM! Clear and informative warning modal.The modal content clearly explains that the DAG will run with the last saved version, and the "Run Anyway" button text sets appropriate expectations.
ui/src/pages/dags/dag/index.tsx (1)
195-242: LGTM! Consistent provider integration with DAGDetailsPanel.The
UnsavedChangesProviderplacement mirrors the pattern inDAGDetailsPanel.tsx, ensuring consistent unsaved changes tracking across both the full page and modal panel views.ui/src/contexts/UnsavedChangesContext.tsx (2)
10-17: LGTM! Safe fallback pattern for context usage.Returning a default object when used outside the provider prevents runtime errors and allows components to be used in different contexts without crashing. The no-op setter is appropriate for the fallback case.
19-31: LGTM! Clean provider implementation.The
useCallbackwrapper ensures a stable setter reference, preventing unnecessary re-renders in consuming components.ui/src/features/dags/components/dag-execution/StepLog.tsx (2)
286-293: LGTM! Smart handling of trailing newline inconsistency.The logic correctly handles the common edge case where
split('\n')produces a trailing empty string, and theeffectiveTotalLinescalculation accommodates potential API inconsistencies in counting trailing newlines.
463-476: LGTM! Improved log content styling and selection.The
log-contentclass enables the custom text selection styling fromglobal.css, andselect-text cursor-texton line content improves copy-paste UX.ui/src/features/dags/components/dag-execution/ExecutionLog.tsx (2)
277-284: LGTM! Consistent trailing newline handling with StepLog.The logic correctly mirrors
StepLog.tsx, ensuring consistent line counting behavior across both log components.
455-468: LGTM! Improved log content styling and selection.The
log-contentclass andselect-text cursor-textstyling match theStepLog.tsximplementation for a consistent user experience.
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.