dia.Link: add getComputedLabel()/getComputedLabels() - #3528
Open
Geliogabalus wants to merge 5 commits into
Open
Geliogabalus wants to merge 5 commits into
Geliogabalus wants to merge 5 commits into
Conversation
label()/labels() always returned a label exactly as stored, leaving every caller to separately re-resolve it against defaultLabel/the built-in default (LinkView duplicated this merge logic in several places: rendering, label dragging, RotateLabel). getComputedLabel()/getComputedLabels() centralize that resolution in link-labels.mjs and expose it directly, and LinkView now calls them instead of re-implementing the merge inline. A label (or defaultLabel) may also carry custom properties beyond markup/ attrs/size/position - these pass through resolution unmodified, with the label's own value winning over defaultLabel's.
kumilingus
reviewed
Sep 30, 2026
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Computed labels expose shared mutable built-in markup and attributes, allowing callers to alter defaults globally.
Review effort: Balanced
Findings: 1
Open (2)
What changed in this PR
Adds computed label APIs to centralize label/default resolution and reuse it throughout link rendering and tools.
Changes:
- Adds
getComputedLabel()andgetComputedLabels(). - Refactors
LinkViewandRotateLabelto use resolved labels. - Adds types, tests, and a changeset.
| File | Description |
|---|---|
.changeset/brave-labels-resolve.md |
Records the new APIs. |
packages/joint-core/src/dia/Link.mjs |
Exposes computed-label methods. |
packages/joint-core/src/dia/link-labels.mjs |
Centralizes label resolution. |
packages/joint-core/src/dia/LinkView.mjs |
Uses resolved labels for rendering and dragging. |
packages/joint-core/src/linkTools/RotateLabel.mjs |
Uses computed label positions. |
packages/joint-core/types/dia.d.ts |
Declares computed-label types and methods. |
packages/joint-core/test/jointjs/links.js |
Tests computed-label resolution. |
packages/joint-core/test/jointjs/linkView.js |
Tests invalid computed positions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1
to
+23
| import { merge } from '../util/index.mjs'; | ||
|
|
||
| // A label as given (own `markup`/`attrs`/`size`/`position`, any of which may be missing), | ||
| // resolved against `link`'s `defaultLabel` and its built-in default. | ||
| export function getComputedLabel(link, label) { | ||
|
|
||
| label = label || {}; | ||
|
|
||
| const builtinDefaultLabel = link._builtins.defaultLabel; | ||
| const defaultLabel = link._getDefaultLabel(); | ||
|
|
||
| // A label's own or `defaultLabel`'s markup, if either is set, is "custom" - the | ||
| // built-in default attrs (`builtinDefaultLabelAttrs`) only make sense for the | ||
| // built-in markup, so they don't apply once a custom one is in play. | ||
| const hasCustomMarkup = !!(label.markup || defaultLabel.markup); | ||
|
|
||
| return Object.assign({}, defaultLabel, label, { | ||
| markup: label.markup || defaultLabel.markup || builtinDefaultLabel.markup, | ||
| attrs: mergeLabelAttrs(hasCustomMarkup, label.attrs, defaultLabel.attrs, builtinDefaultLabel.attrs), | ||
| size: mergeLabelSize(label.size, defaultLabel.size, builtinDefaultLabel.size), | ||
| position: mergeLabelPosition(label.position, defaultLabel.position, builtinDefaultLabel.position) | ||
| }); | ||
| } |
| * The stored label is not modified. | ||
| * | ||
| * @param {number} [idx=0] - The index of the label. Negative values count from the end. | ||
| * @returns {dia.Link.Label | null} A new object with the resolved label, or `null` |
This branch has not been deployed
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
Link#label()/labels()always return a label exactly as stored, leaving every caller to separately re-resolve it againstdefaultLabel/the built-in default.LinkViewduplicated that merge logic in several places (rendering, label dragging,RotateLabel).getComputedLabel(index?)/getComputedLabels(), which return each label resolved againstdefaultLabel/the built-in default, centralizing that resolution into a newlink-labels.mjs.LinkViewnow calls these instead of re-implementing the merge inline.defaultLabel) may carry custom properties beyondmarkup/attrs/size/position- these pass through resolution unmodified, with the label's own value winning overdefaultLabel's.label()/labels()themselves are unchanged - still raw, as stored.Test plan
grunt karma:joint- 2117/2119 passing; the 2 failures (util.breakText ellipsis,element ports > port labels label attributes) are pre-existing, unrelated to this change (confirmed present on a cleanupstream/mastercheckout too, both in unrelated subsystems - text measurement and port label rotation matrix precision)grunt test:ts- passinggetComputedLabel/getComputedLabels(resolution againstdefaultLabel, custom property pass-through) intest/jointjs/links.js