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

Add DashboardLayout component to @toolpad/core (old, PR has been split) #3343

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

Conversation

apedroferreira
Copy link
Member

@apedroferreira apedroferreira commented Mar 28, 2024

Docs/demo: https://deploy-preview-3343--mui-toolpad-docs.netlify.app/toolpad/core/components/dashboard-layout/

(once we have a navigation adapter we can make the demo sidebar links work with local state to provide better examples)
(API docs generation to be added next in separate PR)


Ready with component tests, supports branding with logo and name for header, and navigation with nested items in sidebar. Over time we can expand on this and add more features / improvements, of course.

Please check the types in packages/toolpad-core/src/AppProvider/AppProvider.tsx + component props (packages/toolpad-core/src/AppProvider/AppProvider.tsx and packages/toolpad-core/src/layout/DashboardLayout/DashboardLayout.tsx) for the chosen API.

For now, AppProvider can be imported as a named import from @toolpad/core/AppProvider and DashboardLayout the same way from @toolpad/core/DashboardLayout. They can also be imported directly from @toolpad/core along with some maybe useful types such as Branding and Navigation.

Made it so that pnpm dev can be run to build Toolpad core + watch changes, and then a new Next.js project in playground/toolpad-core-nextjs can be run to develop and see changes live. We can add other projects/frameworks to the playground folder later.

Added documentation page for the DashboardLayout component with demos. The demos can also be used for development but in some cases a real application is better, of course.

Also added simple contribution instructions for Toolpad Core.

@apedroferreira apedroferreira added the new feature New feature or request label Mar 28, 2024
@apedroferreira apedroferreira self-assigned this Mar 28, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 28, 2024
@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 Apr 4, 2024
@apedroferreira apedroferreira requested review from Janpot and a team May 8, 2024 16:30
@apedroferreira
Copy link
Member Author

Submitting for review again, CI is finally fixed.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label May 8, 2024
.npmrc Outdated Show resolved Hide resolved
@oliviertassinari
Copy link
Member

oliviertassinari commented May 10, 2024

It feels like some of the changes in this PR could be extracted to another one that isn't about the DashboardLayout component so that other people can work in parallel on other building blocks. Also, so we aim to never have a single PR open worked on for more than one week, continuously shipping.

packages/create-toolpad-app/src/generateProject.ts Outdated Show resolved Hide resolved
packages/toolpad-core/src/AppProvider/AppProvider.tsx Outdated Show resolved Hide resolved
packages/toolpad-core/.npmrc Outdated Show resolved Hide resolved
packages/toolpad-core/babel.config.js Outdated Show resolved Hide resolved
packages/toolpad-core/package.json Outdated Show resolved Hide resolved
@@ -14,7 +14,7 @@
"markdownlint": "markdownlint-cli2 \"**/*.md\"",
"prettier": "pretty-quick --ignore-path .eslintignore",
"prettier:all": "prettier --write . --ignore-path .eslintignore",
"dev": "dotenv cross-env FORCE_COLOR=1 lerna -- run dev --stream --parallel --ignore docs",
"dev": "dotenv cross-env FORCE_COLOR=1 lerna -- run dev --stream --parallel --ignore docs --ignore toolpad-core-nextjs",
Copy link
Member

Choose a reason for hiding this comment

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

We will move towards nx for orchestration over time

@Janpot
Copy link
Member

Janpot commented May 13, 2024

It feels like some of the changes in this PR could be extracted to another one that isn't about the DashboardLayout component so that other people can work in parallel on other building blocks.

Agreed. We could make the goal of the PR to set up the bare minimum of the basic layout component and provider so that we can test/prove the project setup works. Perhaps we should strip the configuration options from these components and concentrate on nailing the project setup so we can get this trough?
We can then concentrate on API in follow up PR.

@apedroferreira
Copy link
Member Author

It feels like some of the changes in this PR could be extracted to another one that isn't about the DashboardLayout component so that other people can work in parallel on other building blocks.

Agreed. We could make the goal of the PR to set up the bare minimum of the basic layout component and provider so that we can test/prove the project setup works. Perhaps we should strip the configuration options from these components and concentrate on nailing the project setup so we can get this trough? We can then concentrate on API in follow up PR.

The current state just evolved naturally from review feedback + improving the build to fix some bugs with the component itself. I can probably split project setup from the component at this point though, at least, will see what else I can split.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label May 14, 2024
@apedroferreira apedroferreira changed the title Add DashboardLayout component to @toolpad/core Add DashboardLayout component to @toolpad/core (old, PR has been split) May 14, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label May 16, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

None yet

4 participants