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

[Stack] Set useFlexGap as default #28035

Open
oliviertassinari opened this issue Aug 29, 2021 · 5 comments
Open

[Stack] Set useFlexGap as default #28035

oliviertassinari opened this issue Aug 29, 2021 · 5 comments
Labels
breaking change component: Stack The React component. new feature New feature or request

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 29, 2021

Current Behavior 😯

When the Stack items overflow the container, they don't wrap.

Expected Behavior 🤔

When the Stack items overflow the container, they should wrap.

Steps to Reproduce 🕹

Steps:

  1. https://codesandbox.io/s/wispy-leaf-krlx2p?file=/demo.tsx

Screenshot 2021-08-29 at 23 09 32

Context 🔦

Most of the Stack components that we benchmarked against in #18158 don't flex wrap. My assumption is that they don't because the gap is usually implemented with a margin-left but this assumption might not be accurate.

We noticed a regression linked to this behavior in #27973 (comment). It's also why there are still locations in the demos where we use <Box sx={{ '& > :not(style)': { m: 1 } }}> over the simpler <Stack> component.

On Chakra-UI's side, they solved this problem by introducing an extra Wrap component.

For the solution of this issue, I would propose we update the Stack's implementation to wrap by default, and to use the negative margin strategy of the Grid to implement the gap. It seems to have been working well so far.

Would wrapping be the right thing to do by default? Figma's Stack doesn't wrap: https://www.figma.com/file/UW7KPuo4XXBsoqztC8iupE/Stack-wrap?node-id=0%3A1, so maybe this behavior should be optional. It's unclear to me. At the very least, the support for a flexWrap="nowrap" prop seems required.

Your Environment 🌎

v5.0.0-beta.5

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Sep 5, 2021

We could almost use flex gap if it wasn't for Safari iOS

We will be able to use it in v6.

@jonaspiela
Copy link

jonaspiela commented Oct 1, 2022

Works quite well on supported devices:

<Stack
  direction="row"
  spacing={0} /* optional */
  sx={{ flexWrap: 'wrap', gap: 1 }}
>
...
</Stack>

@ollemento

This comment was marked as resolved.

@oliviertassinari
Copy link
Member Author

An RFC for a built in solution: #33754

@oliviertassinari oliviertassinari changed the title [Stack] Items don't wrap correctly [Stack] Set useFlexGap as default Jul 1, 2023
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jul 1, 2023

Since #33754, and the work @siriwatknp did, the initial problem can now be solved with useFlexGap, see the docs https://mui.com/system/react-stack/#flexbox-gap. In practice:

import * as React from "react";
import Paper from "@mui/material/Paper";
import Stack from "@mui/material/Stack";
import { styled } from "@mui/material/styles";

const Item = styled(Paper)(({ theme }) => ({
  ...theme.typography.body2,
  padding: theme.spacing(1),
  textAlign: "center",
  color: theme.palette.text.secondary
}));

export default function DirectionStack() {
  return (
    <div style={{ width: 200, background: "red" }}>
      <Stack direction="row" spacing={2} useFlexGap sx={{ flexWrap: "wrap" }}>
        <Item>Item1</Item>
        <Item>Item2</Item>
        <Item>Item3</Item>
        <Item>Item4</Item>
      </Stack>
    </div>
  );
}
Screenshot 2023-07-01 at 13 17 28

https://codesandbox.io/s/funny-robinson-46s46z?file=/demo.tsx:0-658


We can use this issue to make useFlexGap={true} the default behavior. It's a breaking change.


Now, the problem with flex gap is that spacing can't take negative values, so we likely want to keep both implementations. I think that we are missing a prop-type warning here in the same line of thought so you know to disable flex gap and use margin instead:

export default function DirectionStack() {
  return (
    <div style={{ width: 200, background: "red" }}>
      <Stack spacing={-2} useFlexGap>
        <Item>Item1</Item>
        <Item>Item2</Item>
      </Stack>
    </div>
  );
}

https://codesandbox.io/s/compassionate-water-37rljr?file=/demo.tsx:438-530


One more thing, I think that we should remove the margin reset to reduce the difference of behavior between the flex gap and margin implementation. I can see cases where you need to margin on a different that isn't the one the stack is applied to, it's strange to reset it.

#37525 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: Stack The React component. new feature New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants