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

[docs][toolpad] Remove extra code block #44120

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

bharatkashyap
Copy link
Member

@bharatkashyap bharatkashyap commented Oct 16, 2024

@bharatkashyap bharatkashyap added the docs Improvements or additions to the documentation label Oct 16, 2024
@mui-bot
Copy link

mui-bot commented Oct 16, 2024

Netlify deploy preview

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against cf51ed7

@mnajdova mnajdova changed the title [docs] [toolpad] Remove extra code block [docs][toolpad] Remove extra code block Oct 16, 2024
@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 16, 2024

The demo is quite hard to follow, I thought the point was that this code could be simpler and shown before the demo.

@prakhargupta1
Copy link
Member

prakhargupta1 commented Oct 16, 2024

and shown before the demo.

or, as a preview? Like we show in most demos?

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 14, 2024
@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 17, 2024

or, as a preview? Like we show in most demos?

@prakhargupta1 I don't understand this, right now in https://deploy-preview-44120--material-ui.netlify.app/material-ui/react-dialog/#toolpad-beta, we have noise:

SCR-20241117-qbos

and when we open the demo, we have to struggle to find the piece that I want to copy and paste in my project.


I think that the right move is to:

  1. Keep the simplification to the demo done in this PR.
  2. Not show the inline preview, there is nothing to show AFAIK (like in HEAD).
  3. Update the code example of https://mui.com/material-ui/react-dialog/#toolpad-beta to match with 1.
  4. Move the code example to be before the demo as the demo is only for first-time learning, once you learned the feature, and you repeatedly come to the docs, you need this bloc to copy and paste.
  5. So also add the missing pieces to the code example to make it self-sustained (e.g. imports), but while keeping as simple as possible, simpler than the demo

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 18, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 18, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 19, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 20, 2024
@siriwatknp
Copy link
Member

@bharatkashyap Do you mind changing the <Button loading> back to <LoadingButton>? it will be added in v7 instead. https://mui-org.slack.com/archives/C02BDJNTKF1/p1732593650637169

@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation PR: out-of-date The pull request has merge conflicts and can't be merged Toolpad
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants