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: interactive examples #5330

Closed
wants to merge 3 commits into from
Closed

feat: interactive examples #5330

wants to merge 3 commits into from

Conversation

nalgeon
Copy link
Contributor

@nalgeon nalgeon commented Feb 26, 2024

📑 Summary

I've converted static code examples to interactive.

Resolves #5311

📏 Design Decisions

The output is hidden by default, until the reader runs an example. This is intentional. My observations show that hiding the output provides a better reader experience for two reasons:

  1. The page has less content and is perceived as easier to read, increasing the chances that people will actually read it.

  2. Interacting with the article by clicking Run to see the results provides a small sense of accomplishment. This increases reader engagement.

If you want the output to be be shown by default, I can do that. But please try it yourself — maybe you'll see the benefits I'm talking about.

Regarding live reload (automatic rendering as people type). I've tried mermaid.live and found it quite confusing. As I change the code, the diagram specification is often invalid, so live reload shows lots of errors.

I think it's better to reload the diagram only when the reader explicitly asks for it. It's still easy to do this with the Ctrl+Enter (Cmd+Enter) shortcut while editing the code. I've added a note about this shortcut in the bottom right corner of the editor.

Copy link

netlify bot commented Feb 26, 2024

Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit 156e6c9
🔍 Latest deploy log https://app.netlify.com/sites/mermaid-js/deploys/65dffd389df879000701a33a
😎 Deploy Preview https://deploy-preview-5330--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.

Copy link

codecov bot commented Feb 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 43.37%. Comparing base (1857eb1) to head (156e6c9).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##           develop    #5330       +/-   ##
============================================
- Coverage    76.47%   43.37%   -33.11%     
============================================
  Files          160       23      -137     
  Lines        14014     5130     -8884     
  Branches       820       23      -797     
============================================
- Hits         10717     2225     -8492     
+ Misses        3118     2904      -214     
+ Partials       179        1      -178     
Flag Coverage Δ
e2e ?
unit 43.37% <ø> (ø)

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

see 156 files with indirect coverage changes

@nalgeon
Copy link
Contributor Author

nalgeon commented Feb 26, 2024

The build fails, but the error does not seem to be related to my changes in any way:

git clone --single-branch https://github.com/mermaid-js/mermaid-live-editor.git
cd mermaid-live-editor
npm install
npm ERR! Cannot read properties of null (reading 'edgesOut')

Tried to build mermaid-live-editor locally, got the same error:

131 verbose stack TypeError: Cannot read properties of null (reading 'edgesOut')
131 verbose stack     at #loadPeerSet (/opt/homebrew/Cellar/node@18/18.18.2/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js:1302:38)
131 verbose stack     at async #loadPeerSet (/opt/homebrew/Cellar/node@18/18.18.2/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js:1321:23)
131 verbose stack     at async #buildDepStep (/opt/homebrew/Cellar/node@18/18.18.2/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js:917:11)
131 verbose stack     at async Arborist.buildIdealTree (/opt/homebrew/Cellar/node@18/18.18.2/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js:196:7)
131 verbose stack     at async Promise.all (index 1)
131 verbose stack     at async Arborist.reify (/opt/homebrew/Cellar/node@18/18.18.2/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/reify.js:159:5)
131 verbose stack     at async Install.exec (/opt/homebrew/Cellar/node@18/18.18.2/lib/node_modules/npm/lib/commands/install.js:149:5)
131 verbose stack     at async module.exports (/opt/homebrew/Cellar/node@18/18.18.2/lib/node_modules/npm/lib/cli-entry.js:61:5)

@nalgeon
Copy link
Contributor Author

nalgeon commented Feb 29, 2024

@sidharthv96 any comments?

@sidharthv96
Copy link
Member

@nalgeon, I don't see any major blockers, will try it out locally once I get 10.9.0 out. A bit busy with that now.

@nalgeon
Copy link
Contributor Author

nalgeon commented Mar 10, 2024

@sidharthv96 10.9.0 is out, so could you please take a look at this PR?

@sidharthv96
Copy link
Member

sidharthv96 commented Mar 11, 2024

image image

When comparing these two pages, the second one conveys a lot more information to the user who is reading the documentation site, they can look at the diagrams and then look at the code for what they require.

Interacting with the article by clicking Run to see the results provides a small sense of accomplishment. This increases reader engagement.

For a documentation site, the time it takes for a user to get the answer they need is far more important than enagement.
Now a user needs to a) read all the headings carefully, b) Click run and wait for diagrams to render to see if it's what they want. Earlier, all the user needed to do was scroll and look at the diagrams, and then read the code when they see what they wanted. Having to click run every single time will be very frustrating for users, especially when they want to check something out quickly.

The page has less content and is perceived as easier to read, increasing the chances that people will actually read it.

We don't want the user to read everything, they should only need to read things that they care about. But now, they don't know what they need easily as the diagrams are hidden. So they are forced to read everything.


I understand this is a marketing PR, but Done by Codapi for every single diagram feels very "in your face".
Also, the link opens in the same tab, taking the user away from the documentation.

image

Nit: Ctrl + Enter is not cross platform, so it might be good to detect the OS and show the correct keys.


I was looking at codapi documentation and really loved the idea, especially for building tutorials with snippets.
I also loved the idea of the docs being runnable in place.
But, in our case, I feel that codapi is adding a lot of complexity for what can be achieved by a custom vue component which calls mermaid.render on a button click.

@nalgeon
Copy link
Contributor Author

nalgeon commented Mar 11, 2024

I feel that codapi is adding a lot of complexity for what can be achieved by a custom vue component which calls mermaid.render on a button click.

I wish you had told me this before I spent a full day preparing the PR. I opened issue #5311 with a Mermaid demo specifically for this purpose, so you can decide if the suggested approach fits your project or not.

@nalgeon nalgeon closed this Mar 11, 2024
sidharthv96 added a commit that referenced this pull request Mar 11, 2024
Ctrl/Cmd + Enter and a run button is added.

The feature was first implemented in #5330.

This is a simplified version without introducing additional dependencies.

Co-authored-by: Anton <m@antonz.org>
sidharthv96 added a commit that referenced this pull request Mar 11, 2024
Ctrl/Cmd + Enter and a run button is added.

The feature was first implemented in #5330.

This is a simplified version without introducing additional dependencies.

Co-authored-by: Anton <m@antonz.org>
sidharthv96 added a commit that referenced this pull request Mar 11, 2024
Ctrl/Cmd + Enter and a run button is added.

The feature was first implemented in #5330.

This is a simplified version without introducing additional dependencies.

Co-authored-by: Anton <m@antonz.org>
@sidharthv96
Copy link
Member

I wish you had told me this before I spent a full day preparing the PR. I opened issue #5311 with a Mermaid demo specifically for this purpose, so you can decide if the suggested approach fits your project or not.

The idea of having interactive documentation was really great.
The only information I had when #5311 was raised was the code here and the demo site.

I had pointed out in #5311 (comment) itself that we need the examples to run without clicking a button.

It was only after you raised this PR that I could see the actual implementation, and the complexity it would add.
The amount of extra CSS and JS, without considering the external dependencies, felt like it was more than essential.

But, as the idea was great, I tried to see how much work it would be to add the same feature without a dependency.
Which was ~40 lines in #5376. Had it been too much, we could've easily justified the use of codapi and merged this PR. I have added you as a co-author in #5376.

I hope you understand the rationale behind the decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Interactive documentation
2 participants