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.

Sign up for GitHub

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

Jump to bottom

[docs] Link Toolpad Core components from Material UI docs #43036

Merged
merged 32 commits into from
Aug 5, 2024
Merged

[docs] Link Toolpad Core components from Material UI docs #43036

merged 32 commits into from
Aug 5, 2024

Conversation

prakhargupta1
Copy link
Member

@prakhargupta1 prakhargupta1 commented Jul 23, 2024 •
edited
Loading

Preview of Toolpad Core on templates page: https://deploy-preview-43036--material-ui.netlify.app/material-ui/getting-started/templates/

To do for templates section

  • Add this demo to the templates section as well
  • Toolpad Core is a framework designed to build dashboards and internal tools. It leverages the existing suite of components offered by Material UI and ties them together to help you create applications quickly.
  • Add (beta) chip to this section

@prakhargupta1 prakhargupta1 added the docs Improvements or additions to the documentation label Jul 23, 2024
@mui-bot
Copy link

mui-bot commented Jul 23, 2024 •
edited
Loading

Netlify deploy preview

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 79cf8f6

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 25, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 25, 2024
@prakhargupta1
Copy link
Member Author

@Janpot For this issue I tried mentioning Toolpad components in the core docs in this PR. There are rendering issues that needs your inputs.

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

Janpot commented Jul 30, 2024 •
edited
Loading

@prakhargupta1 Fixed it but in latest weekly we decided to not add demos to these experimental sections. They could confuse users on the page. Users may not always be reading the text and just see a demo => copy + paste.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 31, 2024
@Janpot Janpot marked this pull request as ready for review August 2, 2024 07:13
@Janpot
Copy link
Member

Janpot commented Aug 2, 2024

I reworked copy a bit and removed the demos. I left one demo in as sort of a showcase. It doesn't have a preview to not give users the false impression it's part of Material UI. Just as happy to leave it out as well. wdyt?

@bharatkashyap
Copy link
Member

I reworked copy a bit and removed the demos. I left one demo in as sort of a showcase. It doesn't have a preview to not give users the false impression it's part of Material UI. Just as happy to leave it out as well. wdyt?

This form looks great to me; I would prefer having them on all components that we're touching, not just the Dialog and AppBar

@Janpot
Copy link
Member

Janpot commented Aug 2, 2024

I would prefer having them on all components that we're touching

We have it on appbar, drawer, container, breadcrumbs. Essentially the pages for components people would be on when build application layout. I'm not sure it would make sense to put on other component pages.

I don't think we can really properly showcase the imperative APIs without also showing code and I'd like to avoid that. Therefore I propose we leave Dialog/Snackbar as is.

@Janpot Janpot mentioned this pull request Aug 5, 2024
Closed
10 tasks
@prakhargupta1
Copy link
Member Author

prakhargupta1 commented Aug 5, 2024 •
edited
Loading

Couldn't find a reference of adding beta chip in .md files, so simple added (beta) to the title, like it was done here.

Screenshot 2024-08-05 at 8 03 07 PM

@prakhargupta1 prakhargupta1 requested a review from Janpot August 5, 2024 15:11
@prakhargupta1 prakhargupta1 merged commit cc5abcc into mui:next Aug 5, 2024
22 checks passed
@prakhargupta1 prakhargupta1 deleted the link-toolpad-componens-to-core-docs branch August 5, 2024 15:18
prakhargupta1 added a commit that referenced this pull request Aug 5, 2024
@prakhargupta1
Revert "[docs] Link Toolpad Core components from Material Ui docs (#4…
2903fc3 
…3036)"

This reverts commit cc5abcc.
oliviertassinari
oliviertassinari reviewed Aug 28, 2024
View reviewed changes
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool to have those 👍

docs/data/material/components/snackbars/snackbars.md

### Imperative API

You can create and manipulate notifications imperatively with the [`useNotifications`](https://mui.com/toolpad/core/react-use-notifications/) API in `@toolpad/core`. This API provides state management for opening and closing snackbars. It also allows for queueing multiple notifications at once.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This felt a bit hard to grasp. I could see a live demo like we have above for notistack.

Copy link
Member

@Janpot Janpot Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We intentionally avoided adding code snippets that weren't "@mui/material native" to not confuse users. And a demo without a snippet isn't as useful as the feature is in the API shape. Not sure this fully makes sense though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of having the equivalent to https://mui.com/material-ui/react-snackbar/#notistack.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Planning a refinement for the next iteration mui/toolpad#4007

docs/data/material/components/drawers/drawers.md
@@ -134,3 +134,11 @@ Apps focused on information consumption that use a left-to-right hierarchy.
Apps focused on productivity that require balance across the screen.

{{"demo": "ClippedDrawer.js", "iframe": true}}

## Experimental APIs
Copy link
Member

@oliviertassinari oliviertassinari Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Framing this as an experiment API feels wrong. I can understand why for the others components, these are things that should arguably be built-in features of the core components. But this one feels too high-level and opinionated to fit into Material UI product/brand.

@Janpot Janpot mentioned this pull request Aug 28, 2024
Closed
@oliviertassinari oliviertassinari changed the title [docs] Link Toolpad Core components from Material Ui docs [docs] Link Toolpad Core components from Material UI docs Dec 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oliviertassinari oliviertassinari oliviertassinari left review comments

@Janpot Janpot Janpot approved these changes

@bharatkashyap bharatkashyap bharatkashyap approved these changes

@mnajdova mnajdova Awaiting requested review from mnajdova

Assignees
No one assigned
Labels
docs Improvements or additions to the documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@prakhargupta1 @mui-bot @Janpot @bharatkashyap @oliviertassinari