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

[core] Fix some DashboardLayout bugs and make some docs examples more consis… #3905

Merged
merged 12 commits into from
Aug 9, 2024

Conversation

apedroferreira
Copy link
Member

@apedroferreira apedroferreira commented Aug 7, 2024

Fixes the issues mentioned here #3893 (comment):

  • Fix scrollbar color not always being set when used in iframes with different color schemes (such as home page)
  • Fix some possible issues when the layout is used in iframes (it required adding some styles that in most cases would not be needed, though, with 100vw to set an explicit width)
  • Make some docs examples more consistent with other examples in the page they're in

https://deploy-preview-3905--mui-toolpad-docs.netlify.app/toolpad
https://deploy-preview-3905--mui-toolpad-docs.netlify.app/toolpad/core/react-dashboard-layout/

@apedroferreira apedroferreira added the bug 🐛 Something doesn't work label Aug 7, 2024
@apedroferreira apedroferreira self-assigned this Aug 7, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 8, 2024
import { AppProvider } from '@toolpad/core/AppProvider';
import { DashboardLayout } from '@toolpad/core/DashboardLayout';

const NAVIGATION = [
Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it's adding all this boilerplate here too but this way it matches the other examples...

<Toolbar />
<div>{children}</div>
{children}
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 guess we don't need a div here (unrelated though)

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 9, 2024
@apedroferreira apedroferreira marked this pull request as ready for review August 9, 2024 12:55
@apedroferreira apedroferreira requested a review from a team August 9, 2024 12:55
@Janpot Janpot mentioned this pull request Aug 9, 2024
6 tasks
component="main"
sx={{
flexGrow: 1,
minWidth: { xs: isMobileNavigationOpen ? '100vw' : 'auto', md: 'auto' },
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a comment to the github issue on mui core repository that tracks the issues with the responsive drawer demo.

Copy link
Member Author

Choose a reason for hiding this comment

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

Raised the issue in mui/material-ui#43244 and added todo comments in 5658dc0

Copy link
Member

@Janpot Janpot left a comment

Choose a reason for hiding this comment

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

Looks good. Just add a comment so that we remember what this hack is for and know when we can revert it.

@apedroferreira apedroferreira merged commit f47e822 into mui:master Aug 9, 2024
13 checks passed
@apedroferreira apedroferreira deleted the core-docs-fixes branch August 9, 2024 17:46
@oliviertassinari oliviertassinari added the component: layout This is the name of the generic UI component, not the React module! label Aug 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: layout This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants