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

Allow PageContainer Children to Fully Utilize Viewport Height #4324

Open
aress31 opened this issue Oct 29, 2024 · 8 comments
Open

Allow PageContainer Children to Fully Utilize Viewport Height #4324

aress31 opened this issue Oct 29, 2024 · 8 comments
Assignees

Comments

@aress31
Copy link

aress31 commented Oct 29, 2024

Summary

It appears that the children of a PageContainer cannot fully utilize the entire height of the visible viewport (i.e., 100% height). It would be beneficial to either add a prop or redesign the component to enable this functionality. If achieving this with the sx prop is already possible, please let me know, and consider documenting this feature.

Examples

    <DashboardLayout defaultSidebarCollapsed>
      <PageContainer
        slots={{ toolbar: PageToolbar }}
        fixed
        disableGutters
        sx={{ minWidth: "100%", bgcolor: "cyan" }}
      >
        <Box
          sx={{
            bgcolor: "purple",
            height: "100%",
          }}
        >
          Test
        </Box>
      </PageContainer>
    </DashboardLayout>

Motivation

This design is commonly found in web applications.

Search keywords: full height

@aress31 aress31 added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Oct 29, 2024
@Janpot
Copy link
Member

Janpot commented Oct 29, 2024

PageContainer should behave as a Container component, but with a header. I believe the original Container could be made to behave this way with an sx prop?

Are you interested in contributing a fix?

@apedroferreira
Copy link
Member

apedroferreira commented Oct 29, 2024

Yes, this is possible to do directly inside the DashboardLayout as shown in https://mui.com/toolpad/core/react-dashboard-layout/#full-size-content, but does not seem to work yet inside the PageContainer component.

We probably need to add styles to the PageContainer component in https://github.com/mui/toolpad/blob/master/packages/toolpad-core/src/PageContainer/PageContainer.tsx so that when a component directly inside has flex: 1 or height: 100% it takes up the whole space, just like in that example in the docs.

But an sx prop in PageContainer could also be useful, we do have one already for DashboardLayout.

Feel free to try to create a PR, or we can create one later!

@apedroferreira apedroferreira added scope: toolpad-core Abbreviated to "core" component: PageContainer and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Oct 29, 2024
@SamuelLeapifai
Copy link

I would also like to vote for this feature. I looked into the source code - the PageContainer renders child Stacks, divs and a parent Container, so even if we modify the PageContainer with sx, this is not passed down to those children. Those child divs won't inherit height 100%, so my own component's height 100% is ignored. It would be great to either add this as a prop to PageContainer or have a separate FullHeightPageContainer component with min-height set to 100% or 100vh etc.

@Janpot
Copy link
Member

Janpot commented Nov 8, 2024

I think where we potentially could go with this is to extract a PageHeader component out of what is now PageContainer and let users wrap it themselves (or not) with a Material UI Container. Then deprecate that PageContainer altogether.

Would that make sense?

@aress31
Copy link
Author

aress31 commented Nov 8, 2024

@Janpot that would still introduce an issue of having to calculate the PageHeader height and subtract it to 100vh to achieve a full height layout.

@SamuelLeapifai
Copy link

I think where we potentially could go with this is to extract a PageHeader component out of what is now PageContainer and let users wrap it themselves (or not) with a Material UI Container. Then deprecate that PageContainer altogether.

Would that make sense?

Or keep it and just export the PageHeader. I think PageContainer "as is" is great and simple. Has nice defaults. It's just not overridable enough - in that case would be better to pick and choose only what we need (the whole header with title and breadcrumbs, in our case). We have around 10 pages so far, and only 3 pages are "full screen". The rest is using PageContainer as is and we like it.

@SamuelLeapifai
Copy link

@Janpot that would still introduce an issue of having to calculate the PageHeader height and subtract it to 100vh to achieve a full height layout.

would it? You should still be able to write your own container, give it 100% height, and put header with one more div into it (this div also has 100% height), no?

@Janpot
Copy link
Member

Janpot commented Nov 8, 2024

Should be straightforward with a flex layout. In any case, we wouldn't make this change without a demo of this use-case in the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Development

No branches or pull requests

5 participants