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

feat(sankey): Show values #4748

Merged
merged 5 commits into from
Aug 23, 2023
Merged

feat(sankey): Show values #4748

merged 5 commits into from
Aug 23, 2023

Conversation

sidharthv96
Copy link
Member

@sidharthv96 sidharthv96 commented Aug 19, 2023

📑 Summary

Show values with sankey diagrams.
image

Resolves #4674

📏 Design Decisions

Currently, I've added prefix and suffix as config options, but I doubt that's the best approach.

Either, we should support all config values in the frontmatter, which would be a big change.
Or, we could add support for prefix and suffix into the grammar. (Which we can add/remove as sankey is still in beta).

📋 Tasks

Make sure you

@github-actions github-actions bot added the Type: Enhancement New feature or request label Aug 19, 2023
@codecov
Copy link

codecov bot commented Aug 19, 2023

Codecov Report

Merging #4748 (ce6992e) into develop (adfed1e) will increase coverage by 29.75%.
The diff coverage is 15.38%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##           develop    #4748       +/-   ##
============================================
+ Coverage    45.55%   75.31%   +29.75%     
============================================
  Files           53      146       +93     
  Lines         6629    14569     +7940     
  Branches        32      592      +560     
============================================
+ Hits          3020    10972     +7952     
+ Misses        3608     3478      -130     
- Partials         1      119      +118     
Flag Coverage Δ
e2e 81.53% <15.38%> (?)
unit 45.55% <ø> (ø)

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

Files Changed Coverage Δ
packages/mermaid/src/diagrams/sankey/sankeyDB.ts 91.66% <ø> (ø)
...ages/mermaid/src/diagrams/sankey/sankeyRenderer.ts 74.19% <15.38%> (ø)

... and 140 files with indirect coverage changes

@sidharthv96
Copy link
Member Author

sidharthv96 commented Aug 19, 2023

Should we follow pie convention and add a showData option in the grammar?
Something like showData("$", "B") could handle the prefix and suffix as well.

showData could be showValues too.

Using grammar

sankey-beta

%% Don't show values. Values will be shown by default.
hideData

%% Prefix and suffix
showData("$", "B")

%% Prefix only
showData("$", "")
showData("$")

%% Suffix only
showData("", "Tons")

Using frontmatter

---
config:
  sankey:
    prefix: $
    suffix: B 
---
sankey-beta

@Yokozuna59
Copy link
Member

@sidharthv96 I agree on adding it grammar, although it won't be simple CSV style.

But the suggested sample code is kind of weird; although it can easily stick in your mind when it has been mentioned in the documentation, I don't think users would get the idea from just looking at the code.

If we couldn't find a cleaner way I'd suggest just adding to config instead of making grammar weird.

* feature/frontmatterConfig:
  Update docs
  docs: Add frontmatter config demos
  docs: Add frontmatter config docs
  fix: XSS vulnerability
  chore: Minor typo fixes
  chore: Add test with both frontmatter and directive
  Update docs
  feat: Add support for config in frontmatter
  deps: Update unocss and webpack to address vulnerability.
  chore: Fix type in assignWithDepth
  refactor: Move `sanitizeDirective` into `addDirective`
  refactor: Rename and cleanup `directiveSanitizer`
  chore(deps): update all patch dependencies
  ci(release-drafter): add more release notes categories
@sidharthv96 sidharthv96 changed the base branch from develop to feature/frontmatterConfig August 21, 2023 14:53
@sidharthv96 sidharthv96 self-assigned this Aug 21, 2023
* feature/frontmatterConfig:
  chore: Remove duplicate CI action
  chore: Add circular dependency check in CI
  refactor: Remove circular dependencies
* develop:
  chore: Remove circular dependency
@sidharthv96 sidharthv96 changed the base branch from feature/frontmatterConfig to develop August 22, 2023 08:29
@knsv knsv added this pull request to the merge queue Aug 23, 2023
Merged via the queue into develop with commit c3160b2 Aug 23, 2023
20 of 21 checks passed
fuxingloh referenced this pull request in fuxingloh/contented Aug 28, 2023
[![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.3.1` ->
`10.4.0`](https://renovatebot.com/diffs/npm/mermaid/10.3.1/10.4.0) |
[![age](https://developer.mend.io/api/mc/badges/age/npm/mermaid/10.4.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/mermaid/10.4.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/mermaid/10.3.1/10.4.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/mermaid/10.3.1/10.4.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

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

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

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

#### Features

- feat: Support config in frontmatter. by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/4750](https://togithub.com/mermaid-js/mermaid/pull/4750)
- feat(sankey): Show values by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/4748](https://togithub.com/mermaid-js/mermaid/pull/4748)

#### Docs

- docs: Add development example page. by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/4714](https://togithub.com/mermaid-js/mermaid/pull/4714)
- Documentation for
[#&#8203;2509](https://togithub.com/mermaid-js/mermaid/issues/2509) by
[@&#8203;jason-curtis](https://togithub.com/jason-curtis) in
[https://github.com/mermaid-js/mermaid/pull/4740](https://togithub.com/mermaid-js/mermaid/pull/4740)
- Fixes to Docs sidebar, main page and badges by
[@&#8203;nirname](https://togithub.com/nirname) in
[https://github.com/mermaid-js/mermaid/pull/4742](https://togithub.com/mermaid-js/mermaid/pull/4742)
- Split development documentation into several pages by
[@&#8203;nirname](https://togithub.com/nirname) in
[https://github.com/mermaid-js/mermaid/pull/4744](https://togithub.com/mermaid-js/mermaid/pull/4744)
- Docs: update Latest News section by
[@&#8203;huynhicode](https://togithub.com/huynhicode) in
[https://github.com/mermaid-js/mermaid/pull/4768](https://togithub.com/mermaid-js/mermaid/pull/4768)

#### Chores

- Update all minor dependencies (minor) by
[@&#8203;renovate](https://togithub.com/renovate) in
[https://github.com/mermaid-js/mermaid/pull/4732](https://togithub.com/mermaid-js/mermaid/pull/4732)
- Update all patch dependencies (patch) by
[@&#8203;renovate](https://togithub.com/renovate) in
[https://github.com/mermaid-js/mermaid/pull/4731](https://togithub.com/mermaid-js/mermaid/pull/4731)
- convert `assignWithDepth` to TS by
[@&#8203;Yokozuna59](https://togithub.com/Yokozuna59) in
[https://github.com/mermaid-js/mermaid/pull/4717](https://togithub.com/mermaid-js/mermaid/pull/4717)
- convert `diagrams/common/svgDrawCommon.js` to ts by
[@&#8203;Yokozuna59](https://togithub.com/Yokozuna59) in
[https://github.com/mermaid-js/mermaid/pull/4724](https://togithub.com/mermaid-js/mermaid/pull/4724)
- ci(release-drafter): add more release notes categories by
[@&#8203;aloisklink](https://togithub.com/aloisklink) in
[https://github.com/mermaid-js/mermaid/pull/4752](https://togithub.com/mermaid-js/mermaid/pull/4752)
- chore(deps): update all patch dependencies (patch) by
[@&#8203;renovate](https://togithub.com/renovate) in
[https://github.com/mermaid-js/mermaid/pull/4753](https://togithub.com/mermaid-js/mermaid/pull/4753)
- standardized pie definitions by
[@&#8203;Yokozuna59](https://togithub.com/Yokozuna59) in
[https://github.com/mermaid-js/mermaid/pull/4501](https://togithub.com/mermaid-js/mermaid/pull/4501)
- Remove Circular Dependencies by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/4761](https://togithub.com/mermaid-js/mermaid/pull/4761)
- chore: Enforce type imports by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/4763](https://togithub.com/mermaid-js/mermaid/pull/4763)
- chore: Preview PRs with mermaid-live-editor on Netlify by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/4769](https://togithub.com/mermaid-js/mermaid/pull/4769)

#### New Contributors

- [@&#8203;jason-curtis](https://togithub.com/jason-curtis) made their
first contribution in
[https://github.com/mermaid-js/mermaid/pull/4740](https://togithub.com/mermaid-js/mermaid/pull/4740)

**Full Changelog**:
mermaid-js/mermaid@v10.3.1...v10.4.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:eyJjcmVhdGVkSW5WZXIiOiIzNi41Ni4wIiwidXBkYXRlZEluVmVyIjoiMzYuNTYuMCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

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: Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sankey nodes doesn't show values
4 participants