fix(apollo-react): edge labels lose background and can render under crossing lines#955
Draft
dbacomputer wants to merge 2 commits into
Draft
fix(apollo-react): edge labels lose background and can render under crossing lines#955dbacomputer wants to merge 2 commits into
dbacomputer wants to merge 2 commits into
Conversation
…nder crossing lines Two bugs in CanvasEdge/SequenceEdge labels, plus a new Storybook reference page documenting label behavior across themes and edge cases. Bug 1 — line renders over the label: EdgeLabel rendered as a raw foreignObject inside its own edge's <g>, so it competed in the same per-edge z-index/DOM-order stack as every other edge's stroke. An unselected edge crossing near a label could paint its line over that label. Fixed by portaling the label through xyflow's EdgeLabelRenderer instead (the pattern StageEdge/EdgeToolbar already use), which always paints after every edge's own <svg>, regardless of z-index or array order. Bug 2 — label background can render fully transparent: `background: var(--canvas-background)` had no fallback value. That variable only resolves under a themed ancestor (body.light/.future-*/.vertex/.canvas per canvas/styles/variables.css); a host that mounts the canvas without one (e.g. a shadow-DOM host) got a transparent label with illegible text over the line. Added a `--color-background` fallback to both EdgeLabel.tsx and StageEdge.tsx's StageEdgeLabel. New: Components/Edges/EdgeLabels Storybook page Consolidates label documentation that was previously scattered (and, in one spot, stale — SequenceEdge.stories.tsx's old EdgeLabels story still claimed labels render via "SVG foreignObject") into one dedicated reference page: - Orientation, Diff States: baseline label behavior - Crossing Labeled Edges: regression coverage for Bug 1 - Themes: the same label across all 9 canvas themes side by side - Missing Theme Fallback: reproduces Bug 2's scenario directly (simulates an unthemed ancestor via CSS custom-property `initial`), and documents the one remaining gap the fallback doesn't cover (no themed ancestor at all, background still transparent) - Overflow: long label text and short-edge crowding (known, undocumented limitations, not fixed here) - Execution Status, Bent Path, Read Only: label composes correctly with execution-status coloring, multi-segment waypoint routing, and readonly mode Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Dependency License Review
License distribution
Excluded packages
|
There was a problem hiding this comment.
Pull request overview
Fixes two edge-label rendering issues in the apollo-react canvas: (1) labels being obscured by crossing edge strokes due to per-edge SVG stacking order, and (2) label backgrounds becoming transparent when canvas theme variables aren’t present. It also consolidates label behavior guidance into a new Storybook reference page.
Changes:
- Portal
EdgeLabelthrough xyflow’sEdgeLabelRendererto ensure labels paint above all edge SVG strokes. - Add CSS variable fallbacks for label backgrounds (and hover background for
StageEdgeLabel) to avoid transparent labels in unthemed/partially themed hosts. - Replace scattered/partially stale label Storybook content with a dedicated
Components/Edges/EdgeLabelsreference page.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/apollo-react/src/canvas/components/StageNode/StageEdge.tsx | Adds background/hover background fallbacks for stage edge labels. |
| packages/apollo-react/src/canvas/components/Edges/shared/primitives/EdgeLabel.tsx | Switches label rendering to EdgeLabelRenderer portal and adds background fallback styling via Tailwind classes. |
| packages/apollo-react/src/canvas/components/Edges/shared/primitives/EdgeLabel.test.tsx | Updates unit tests to reflect portal-based markup and class-based styling. |
| packages/apollo-react/src/canvas/components/Edges/SequenceEdge.stories.tsx | Removes the old label story and points readers to the new reference page. |
| packages/apollo-react/src/canvas/components/Edges/EdgeLabel.stories.tsx | New consolidated Storybook page documenting label behavior across edge cases and themes. |
Comment on lines
22
to
25
| &:hover { | ||
| background: var(--canvas-background-hover); | ||
| background: var(--canvas-background-hover, var(--color-background-hover)); | ||
| border-color: var(--canvas-border-hover); | ||
| } |
Comment on lines
+12
to
+13
| // Falls back to --color-background when a host doesn't import canvas/styles/variables.css | ||
| // (e.g. a shadow-DOM host), so the label never renders with a transparent background. |
Comment on lines
+385
to
+392
| * Simulates a host that does not theme its canvas ancestor. `--canvas-background` | ||
| * and `--color-background` are set to `initial` (the CSS-spec guaranteed-invalid | ||
| * value) on a wrapping div, which is what actually happens when a host mounts | ||
| * the canvas outside any `body.<theme>` / `.future-*` / `.vertex` / `.canvas` | ||
| * ancestor (for example, a shadow-DOM host). Column 2 shows the fix's fallback | ||
| * resolving; column 3 is the known remaining gap: when neither variable | ||
| * resolves, the label background is transparent. | ||
| */ |
Comment on lines
+420
to
+423
| <div style={{ ...columnStyle, '--canvas-background': 'initial' } as CSSProperties}> | ||
| <p style={{ fontSize: 13, margin: 0 }}> | ||
| --canvas-background unset (variables.css not imported): falls back to --color-background. | ||
| </p> |
Comment on lines
+433
to
+436
| ...columnStyle, | ||
| '--canvas-background': 'initial', | ||
| '--color-background': 'initial', | ||
| } as CSSProperties |
Comment on lines
+439
to
+442
| <p style={{ fontSize: 13, margin: 0 }}> | ||
| Both unset, no themed ancestor at all: background is transparent. Known gap, not covered | ||
| by the current fallback chain. | ||
| </p> |
Contributor
📊 Coverage + size by packagePer-package coverage and bundle size on this PR. New-line coverage = of the source lines this PR adds or changes, the % hit by tests.
"Coverage" is each package's own |
github-actions
Bot
added
size:XXL
dbacomputer
force-pushed
the
add-label-background-color
branch
from
July 22, 2026 20:50
44a5899 to
69aed8c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
packages/apollo-react/src/canvas/components/Edges/EdgeLabel.stories.tsx:392
- The MissingThemeFallback story comment says setting custom props to
initialis a "guaranteed-invalid value", butinitialis a valid CSS-wide keyword and will not trigger var() fallback. The comment should describe the actual mechanism used to force fallback/unresolved variables.
/**
* Simulates a host that does not theme its canvas ancestor. `--canvas-background`
* and `--color-background` are set to `initial` (the CSS-spec guaranteed-invalid
* value) on a wrapping div, which is what actually happens when a host mounts
* the canvas outside any `body.<theme>` / `.future-*` / `.vertex` / `.canvas`
* ancestor (for example, a shadow-DOM host). Column 2 shows the fix's fallback
* resolving; column 3 is the known remaining gap: when neither variable
* resolves, the label background is transparent.
*/
packages/apollo-react/src/canvas/components/Edges/EdgeLabel.stories.tsx:423
- Setting
--canvas-background: initialdoes not simulate an unresolved/missing variable for var() fallback (it makes the resolved value the CSS keywordinitial, which yields a transparent background and skips the fallback). Use a missing var() reference to make the custom property invalid at computed-value time so the fallback to--color-backgroundis exercised.
<div style={{ ...columnStyle, '--canvas-background': 'initial' } as CSSProperties}>
<p style={{ fontSize: 13, margin: 0 }}>
--canvas-background unset (variables.css not imported): falls back to --color-background.
</p>
packages/apollo-react/src/canvas/components/Edges/EdgeLabel.stories.tsx:436
- Similarly, setting both variables to
initialwill not demonstrate the "neither resolves" scenario; it sets both to a valid keyword and prevents var() fallback. For the intended demo, both variables should be forced invalid (via a missing var()) so the label background becomes transparent due to an unresolved chain.
...columnStyle,
'--canvas-background': 'initial',
'--color-background': 'initial',
} as CSSProperties
Comment on lines
+314
to
+330
| /** | ||
| * The same labeled edge under every canvas theme. Each cell forces its theme | ||
| * class on a wrapping div (the same selectors variables.css matches on | ||
| * `body.<theme>`), independent of the global Storybook theme toolbar, so all | ||
| * nine render simultaneously for comparison. | ||
| */ | ||
| const THEME_CLASSES = [ | ||
| 'light', | ||
| 'dark', | ||
| 'light-hc', | ||
| 'dark-hc', | ||
| 'future-light', | ||
| 'future-dark', | ||
| 'wireframe', | ||
| 'vertex', | ||
| 'canvas', | ||
| ] as const; |
Comment on lines
+372
to
+382
| export const Themes: Story = { | ||
| render: () => <ThemesStory />, | ||
| parameters: { | ||
| docs: { | ||
| description: { | ||
| story: | ||
| 'The same labeled edge rendered under all nine canvas themes at once, to compare label contrast against each theme\'s background and border colors.', | ||
| }, | ||
| }, | ||
| }, | ||
| }; |
…view Node labels support double-click-to-edit (BaseNode/NodeLabel's EditableLabel), but edge labels do not. EdgeLabel is pointer-events: none and CanvasEdgeData has no onLabelChange field, so there's no way to rename a label from the canvas. Clicking the label also passes straight through to the edge underneath and selects the whole edge, since the label isn't a hit target itself, the same root cause as the missing editing support: enabling pointer events on the label to support editing also changes what clicking it does today. Adds a story that surfaces both observations together and lays out two paths forward (keep labels pure-display vs. add inline editing, with the concrete changes each would require) as an open question for design/ product review, rather than a decision made here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
dbacomputer
force-pushed
the
add-label-background-color
branch
from
July 22, 2026 20:57
69aed8c to
e81a659
Compare
Comment on lines
12
to
+13
| color: var(--canvas-foreground); | ||
| background: var(--canvas-background); | ||
| background: var(--canvas-background, var(--color-background)); |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
packages/apollo-react/src/canvas/components/Edges/EdgeLabel.stories.tsx:425
- The story comment says setting custom properties to
initialis “CSS-spec guaranteed-invalid”, butinitialis a valid keyword forbackground/borderand won’t exercise thevar(--canvas-background, var(--color-background))fallback chain. Consider describing (and using) an invalid-at-computed-value setup instead (e.g. a self-referentialvar()cycle) so the story matches what the component actually does.
* Simulates a host that does not theme its canvas ancestor. `--canvas-background`
* and `--color-background` are set to `initial` (the CSS-spec guaranteed-invalid
* value) on a wrapping div, which is what actually happens when a host mounts
* the canvas outside any `body.<theme>` / `.future-*` / `.vertex` / `.canvas`
* ancestor (for example, a shadow-DOM host). Column 2 shows the fix's fallback
packages/apollo-react/src/canvas/components/Edges/EdgeLabel.stories.tsx:459
- Setting
--canvas-background: 'initial'here won’t trigger the fallback to--color-backgroundbecause the variable is still defined (and resolves to the validinitialkeyword). Use an invalid-at-computed-value value (e.g. a self-referential var) so this column actually demonstrates the fallback behavior described in the text.
<div style={{ ...columnStyle, '--canvas-background': 'initial' } as CSSProperties}>
<p style={{ fontSize: 13, margin: 0 }}>
--canvas-background unset (variables.css not imported): falls back to --color-background.
</p>
packages/apollo-react/src/canvas/components/Edges/EdgeLabel.stories.tsx:472
- Same issue as the prior column:
initialdoesn’t simulate “unset”. If the goal is to show the remaining gap when neither--canvas-backgroundnor--color-backgroundresolve, make both variables invalid at computed-value time (self-referential) so the label’s background computation actually ends up transparent.
{
...columnStyle,
'--canvas-background': 'initial',
'--color-background': 'initial',
} as CSSProperties
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
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.
Summary
Two bugs in
CanvasEdge/SequenceEdgelabels, plus a new Storybook reference page documenting label behavior across themes and edge cases.Bug 1 — line renders over the label
EdgeLabelrendered as a rawforeignObjectinside its own edge's<g>, so it competed in the same per-edge z-index/DOM-order stack as every other edge's stroke. An unselected edge crossing near a label could paint its line over that label.Fix: portal the label through xyflow's
EdgeLabelRendererinstead (the patternStageEdge/EdgeToolbaralready use), which always paints after every edge's own<svg>, regardless of z-index or array order.Bug 2 — label background can render fully transparent
background: var(--canvas-background)had no fallback value. That variable only resolves under a themed ancestor (body.light/.future-*/.vertex/.canvaspercanvas/styles/variables.css); a host that mounts the canvas without one (e.g. a shadow-DOM host) got a transparent label with illegible text over the line.Fix: added a
--color-backgroundfallback to bothEdgeLabel.tsxandStageEdge.tsx'sStageEdgeLabel.New:
Components/Edges/EdgeLabelsStorybook pageConsolidates label documentation that was previously scattered across
CanvasEdge.stories.tsxandSequenceEdge.stories.tsx(and, in one spot, stale — the oldSequenceEdgeEdgeLabelsstory still claimed labels render via "SVG foreignObject") into one dedicated reference page:initial), and transparently documents the one remaining gap the fallback doesn't cover (no themed ancestor at all → background still transparent)BaseNode/NodeLabel'sEditableLabel); edge labels don't (EdgeLabelispointer-events: none,CanvasEdgeDatahas noonLabelChange). Clicking the label also passes through to the edge underneath and selects it instead, since the label isn't a hit target — the same root cause as the missing editing support, not a separate bug. The story lays out both paths forward:data.labelupstream.onLabelChangecallback onCanvasEdgeData,pointer-events: autoon the label (likely gated on a handler being provided), an explicit decision on click semantics (does a single click still select the edge, does double-click enter edit mode likeNodeLabel), an inline textarea reusing that pattern, and handling for empty labels / Escape-to-cancel / Enter-to-commit.Out of scope (tracked as a follow-up)
SequenceEdge.stories.tsx'sExecutionStates/ValidationStatesstories duplicate content already onCanvasEdge.stories.tsx(SequenceEdge is a thin preset over CanvasEdge, so the underlying status-to-color logic is identical). Left alone here to keep this PR focused; will be addressed separately.Test plan
EdgeLabel.test.tsxupdated for the newEdgeLabelRenderer-based markupHandleRouting, etc.)🤖 Generated with Claude Code