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] Improve Core tutorial #3874

Merged
merged 6 commits into from
Aug 5, 2024

Conversation

bharatkashyap
Copy link
Member

  • Bring the core-tutorial example in sync with the docs page
  • Improve the structure of core-tutorial to make the tutorial work better, for instance: Add PageContainer to dashboard/layout instead of dashboard/page/layout
  • Miscellaneous small improvements to the tutorial docs page
  • Also add a NavigateButton to the example to improve interactivity on the first button in dev mode:
Before After
before.mov
after.mov

@bharatkashyap bharatkashyap added the docs Improvements or additions to the documentation label Aug 1, 2024
@Janpot
Copy link
Member

Janpot commented Aug 1, 2024

Improve the structure of core-tutorial to make the tutorial work better, for instance: Add PageContainer to dashboard/layout instead of dashboard/page/layout

I did this intentionally, to show how the composability works together with app router layouts

import { LoadingButton } from '@mui/lab';
import { Box } from '@mui/material';

export default function NavigateButton() {
Copy link
Member

@apedroferreira apedroferreira Aug 1, 2024

Choose a reason for hiding this comment

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

I think it doesn't make sense to make this a separate file/component? Seems too specific, better for it to be in the intro page(s) directly?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a client component, so – I could be wrong – it must be in its own file with "use client" at the top

Copy link
Member

Choose a reason for hiding this comment

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

I think you can have 'use client' as the first line inside the component?

But if in terms of file structure you think this makes more sense anyway it's not a big deal!

title: 'Page',
icon: <DashboardIcon />,
},
];

export default function RootLayout({ children }: Readonly<{ children: React.ReactNode }>) {
return (
<html lang="en">
<html lang="en" data-toolpad-color-scheme="light">
Copy link
Member

Choose a reason for hiding this comment

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

Matching the playground should be good here!

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the playground app also contains the same attribute in the app/layout.tsx file, isn'it? That's what I can see here: https://github.com/mui/mui-toolpad/blob/master/playground/nextjs/src/app/layout.tsx#L27

Copy link
Member

Choose a reason for hiding this comment

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

Yep that's what I meant, was just confirming that it seems good!

@bharatkashyap
Copy link
Member Author

Improve the structure of core-tutorial to make the tutorial work better, for instance: Add PageContainer to dashboard/layout instead of dashboard/page/layout

I did this intentionally, to show how the composability works together with app router layouts

The issue with this was that adding a second page required adding a layout.tsx as well to the page folder; the breadcrumbs and title would not automatically appear to the newly created page. (This is what happened for me, perhaps I missed a step while following the original tutorial)

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

Janpot commented Aug 2, 2024

The issue with this was that adding a second page required adding a layout.tsx as well to the page folder; the breadcrumbs and title would not automatically appear to the newly created page. (This is what happened for me, perhaps I missed a step while following the original tutorial)

Usually, the features of the CLI should drive the tutorial content, not the other way around. But I'm not heavily against the change, we can go with it for now.

@bharatkashyap bharatkashyap merged commit 89623f2 into mui:master Aug 5, 2024
14 checks passed
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants