Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug/5059 fix external connection after updating edges #5127

Conversation

mathbraga
Copy link
Contributor

@mathbraga mathbraga commented Dec 8, 2023

📑 Summary

Signals a subgraph with the external connection flag in case one of its children is used as an anchor node linking to outside.

Resolves #5059

📏 Design Decisions

So when a subgraph has a link to an outside node, it picks a "fake node" to be used as a reference for edges it is part of. The issue is that when it does so to a node that is part of another internal subgraph, this subgraph will now also have a link to the outside, so it needs to be updated with the cluster.externalConnections = true.

Only solving the issue described above can make some layouts look inconsistent, the one reported in #5059 would look like this:
image

I believe this behaviour occurs because dagre-d3 identifies the edge going down from WireProperty1 (the fake node in this case), and positions the nodes horizontally so that the edge won't cut through other child nodes in the subgraph.

So I also added an implementation to change the fake node to its parent in case it does not originally have outside links, making it a valid target for an edge. In my opinion the resulting layout better represents its input code:
image

📋 Tasks

Make sure you

Copy link

netlify bot commented Dec 8, 2023

Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit 679f77d
🔍 Latest deploy log https://app.netlify.com/sites/mermaid-js/deploys/65aef8cdb374ac0008531515
😎 Deploy Preview https://deploy-preview-5127--mermaid-js.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions github-actions bot added the Type: Bug / Error Something isn't working or is incorrect label Dec 8, 2023
Copy link

codecov bot commented Dec 8, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7b62466) 79.32% compared to head (679f77d) 79.34%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #5127      +/-   ##
===========================================
+ Coverage    79.32%   79.34%   +0.01%     
===========================================
  Files          167      167              
  Lines        13873    13882       +9     
  Branches       707      707              
===========================================
+ Hits         11005    11014       +9     
  Misses        2715     2715              
  Partials       153      153              
Flag Coverage Δ
e2e 85.13% <100.00%> (+0.01%) ⬆️
unit 43.04% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...ages/mermaid/src/dagre-wrapper/mermaid-graphlib.js 91.32% <100.00%> (+0.37%) ⬆️

@sidharthv96 sidharthv96 added this pull request to the merge queue Jan 19, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 19, 2024
@sidharthv96 sidharthv96 added this pull request to the merge queue Jan 23, 2024
Merged via the queue into mermaid-js:develop with commit 1612d3f Jan 23, 2024
20 checks passed
fuxingloh pushed a commit to fuxingloh/contented that referenced this pull request Feb 3, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [mermaid](https://togithub.com/mermaid-js/mermaid) | [`10.7.0` ->
`10.8.0`](https://renovatebot.com/diffs/npm/mermaid/10.7.0/10.8.0) |
[![age](https://developer.mend.io/api/mc/badges/age/npm/mermaid/10.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/mermaid/10.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/mermaid/10.7.0/10.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/mermaid/10.7.0/10.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>mermaid-js/mermaid (mermaid)</summary>

###
[`v10.8.0`](https://togithub.com/mermaid-js/mermaid/releases/tag/v10.8.0)

[Compare
Source](https://togithub.com/mermaid-js/mermaid/compare/v10.7.0...v10.8.0)

### v10.8.0

#### Features

<img width="375" alt="image"
src="https://github.com/mermaid-js/mermaid/assets/5837277/c17405fb-2a06-4f7c-9c77-5eaba6c76747">

- Adding new diagram type - Block Diagram by
[@&#8203;knsv](https://togithub.com/knsv) in
[mermaid-js/mermaid#5221

- Feature/5114 add parallel commit config by
[@&#8203;mathbraga](https://togithub.com/mathbraga) in
[mermaid-js/mermaid#5161

- Changes to Gantt Parsers to allow hashes and semicolons to titles,
sections, and task data. by
[@&#8203;FutzMonitor](https://togithub.com/FutzMonitor) in
[mermaid-js/mermaid#5095

- Feature/4653 add actor-top class to sequence diagram by
[@&#8203;Ronid1](https://togithub.com/Ronid1) in
[mermaid-js/mermaid#5241

#### Documentation

- Updated gantt chart docs to show all config options by
[@&#8203;murdoa](https://togithub.com/murdoa) in
[mermaid-js/mermaid#5192
- Contribution documentation improvements by
[@&#8203;nirname](https://togithub.com/nirname) in
[mermaid-js/mermaid#5132
- Update flowchart.md - how to use font-awesome
[#&#8203;5195](https://togithub.com/mermaid-js/mermaid/issues/5195) by
[@&#8203;arukiidou](https://togithub.com/arukiidou) in
[mermaid-js/mermaid#5196
- Add more detailed docs for Gantt tasks by
[@&#8203;sorenisanerd](https://togithub.com/sorenisanerd) in
[mermaid-js/mermaid#5194
- Docs/4974 reorder integration links by
[@&#8203;Ronid1](https://togithub.com/Ronid1) in
[mermaid-js/mermaid#5066
- docs: fix swimm link by
[@&#8203;Yokozuna59](https://togithub.com/Yokozuna59) in
[mermaid-js/mermaid#5219
- Update Slack community links to Discord by
[@&#8203;Olegt0rr](https://togithub.com/Olegt0rr) in
[mermaid-js/mermaid#5225
- Docs: Mermaid chart updates by
[@&#8203;huynhicode](https://togithub.com/huynhicode) in
[mermaid-js/mermaid#5232
- Fix typos in timeline syntax samples by
[@&#8203;sblom](https://togithub.com/sblom) in
[mermaid-js/mermaid#5139

#### Bug fixes

- Bug/5059 fix external connection after updating edges by
[@&#8203;mathbraga](https://togithub.com/mathbraga) in
[mermaid-js/mermaid#5127
- \[Fix] Sequence diagram actor menu popup by
[@&#8203;vitorsss](https://togithub.com/vitorsss) in
[mermaid-js/mermaid#5160
- fix: Dompurify Hooks by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#5236
- Accurate pie chart labeling for text alignment by
[@&#8203;JenningsWilliam](https://togithub.com/JenningsWilliam) in
[mermaid-js/mermaid#5141
- fix: Redirect of old URLs by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#5250
- Fixed Typo in ErrorRenderer.ts by
[@&#8203;FutzMonitor](https://togithub.com/FutzMonitor) in
[mermaid-js/mermaid#5256

#### Chores

- Revert "Revert 5041 feature/4935 subgraph title margin config option"
by [@&#8203;mathbraga](https://togithub.com/mathbraga) in
[mermaid-js/mermaid#5205
- build(deps-dev): bump follow-redirects from 1.15.2 to 1.15.5 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[mermaid-js/mermaid#5200
- chore(deps): update all patch dependencies (patch) by
[@&#8203;renovate](https://togithub.com/renovate) in
[mermaid-js/mermaid#5150
- E2E Image comparison by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#5208
- E2E test by [@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#5210
- Optimise caching of test results by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#5213
- Update update-browserlist.yml to fix deprecation and action fails by
[@&#8203;Abrifq](https://togithub.com/Abrifq) in
[mermaid-js/mermaid#5151
- UpdateCypress by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#5228
- Use node v20 by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#5248
- Convert Mindmap to TS by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#5247
- chore: Add interface naming Convention by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#5254

#### New Contributors

- [@&#8203;murdoa](https://togithub.com/murdoa) made their first
contribution in
[mermaid-js/mermaid#5192
- [@&#8203;arukiidou](https://togithub.com/arukiidou) made their first
contribution in
[mermaid-js/mermaid#5196
- [@&#8203;sorenisanerd](https://togithub.com/sorenisanerd) made their
first contribution in
[mermaid-js/mermaid#5194
- [@&#8203;Ronid1](https://togithub.com/Ronid1) made their first
contribution in
[mermaid-js/mermaid#5066
- [@&#8203;Olegt0rr](https://togithub.com/Olegt0rr) made their first
contribution in
[mermaid-js/mermaid#5225
- [@&#8203;vitorsss](https://togithub.com/vitorsss) made their first
contribution in
[mermaid-js/mermaid#5160
- [@&#8203;sblom](https://togithub.com/sblom) made their first
contribution in
[mermaid-js/mermaid#5139
- [@&#8203;JenningsWilliam](https://togithub.com/JenningsWilliam) made
their first contribution in
[mermaid-js/mermaid#5141

**Full Changelog**:
mermaid-js/mermaid@v10.7.0...v10.8.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/levaintech/contented).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xNTMuMiIsInVwZGF0ZWRJblZlciI6IjM3LjE1My4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug / Error Something isn't working or is incorrect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Impossible to link from a subgraph if it contains only subgraphs (and no node) -> syntax error in text
2 participants