fix: Adjust piechart viewbox for mobile devices with small width #4288
Conversation
This file contains 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
iwestlin
force-pushed
the
fix/pie-chart-viewbox
branch
from
88ad9cd
to
534dedc
Compare
sidharthv96
requested changes
Jun 26, 2023
sidharthv96
reviewed
Jun 26, 2023
iwestlin
added a commit
to iwestlin/mermaid
that referenced
this pull request
Jun 26, 2023
|
Hi, @sidharthv96 Thank you for taking the time to review my work. I have made the changes you suggested and pushed a new commit. |
iwestlin
added a commit
to iwestlin/mermaid
that referenced
this pull request
Jun 26, 2023
iwestlin
force-pushed
the
fix/pie-chart-viewbox
branch
from
0b79ce1
to
ac2b647
Compare
iwestlin
added a commit
to iwestlin/mermaid
that referenced
this pull request
Jun 26, 2023
iwestlin
force-pushed
the
fix/pie-chart-viewbox
branch
from
ac2b647
to
e41b252
Compare
iwestlin
added a commit
to iwestlin/mermaid
that referenced
this pull request
Jun 26, 2023
iwestlin
force-pushed
the
fix/pie-chart-viewbox
branch
from
e41b252
to
4937215
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #4288 +/- ##
===========================================
+ Coverage 79.30% 79.38% +0.08%
===========================================
Files 165 165
Lines 13860 13851 -9
Branches 704 703 -1
===========================================
+ Hits 10991 10995 +4
+ Misses 2719 2706 -13
Partials 150 150
Flags with carried forward coverage won't be shown. Click here to find out more.
|
iwestlin
force-pushed
the
fix/pie-chart-viewbox
branch
from
184ad50
to
f211ed6
Compare
sidharthv96
requested changes
Jul 7, 2023
4 tasks
* develop: (596 commits) chore(deps): update all minor dependencies chore(deps): update all patch dependencies fix: flowchart image without text fix Types chore: Update pnpm-lock chore: Add tests for calculateDeltaAndAngle fix: mermaid-js#5064 Handle case when line has only one point reset the testTimeout to 5 seconds and change it directly in the test update testTimeout from 5 seconds to 10 seconds Update all patch dependencies fix broken link add latest blog post Update all minor dependencies Update all patch dependencies Fix docs Update packages/mermaid/src/docs/community/questions-and-suggestions.md Update packages/mermaid/src/diagrams/class/classRenderer-v2.ts update edge ids draw top actors with lines first followed by messages Bump GitHub workflow actions to latest versions ...
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
github-actions
bot
added
the
Type: Bug / Error
aloisklink
approved these changes
Nov 28, 2023
There was a problem hiding this comment.
Looks good, assuming we can get E2E tests passing. I think I've fixed them locally, so I'll push up a fix (see 005c998).
Something to add to the PR description, the pieConfig.useWidth config field is no longer used, but that's okay in my opinion since this config option wasn't documented anyway.
It looks like this PR also improves pie charts that currently have labels that overflow, so this fix is extra nice 🥳:
|
@iwestlin, Thank you for the contribution! |
sidharthv96
added a commit
that referenced
this pull request
Dec 4, 2023
* develop: (47 commits) chore(deps): update all minor dependencies chore: Rename test test: Add unit test for generic classname and namespace Split type from generic class name Referenced the PmWiki's Cookbook recipe enabling MermaidJs schematics in wiki pages test(e2e): fix pie chart E2E tests for PR #4288 Add dummy commit to trigger GH checks chore: Revert unnecessary export refactor: Remove unnecessary calculations chore: Fix computeWidth function chore: Cleanup setupGraphViewbox Update docs update mermaidAPI to cleanup the text before passing to getDiagramFromText Add test for subgraphs with title margins and edge labels Modify margin logic to avoid creating unnecessary space in subgraph review fixes add test remove unused variable fix: clean comments in text in getDiagramFromText API so flowchart works well chore(deps): update all minor dependencies ...
sidharthv96
added a commit
that referenced
this pull request
Dec 6, 2023
* develop: (70 commits) build(deps-dev): bump vite from 4.4.9 to 4.4.12 Changes to .prettierignore 1. Added 'demos/dev/**' to be ignored by Prettier. 2. Added '!/demos/dev/example.html' so that Prettier ensures no one changes the example.html in a way that doesn't obey the Prettier code formatting rules. build: use `tsx` instead of `ts-node-esm` chore: Downgrade node to 18.18.2 fix: #5100 Add viewbox to sankey chore(deps): update all minor dependencies chore: Rename test test: Add unit test for generic classname and namespace fix: Check if parentCommit is provided Split type from generic class name Condition of Parent Id Without Merge Commit Added Referenced the PmWiki's Cookbook recipe enabling MermaidJs schematics in wiki pages test(e2e): fix pie chart E2E tests for PR #4288 Add dummy commit to trigger GH checks chore: Revert unnecessary export refactor: Remove unnecessary calculations chore: Fix computeWidth function chore: Cleanup setupGraphViewbox Update docs update mermaidAPI to cleanup the text before passing to getDiagramFromText ...
sidharthv96
added a commit
to NicolasNewman/mermaid
that referenced
this pull request
Dec 6, 2023
* develop: (192 commits) build(deps-dev): bump vite from 4.4.9 to 4.4.12 Changes to .prettierignore 1. Added 'demos/dev/**' to be ignored by Prettier. 2. Added '!/demos/dev/example.html' so that Prettier ensures no one changes the example.html in a way that doesn't obey the Prettier code formatting rules. build: use `tsx` instead of `ts-node-esm` chore: Downgrade node to 18.18.2 fix: mermaid-js#5100 Add viewbox to sankey chore(deps): update all minor dependencies chore: Rename test test: Add unit test for generic classname and namespace fix: Check if parentCommit is provided Split type from generic class name Condition of Parent Id Without Merge Commit Added Referenced the PmWiki's Cookbook recipe enabling MermaidJs schematics in wiki pages test(e2e): fix pie chart E2E tests for PR mermaid-js#4288 Add dummy commit to trigger GH checks chore: Revert unnecessary export refactor: Remove unnecessary calculations chore: Fix computeWidth function chore: Cleanup setupGraphViewbox Update docs update mermaidAPI to cleanup the text before passing to getDiagramFromText ...
sidharthv96
added a commit
that referenced
this pull request
Dec 7, 2023
* next: (387 commits) build(deps-dev): bump vite from 4.4.9 to 4.4.12 Changes to .prettierignore 1. Added 'demos/dev/**' to be ignored by Prettier. 2. Added '!/demos/dev/example.html' so that Prettier ensures no one changes the example.html in a way that doesn't obey the Prettier code formatting rules. build: use `tsx` instead of `ts-node-esm` chore: Downgrade node to 18.18.2 fix: #5100 Add viewbox to sankey chore(deps): update all minor dependencies chore: Rename test test: Add unit test for generic classname and namespace fix: Check if parentCommit is provided Split type from generic class name Condition of Parent Id Without Merge Commit Added Referenced the PmWiki's Cookbook recipe enabling MermaidJs schematics in wiki pages test(e2e): fix pie chart E2E tests for PR #4288 Add dummy commit to trigger GH checks chore: Revert unnecessary export refactor: Remove unnecessary calculations chore: Fix computeWidth function chore: Cleanup setupGraphViewbox Update docs update mermaidAPI to cleanup the text before passing to getDiagramFromText ...
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.
Resolves #4082
Resolves #2073
Resolves #1862
https://mermaid.js.org/syntax/pie.html#example This example will cut off the legend part in small width devices like mobile.
After implementing this PR, the pie chart will be able to display legends on small width devices. Please see the included image below:
As someone new to SVG, I've only spent a few hours working on this issue. The most challenging aspect was getting the legend text's width before attaching it to the real DOM. I found a solution on Stack Overflow that involved using the canvas element. However, I didn't see the canvas element used in the Mermaid-js codebase, so it's possible that it might not work in nodejs/server-side implementations.I'm not entirely certain if I've completed everything correctly yet, and I haven't run any tests. I'm simply attempting to attract the attention of the maintainers.😅