Repository navigation
feat(ui): share kit - graph export and copy links - #2534
Conversation
The mount-time formatDocument rewrote files the user never touched, lighting up Save/Discard and arming the external-change conflict dialog on first open. formatOnType reformatted the whole document on every Enter. Manual format (Shift+Alt+F) still works.
Backend validation errors (line:column prefixes and caret excerpts) were passed as the modal's hint, which collapses newlines into one muted proportional-font paragraph. Add a details channel rendered monospace with line breaks preserved, and stop treating an empty errors array as a failure on spec save.
The retry action re-executes only the selected step, but the tooltip and dialog title said "Retry from this step". Rename to match the API contract, and give the success path the same toast plus refresh the run-level retry already has instead of silently closing the dialog.
The header button said Enqueue while its tooltip, the modal it opens, and the CLI all say start. The modal still switches to Enqueue wording when the queue toggle is on.
The executions list rendered "No DAG runs found" while the first page was still loading, then swapped in the table. Thread the pagination hook's initial-loading flag into both list views and show a quiet inline loading row instead.
The workflow list told users with zero workflows to adjust filters they never set. Branch the copy: a pristine all-workflows view now invites creating the first workflow. The runs empty state is reworded to be range-aware instead of presuming filters, and the duplicated workflow empty-state markup is extracted into one component.
The schema doc sidebar has an Examples panel, and monaco-yaml surfaces schema examples in hover, value completion, and property snippets, but the schema carried none. Seed concise examples for the most-used DAG and step properties.
Every tab, bookmark, and history entry read the static "Dagu". Pages already publish their titles through AppBarContext; mirror that into document.title.
Sharing a workflow required select-all inside Monaco, and sharing a run meant grabbing the address bar. Add a Copy button to the spec editor header (visible to read-only viewers too) and a Copy link button on the run header. The three hand-rolled copy-feedback blocks consolidate into a useCopyFeedback hook.
…used The v1 executor key was rejected with a bare invalid-keys error while 70+ other legacy keys get migration hints, and renamed keys (precondition, dir) were mislabeled as snake_case fixes. Split the hint map into casing renames and removed keys with full replacement clauses, and drop the dead run->call entry (run is a valid v2 step key).
The same blob-and-anchor download sequence was copied across the step log, execution log, and artifacts tab. Consolidate into lib/download with Content-Disposition filename handling and bearer auth.
The rendered graph SVG existed in the DOM with no way out; sharing it meant an OS screenshot. The graph control bar gains Export as PNG and Export as SVG actions: the serialized clone strips the on-screen zoom transform, pins dimensions from the viewBox, and bakes the card background so dark-theme exports stay readable. Mermaid embeds its styles and the status strokes are inline, so the file is self-contained. The DAG header also gains a copy-link button beside copy-name.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds shared download helpers, centralizes artifact and log downloads, adds DAG page-link copying, and adds named PNG/SVG export controls for Mermaid-rendered graphs. ChangesDAG UI enhancements
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Graph
participant exportGraphSvg
participant exportGraphPng
participant downloadBlob
Graph->>exportGraphSvg: Export Mermaid SVG
exportGraphSvg->>downloadBlob: Download serialized SVG
Graph->>exportGraphPng: Export Mermaid PNG
exportGraphPng->>downloadBlob: Download rasterized PNG
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
Chromium marks a canvas tainted when the drawn SVG contains foreignObject, so PNG export threw SecurityError at toBlob (caught in live browser verification). Replace mermaid's HTML labels with positioned SVG text in the export clone, styled from the rendered labels; this also makes the exported SVG render in non-browser tools.
Constructing Response from a Blob requires Blob.stream, which the CI jsdom/Node combination does not provide; the suite failed there with 'object.stream is not a function'. Plain response stubs cover the same behavior.
# Conflicts: # ui/src/__tests__/App.test.tsx # ui/src/features/dags/components/dag-details/DAGHeader.tsx # ui/src/features/dags/components/visualization/Graph.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/src/features/dags/components/visualization/exportGraph.ts`:
- Around line 41-51: The label conversion must preserve the source foreignObject
position. In the conversion logic around the created SVG text element, read the
source x and y attributes and offset the centered text coordinates by those
values while retaining width/height centering; add a regression test covering a
foreignObject label positioned away from the origin.
In `@ui/src/lib/download.ts`:
- Around line 12-13: Defer URL cleanup in the download helper after link.click()
using a scheduled delay rather than revoking the object URL in the same task.
Update ui/src/lib/download.ts lines 12-13 accordingly, and modify
ui/src/lib/__tests__/download.test.ts lines 34-38 to use fake timers and verify
revocation occurs only after the scheduled delay.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c61cee9-10e2-43f7-aab6-e64731553ab1
📒 Files selected for processing (13)
ui/src/features/dags/components/DAGStatus.tsxui/src/features/dags/components/artifacts/ArtifactsTab.tsxui/src/features/dags/components/dag-details/DAGHeader.tsxui/src/features/dags/components/dag-editor/DAGSpec.tsxui/src/features/dags/components/dag-editor/DAGSpecReadOnly.tsxui/src/features/dags/components/dag-execution/ExecutionLog.tsxui/src/features/dags/components/dag-execution/StepLog.tsxui/src/features/dags/components/visualization/DAGGraph.tsxui/src/features/dags/components/visualization/Graph.tsxui/src/features/dags/components/visualization/__tests__/Graph.test.tsxui/src/features/dags/components/visualization/exportGraph.tsui/src/lib/__tests__/download.test.tsui/src/lib/download.ts
| const width = Number(label.getAttribute('width') ?? 0); | ||
| const height = Number(label.getAttribute('height') ?? 0); | ||
|
|
||
| const text = document.createElementNS( | ||
| 'http://www.w3.org/2000/svg', | ||
| 'text' | ||
| ); | ||
| text.setAttribute('x', String(width / 2)); | ||
| text.setAttribute('y', String(height / 2)); | ||
| text.setAttribute('text-anchor', 'middle'); | ||
| text.setAttribute('dominant-baseline', 'central'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'exportGraph\.ts$' . || true
echo "== file outline/contents =="
if [ -f ui/src/features/dags/components/visualization/exportGraph.ts ]; then
wc -l ui/src/features/dags/components/visualization/exportGraph.ts
cat -n ui/src/features/dags/components/visualization/exportGraph.ts
fi
echo "== related tests =="
git ls-files | rg 'exportGraph|visualization|mermaid|dags/components/visualization' || true
echo "== searches for replaceForeignObjectLabels/usages =="
rg -n "replaceForeignObjectLabels|foreignObject|exportGraph" .Repository: dagucloud/dagu
Length of output: 8947
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Graph tests around export behavior =="
sed -n '140,230p' ui/src/features/dags/components/visualization/__tests__/Graph.test.tsx | cat -n
echo "== mermaid fixtures/usages mentioning foreignObject? =="
rg -n "foreignObject|x=\"|y=|rendered\"|serializeGraphSvg|exportGraphSvg" ui/src/components/ui ui/src/features/dags/components/visualization --glob '*.ts' --glob '*.tsx' || trueRepository: dagucloud/dagu
Length of output: 6778
🌐 Web query:
MDN SVG foreignObject x y width height coordinate system semantics
💡 Result:
The SVG foreignObject element is a container that allows for the inclusion of elements from a different XML namespace (most commonly (X)HTML) within an SVG document [1]. It acts as a bridge, enabling the rendering of non-SVG content inside an SVG graphic [2][3]. Coordinate System and Positioning The foreignObject element uses the SVG coordinate system to define its placement and size [2][4]. Its positioning and dimensions are governed by four primary attributes: - x and y: These attributes define the coordinate of the top-left corner of the rectangular region where the foreign content will be rendered [1][3]. They default to 0 [1]. - width and height: These attributes define the dimensions of this rectangular region [1][3]. They default to auto [1]. In SVG2, these four attributes are categorized as Geometry Properties, meaning they can be set either as XML attributes or as CSS properties [1]. Semantics and Layout The foreignObject element serves a specific dual purpose regarding layout: 1. Positioning Rectangle: The x, y, width, and height attributes define a "positioning rectangle" in the SVG coordinate space [2][5]. This rectangle functions as the bounding box for the element [5]. 2. CSS Containing Block: For the child content (such as HTML), the foreignObject establishes a new CSS containing block [2][6]. This effectively resets the scope of layout for that child content [4][6]. The child content is laid out within this block using standard CSS rules [2]. Any absolutely positioned elements inside the foreignObject are positioned relative to this containing block [2][5]. Interaction with SVG - Transformations: The foreign content is subject to the same SVG transformations (e.g., rotate, scale, translate), filters, clipping, and masking as any other SVG element [2][3]. - Overflow: The CSS overflow property determines whether child content that extends outside the defined positioning rectangle is clipped or visible [2][5]. - Rendering: Because the foreignObject container is an SVG element, it is part of the SVG rendering tree. However, the browser uses different processes to render the nested foreign content [2][3]. If you wish to nest SVG elements inside the foreignObject, you must include a root svg element within it to re-establish an SVG document fragment and viewport [2][4]. Top Results: [1][2][4][6][5]
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/SVG/Reference/Element/foreignObject
- 2: https://www.w3.org/TR/SVG2/embedded.html
- 3: https://www.w3.org/TR/SVG11/extend.html
- 4: https://www.w3.org/TR/SVG/coords.html
- 5: https://svgwg.org/svg2-draft/embedded.html
- 6: https://svgwg.org/svg2-draft/coords.html
🌐 Web query:
SVG text dominant-baseline central default behavior x y text-anchor middle
💡 Result:
In SVG, the dominant-baseline and text-anchor attributes work together to define the position of the text relative to the coordinates specified by the x and y attributes [1]. Dominant-Baseline The dominant-baseline attribute controls the vertical alignment of the text by specifying which baseline should be aligned to the y coordinate [2][1]. - Default Behavior: The default value is auto [2][3]. For a horizontal writing mode, auto computes to alphabetic [2][4]. For a vertical writing mode, auto computes to central [2][4][3]. - Central: When dominant-baseline="central" is explicitly set, the baseline identifier is set to central, effectively aligning the vertical midpoint of the font's em-box (or the constructed baseline table) with the y coordinate [2][3]. Text-Anchor The text-anchor attribute controls the horizontal alignment of the text relative to the x coordinate [5][1]. - Middle: When text-anchor="middle" is used, the geometric midpoint of the rendered text string is aligned exactly with the x coordinate [5][6]. Combined Interaction When you use dominant-baseline="central" and text-anchor="middle" on an SVG text element, you are instructing the rendering engine to place the geometric center of the text exactly at the point defined by the x and y attributes [5][1]. This is a common technique for centering text elements within a specific area or relative to a coordinate point [1]. Summary table for defaults (when auto is used): Writing Mode | Dominant-Baseline (Computed) | Text-Anchor (Default) --- | --- | --- Horizontal | Alphabetic | Start Vertical | Central | Start
Citations:
- 1: https://www.d.umn.edu/~gshute/webdev/SVG/text-positioning.html
- 2: https://developer.mozilla.org/en-US/docs/Web/SVG/Reference/Attribute/dominant-baseline
- 3: https://github.com/mdn/content/blob/main/files/en-us/web/svg/reference/attribute/dominant-baseline/index.md
- 4: https://www.w3.org/Graphics/SVG/WG/wiki/How_to_determine_dominant_baseline
- 5: https://developer.mozilla.org/en-US/docs/Web/SVG/Reference/Attribute/text-anchor
- 6: https://www.w3.org/TR/SVG11/text.html
Preserve the source foreignObject coordinates when converting labels.
foreignObject uses x, y, width, and height to define its rectangle. Replacing it with centered text without adding the source x and y shifts labels that are not at the origin. Preserve the source coordinates before centering, and add a regression test with a positioned foreignObject label.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ui/src/features/dags/components/visualization/exportGraph.ts` around lines 41
- 51, The label conversion must preserve the source foreignObject position. In
the conversion logic around the created SVG text element, read the source x and
y attributes and offset the centered text coordinates by those values while
retaining width/height centering; add a regression test covering a foreignObject
label positioned away from the origin.
| link.click(); | ||
| URL.revokeObjectURL(objectUrl); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Defer object URL cleanup after browser download navigation.
The helper revokes the object URL in the same task as link.click(). The test preserves that unsafe timing.
ui/src/lib/download.ts#L12-L13: scheduleURL.revokeObjectURL(objectUrl)afterlink.click().ui/src/lib/__tests__/download.test.ts#L34-L38: use fake timers and verify cleanup after the scheduled delay.
Proposed fix
- URL.revokeObjectURL(objectUrl);
+ window.setTimeout(() => URL.revokeObjectURL(objectUrl), 0);📍 Affects 2 files
ui/src/lib/download.ts#L12-L13(this comment)ui/src/lib/__tests__/download.test.ts#L34-L38
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ui/src/lib/download.ts` around lines 12 - 13, Defer URL cleanup in the
download helper after link.click() using a scheduled delay rather than revoking
the object URL in the same task. Update ui/src/lib/download.ts lines 12-13
accordingly, and modify ui/src/lib/__tests__/download.test.ts lines 34-38 to use
fake timers and verify revocation occurs only after the scheduled delay.
Exported label text picked up mermaid's shape-oriented stylesheet inside the SVG (cream fill plus a thick stroke in the node color), rendering as unreadable outlines; inline styles on the replacement text now carry the rendered label's color and font. Labels also keep the source foreignObject's x/y offset instead of assuming the origin. Object URL revocation moves out of the click task so the download cannot be aborted by immediate cleanup.
Summary
The audit's weakest area against the adoption goal was shareability: the rendered DAG graph SVG sat in the DOM with no way out, and sharing anything meant OS screenshots or the address bar.
Stacked on #2531 (uses its `useCopyFeedback` hook).
Validation
Summary by CodeRabbit
New Features
Improvements