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

Replacing "graph" with "flowchart" #654

Merged
merged 11 commits into from Feb 8, 2023
Merged

Conversation

victorp13
Copy link
Contributor

📑 Summary

Since the documentation mentions "flowchart" in the example pages, it makes more sense to also show a flowchart as the first example loaded in the editor. It looks to me like "graph" was deprecated? Not 100% sure.

Resolves #653

📏 Design Decisions

Describe the way your implementation works or what design decisions you made if applicable:

📋 Tasks

Make sure you

  • 📖 have read the contribution guidelines
  • 💻 have added unit/e2e tests (if appropriate)
  • 🔖 targeted develop branch

Since the documentation mentions "flowchart" in the example pages, it makes more sense to also show a flowchart as the first example loaded in the editor. It looks to me like "graph" was deprecated? Not 100% sure.
@netlify
Copy link

netlify bot commented Feb 16, 2022

Deploy Preview for mermaidjs ready!

Name Link
🔨 Latest commit 9853442
🔍 Latest deploy log https://app.netlify.com/sites/mermaidjs/deploys/63e29c9fe380f10008968a0e
😎 Deploy Preview https://deploy-preview-654--mermaidjs.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 settings.

@victorp13 victorp13 changed the title Replacing "chart" with "flowchart" Replacing "graph" with "flowchart" Feb 16, 2022
@sidharthv96
Copy link
Member

Thanks @victorp13 !
Can you also change the value here too?

@Yash-Singh1
Copy link
Member

The snapshot tests seem to be failing due to the changes in cypress/snapshots.js:

"1": "{\"code\":\"graph TD\\n A[Christmas] -->|Get money| B(Go shopping)\\n B --> C{Let me think}\\n C -->|One| D[Laptop]\\n C -->|Two| E[iPhone]\\n C -->|Three| F[fa:fa-car Car]\\n \",\"mermaid\":\"{\\n \\\"theme\\\": \\\"default\\\"\\n}\",\"updateEditor\":true,\"autoSync\":true,\"updateDiagram\":true}"
},
"Check Redirect from old URL": {
"1": "{\"code\":\"graph TD\\n A[Christmas] -->|Get money| B(Go shopping)\\n B --> C{Let me think}\\n C -->|One| D[Laptop]\\n C -->|Two| E[iPhone]\\n C -->|Three| F[fa:fa-car Car]\",\"mermaid\":\"{\\n \\\"theme\\\": \\\"default\\\"\\n}\",\"updateEditor\":false,\"autoSync\":true,\"updateDiagram\":true}"
},
"should load diagram from gist": {
"1": "{\"code\":\"graph TD\\n A[Party] -->|Get money| B(Go shopping!!)\\n \",\"mermaid\":\"{\\n \\\"theme\\\": \\\"forest\\\",\\n \\\"test\\\": \\\"hello world\\\"\\n}\",\"updateEditor\":false,\"autoSync\":true,\"updateDiagram\":true,\"loader\":{\"type\":\"gist\",\"config\":{\"url\":\"https://gist.github.com/sidharthv96/6268a23e673a533dcb198f241fd7012a\"}}}"
},
"should load diagram from gist revision": {
"1": "{\"code\":\"graph TD\\n A[Party] -->|Get money| B(Go shopping)\\n \",\"mermaid\":\"{\\n \\\"theme\\\": \\\"forest\\\",\\n \\\"test\\\": \\\"hello\\\"\\n}\",\"updateEditor\":false,\"autoSync\":true,\"updateDiagram\":true,\"loader\":{\"type\":\"gist\",\"config\":{\"url\":\"https://gist.github.com/sidharthv96/6268a23e673a533dcb198f241fd7012a/ec9b4ab0e41e4ff6287326cd3cb47affd7851e19\"}}}"
},
"should load diagram from raw files": {
"1": "{\"code\":\"graph TD\\n A[Party] -->|Get money| B(Go shopping!!)\\n \",\"mermaid\":\"{\\n \\\"theme\\\": \\\"forest\\\",\\n \\\"test\\\": \\\"hello world\\\"\\n}\",\"updateEditor\":false,\"autoSync\":true,\"updateDiagram\":true,\"loader\":{\"type\":\"files\",\"config\":{\"codeURL\":\"https://gist.githubusercontent.com/sidharthv96/6268a23e673a533dcb198f241fd7012a/raw/4eb03887e6a41397e80bdcdbf94017c498f8f1e2/code.mmd\",\"configURL\":\"https://gist.githubusercontent.com/sidharthv96/6268a23e673a533dcb198f241fd7012a/raw/4eb03887e6a41397e80bdcdbf94017c498f8f1e2/config.json\"}}}"
}
},
"__version": "9.3.1",
"Auto sync tests": {
"should dim diagram when code is edited": {
"1": "{\"code\":\"graph TD\\n A[Christmas] -->|Get money| B(Go shopping)\\n B --> C{Let me think}\\n C -->|One| D[Laptop]\\n C -->|Two| E[iPhone]\\n C -->|Three| F[fa:fa-car Car]\\n C --> Test\",\"mermaid\":\"{\\n \\\"theme\\\": \\\"default\\\"\\n}\",\"updateEditor\":false,\"autoSync\":false,\"updateDiagram\":false}"
},
"should not dim diagram when code is in sync": {
"1": "{\"code\":\"graph TD\\n A[Christmas] -->|Get money| B(Go shopping)\\n B --> C{Let me think}\\n C -->|One| D[Laptop]\\n C -->|Two| E[iPhone]\\n C -->|Three| F[fa:fa-car Car]\\n C --> Testing\",\"mermaid\":\"{\\n \\\"theme\\\": \\\"default\\\"\\n}\",\"updateEditor\":false,\"autoSync\":true,\"updateDiagram\":false}"

And in loadSite.spec.ts:

`{"code":"graph TD\\nA[\\"<img src='https://via.placeholder.com/64' width=64/>\\"]","mermaid":"{\\"securityLevel\\": \\"loose\\", \\"theme\\": \\"forest\\"}","updateEditor":true,"autoSync":true,"updateDiagram":true}`,

Could you update those too?

@sidharthv96
Copy link
Member

sidharthv96 commented Feb 22, 2022

Okay, so this is a valid bug when loading images

Works

graph TD
  A["<img src='https://picsum.photos/200' width=64/>"]

Does not work

flowchart TD
  A["<img src='https://picsum.photos/200' width=64/>"]

So there is compatibility difference with graph and flowchart.

This needs to be addressed in the mermaid library and is not directly linked to this PR.

@knsv @ashishjain0512 is this an intentional omission in flowchart?

@github-actions
Copy link

This pr is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 14 days

@github-actions github-actions bot added the Stale label May 26, 2022
@sidharthv96 sidharthv96 added retained Will not be auto-closed and removed Stale labels Jun 12, 2022
* origin/develop: (509 commits)
  chore(deps): update all non-major dependencies
  chore(deps): update dependency eslint to v8.33.0
  chore(deps): update all non-major dependencies
  chore(deps): update all non-major dependencies
  chore(deps): update all non-major dependencies
  chore(deps): update all non-major dependencies
  chore(deps): update dependency eslint to v8.30.0
  Fix links
  chore(deps): update all non-major dependencies
  SvelteKit 1.0!
  Update sveltekit
  Skip updatebrowserslit on renovate
  v9.3.0
  mermaid 9.3.0-rc.7
  Fix vite.config.js typing
  mermaid 9.3.0-rc.6
  rc.5
  fix: Adjust image size
  Update to rc.4
  Update to rc.3
  ...
@sidharthv96 sidharthv96 changed the base branch from master to develop February 7, 2023 18:38
@sidharthv96 sidharthv96 merged commit 9e25c2d into mermaid-js:develop Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
retained Will not be auto-closed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

First example uses "graph" instead of "flowchart"
3 participants