Repository navigation
fix: click action for subgraphs - #7988
Open
thiagobarbosa wants to merge 3 commits into
Open
thiagobarbosa wants to merge 3 commits into
thiagobarbosa wants to merge 3 commits into
Conversation
Resolves mermaid-js#5428 mermaid-js#5428 Generated by issue-coder. Review before pushing.
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🦋 Changeset detectedLatest commit: f53dcc2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
❌ Lockfile Validation Failed The following issue(s) were detected: Please address these and push an update. Posted automatically by GitHub Actions |
Contributor
📝 WalkthroughWalkthroughFlowchart subgraphs now support click callbacks and links, including link targets and interaction metadata for collapsed and non-collapsed nodes. Cluster labels render linked subgraphs as SVG anchors, with tests covering links, clickable classes, and explicit targets. ChangesSubgraph interaction support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant FlowchartParser
participant FlowDB
participant ClusterRenderer
participant SVG
FlowchartParser->>FlowDB: Parse subgraph click link and target
FlowDB->>ClusterRenderer: Provide subgraph interaction metadata
ClusterRenderer->>SVG: Render linked label as an anchor
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@packages/mermaid/src/diagrams/flowchart/flowDb.ts`:
- Around line 1226-1229: Update the collapsed-subgraph handling in the flowchart
database logic to preserve and propagate link, linkTarget, haveCallback, and
tooltip metadata before rendering. Ensure both collapsed and non-collapsed
branches populate these fields, using subGraph.id with getTooltip for the
tooltip, so collapsed subgraph interactions remain functional.
In `@packages/mermaid/src/rendering-util/rendering-elements/clusters.js`:
- Around line 31-48: Update the link-target selection in the cluster label
anchor creation so non-sandbox links default to `_blank` when `node.linkTarget`
is absent, while preserving explicit `node.linkTarget` values and the sandbox
`_top` behavior.
🪄 Autofix (Beta)
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: 3017772d-817b-4bc6-b28f-385ceb258d7f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
packages/mermaid/src/diagrams/flowchart/flowDb.tspackages/mermaid/src/diagrams/flowchart/parser/flow-interactions.spec.jspackages/mermaid/src/diagrams/flowchart/types.tspackages/mermaid/src/rendering-util/rendering-elements/clusters.js
- Propagate link/linkTarget/haveCallback/tooltip to collapsed subgraph nodes, not just the non-collapsed branch, so click on a collapsed subgraph works too. - Default cluster label link target to `_blank` outside sandbox mode when no explicit linkTarget is set, instead of leaving it unset. - Restore pnpm-lock.yaml to drop unintended churn that broke `validate-lockfile`/`--frozen-lockfile` installs in CI. - Add missing changeset.
@mermaid-js/examples
mermaid
@mermaid-js/layout-elk
@mermaid-js/layout-tidy-tree
@mermaid-js/mermaid-zenuml
@mermaid-js/parser
@mermaid-js/tiny
commit: |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #7988 +/- ##
===========================================
+ Coverage 77.46% 77.51% +0.04%
===========================================
Files 564 564
Lines 74906 74934 +28
Branches 12661 12667 +6
===========================================
+ Hits 58026 58083 +57
+ Misses 15881 15853 -28
+ Partials 999 998 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
- Add tests to verify `<a>` wrapping for cluster labels based on `node.link`. - Test different `linkTarget` values and behavior under `securityLevel: sandbox`. - Ensure no anchor wrapping occurs when `node.link` is not set.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/mermaid/src/rendering-util/rendering-elements/clusters.spec.js`:
- Around line 63-68: Update the test suite setup around the global
SVGElement.prototype.getBBox stub to capture its original property descriptor
before replacement and restore that descriptor in afterEach or afterAll. Ensure
the restoration removes the stub and reinstates the original implementation for
subsequent tests.
🪄 Autofix (Beta)
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: 56627fd2-dce9-4235-9ae8-c94570d835d7
📒 Files selected for processing (1)
packages/mermaid/src/rendering-util/rendering-elements/clusters.spec.js
Comment on lines
+63
to
+68
| global.SVGElement.prototype.getBBox = () => ({ | ||
| x: 0, | ||
| y: 0, | ||
| width: 40, | ||
| height: 20, | ||
| }); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Restore the global getBBox stub after the suite.
This permanently replaces SVGElement.prototype.getBBox, leaking the stub into later tests and potentially masking layout behavior. Save and restore the original property descriptor in afterEach/afterAll.
🤖 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 `@packages/mermaid/src/rendering-util/rendering-elements/clusters.spec.js`
around lines 63 - 68, Update the test suite setup around the global
SVGElement.prototype.getBBox stub to capture its original property descriptor
before replacement and restore that descriptor in afterEach or afterAll. Ensure
the restoration removes the stub and reinstates the original implementation for
subsequent tests.
1 of 4 tasks
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
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.
Fixes #5428
Summary
Adds support for the
clickdirective on flowchart subgraphs. Previouslyclickonly worked on individual nodes/vertices — writingclick mySubgraph "https://example.com"had no effect because the subgraph lookup was never checked and the resulting group node never received link data.Changes
flowDb.tssetClickEventnow falls back tothis.subGraphLookup.get(id)wheniddoesn't match a vertex, so callbacks can target subgraphs.setLinknow also writeslink/linkTargetonto the matching subgraph entry, not just vertices.link,linkTarget,haveCallback, andtooltipare now carried over from the subgraph definition onto the rendered node.types.ts:FlowSubGraphgains optionallink,linkTarget, andhaveCallbackfields to carry this data through.clusters.js: therectcluster renderer wraps the cluster label in an<a>element whennode.linkis set (mirroring how regular node links are rendered), respectinglinkTargetand forcing_topundersecurityLevel: 'sandbox'.flow-interactions.spec.js: adds coverage forclick <subgraph> "<url>"(with and without a target), asserting both the parsed subgraph data and the resulting rendered group node carry the link/class.pnpm-lock.yaml: lockfile churn from a routinepnpm install(no intentional dependency changes).Testing
flow-interactions.spec.jscovering parsing aclickon a subgraph both with and without an explicit link target.clickon a subgraph and confirm the group label becomes a clickable link in the browser.Notes / follow-ups
.changeset/; consider adding one before merging per this repo's release process.click— worth a follow-up doc update.Summary
Adds support for
clickdirectives on flowchart subgraphs.click/hrefapplied to a subgraph ID correctly resolves the target (vertex or subgraph), records callback presence (haveCallback), and associates link metadata.linkandlinkTarget(formatted via existingformatUrllogic) and are markedclickable.shape: 'collapsedGroup'), andshape: 'rect'),the emitted rendered node now includes
link,linkTarget,haveCallback, andtooltip.node.linkis present the cluster label is wrapped in an SVG<a>:_topwhensecurityLevel: 'sandbox',node.linkTarget(or defaults to_blank).FlowSubGraphtype with optionallink,linkTarget, andhaveCallback.linkTargethandling) and validates cluster label anchor rendering behavior with sandbox security rules.clickon flowchart subgraphs.