mui / material-ui

Materialย UI: Ready-to-use foundational React components, free forever. It includes Material UI, which implements Google's Material Design.
https://mui.com/material-ui/
MIT License
92.4k stars 31.82k forks source link

Accordion: disable separator #37019

Open thany opened 1 year ago

thany commented 1 year ago

Duplicates

Latest version

Summary ๐Ÿ’ก

I'd like a prop to disable the separator between accordions. Right now there exists a grey line between each two accordions that I haven't asked for. It's too late now to only render them when a hypothetical enableSeparators prop is included, so we could now provide a disableSeparators prop instead.

Only workaround atm is to set content: none via a styleOverrides, which always feels kind of hacky. I mean, what if MUI in the future decides to draw this line using any other line drawing method? Then this override will no longer have any effect.

Examples ๐ŸŒˆ

These lines I'd like to remove: image

This has already applied default props in the theme:

MuiAccordion: {
  defaultProps: {
    disableGutters: true,
    square: true,
    elevation: 0,
  },
}

Now the line above an expanded accordion disappears because it is moved to top: -1px which is weird on its own, but I'm sure there's a reason behind doing it that way. More importantly, I want them all gone, all the time.

Motivation ๐Ÿ”ฆ

Motivation is, well, design I guess? Our designer wants them gone, that's all ๐Ÿ˜€

siriwatknp commented 1 year ago

@thany Thanks for sharing! Could you put it in a CodeSandbox? I can propose a workaround to remove that separator.

thany commented 1 year ago

I already have a workaround, so I'd like to work towards a proper solution ๐Ÿ™‚

Anyway, this sandbox perfectly reproduces the problem: https://codesandbox.io/s/hopeful-breeze-foc827

Vansh-Baghel commented 1 year ago

@siriwatknp Can I work on this issue if its ok? Where will I find CSS file? As in BasicAccordion.js, I see the components are imported from @mui/material/

siriwatknp commented 1 year ago

@thany From the doc, it says "This component is no longer documented in the Material Design guidelines, but Material UI will continue to support it.", so I think we can add the prop disableSeparator to the component.

@Vansh-Baghel If you like to work on this, you can take a look at packages/mui-material/src/Accordion/Accordion.js.

Vansh-Baghel commented 1 year ago

@siriwatknp I am having tough time in understanding that how the styles are assigned. I added disableSeparator in Accordion.js (under ownerState and props variable), but couldn't find the file to trigger it and make changes with its css. If its possible, could you please guide me that through which files the changes must be done to change the CSS w.r.t. the props?

Also added this in Accordion.d.ts, hope its needed:

  disableSeparator?: boolean;
  /**
   * If `true`, it will remove the margin lines from accordion, else will keep it.
   */
divinedouggy commented 2 months ago

How interesting that I'm looking this issue up, and find this Issue a year later to the day it was opened! Needless to say, it's still an issue. The "content: none" doesn't seem to work anymore, unless I did something wrong. But this worked a breeze for me... put this as a prop on your Accordion:

    sx={{
        '&:before': {
            display: 'none',
        }
    }}

I realize this is just another workaround and doesn't solve the issue... but this is for anybody looking this up on a search engine and finding this thread like I did.

clefayomide commented 2 weeks ago

How interesting that I'm looking this issue up, and find this Issue a year later to the day it was opened! Needless to say, it's still an issue. The "content: none" doesn't seem to work anymore, unless I did something wrong. But this worked a breeze for me... put this as a prop on your Accordion:

    sx={{
        '&:before': {
            display: 'none',
        }
    }}

I realize this is just another workaround and doesn't solve the issue... but this is for anybody looking this up on a search engine and finding this thread like I did.

you are a life saver!

divinedouggy commented 2 weeks ago

you are a life saver!

That made my day!

thany commented 1 week ago

@divinedouggy

How interesting that I'm looking this issue up, and find this Issue a year later to the day it was opened! Needless to say, it's still an issue. The "content: none" doesn't seem to work anymore, unless I did something wrong. But this worked a breeze for me... put this as a prop on your Accordion:

My reasoning for requesting a proper fix from the framework devs, is exactly the point you're making:

I realize this is just another workaround and doesn't solve the issue... but this is for anybody looking this up on a search engine and finding this thread like I did.

In other words, any workaround folks come up with, might break in a future update. It's the very reason I prefer not to customise a UI framework beyond its supported configuration if I can help it. Throwing arbitrary CSS at it, may work now, but nobody knows for how long.

To be clear though, this is not to say your workaround isn't appreciated ๐Ÿ˜Š