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

[FEAT] ThemedLayoutV2 Control Over Children #6498

Open
aress31 opened this issue Nov 18, 2024 · 4 comments · May be fixed by #6502
Open

[FEAT] ThemedLayoutV2 Control Over Children #6498

aress31 opened this issue Nov 18, 2024 · 4 comments · May be fixed by #6502
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@aress31
Copy link

aress31 commented Nov 18, 2024

Is your feature request related to a problem? Please describe.

Right now it is not possible to control the children fully, for example background color and padding as children is already wrapped by a styled Box, see:

import React from "react";

import Box from "@mui/material/Box";

import { ThemedLayoutContextProvider } from "@contexts";
import { ThemedSiderV2 as DefaultSider } from "./sider";
import { ThemedHeaderV2 as DefaultHeader } from "./header";
import type { RefineThemedLayoutV2Props } from "./types";

export const ThemedLayoutV2: React.FC<RefineThemedLayoutV2Props> = ({
  Sider,
  Header,
  Title,
  Footer,
  OffLayoutArea,
  children,
  initialSiderCollapsed,
}) => {
  const SiderToRender = Sider ?? DefaultSider;
  const HeaderToRender = Header ?? DefaultHeader;

  return (
    <ThemedLayoutContextProvider initialSiderCollapsed={initialSiderCollapsed}>
      <Box display="flex" flexDirection="row">
        <SiderToRender Title={Title} />
        <Box
          sx={[
            {
              display: "flex",
              flexDirection: "column",
              flex: 1,
              minWidth: "1px",
              minHeight: "1px",
            },
          ]}
        >
          <HeaderToRender />
          <Box
            component="main"
            sx={{
              p: { xs: 1, md: 2, lg: 3 },
              flexGrow: 1,
              bgcolor: (theme) => theme.palette.background.default,
            }}
          >
            {children}
          </Box>
          {Footer && <Footer />}
        </Box>
        {OffLayoutArea && <OffLayoutArea />}
      </Box>
    </ThemedLayoutContextProvider>
  );
};

Describe alternatives you've considered

Provide full control over children rendering.

Additional context

The only thing to do is to delete the wrapping Box component.

Describe the thing to improve

See above.

@aress31 aress31 added the enhancement New feature or request label Nov 18, 2024
@aress31
Copy link
Author

aress31 commented Nov 18, 2024

Whilst at it might as well make the siderbar and header conditional rendering like the footer, this way there will be no need to do something like:

<ThemedLayoutV2 Header={CustomHeader} Sider={() => null}>

@alicanerdurmaz alicanerdurmaz added the good first issue Good for newcomers label Nov 18, 2024
@alicanerdurmaz
Copy link
Member

alicanerdurmaz commented Nov 18, 2024

Hello, @aress31, Nice catch! thanks.

I believe <ThemedLayoutV2 /> can take new props called childrenBoxProps and containerBoxProps (I'm fully open to better names) to customize these <Box /> components.

Do you want to work on this?

@aress31
Copy link
Author

aress31 commented Nov 18, 2024

Why not remove the Box entirely? If a user needs it, they can wrap the content like this:

<Box
    component="main"
    sx={{
        p: { xs: 1, md: 2, lg: 3 },
        flexGrow: 1,
        bgcolor: (theme) => theme.palette.background.default,
    }}
>

Feel free to assign it to me. For the PR, do you create the branch, or should I create it with the issue number?

@alicanerdurmaz
Copy link
Member

@aress31 You have a point but If we remove <Box /> right now it causes breaking changes. So, because of that, we can't remove it.

You can create the branch ofc and we have contributing guide. Feel free to ask any questions if you encounter any problems. 🚀

@aress31 aress31 linked a pull request Nov 18, 2024 that will close this issue
4 tasks
@alicanerdurmaz alicanerdurmaz linked a pull request Nov 20, 2024 that will close this issue
4 tasks
@BatuhanW BatuhanW added this to the December '24 Release milestone Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants