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

[Dialog] DialogActions spacing breaks when elements aren't the same type #36947

Closed
2 tasks done
mikew opened this issue Apr 20, 2023 · 9 comments · Fixed by #39189
Closed
2 tasks done

[Dialog] DialogActions spacing breaks when elements aren't the same type #36947

mikew opened this issue Apr 20, 2023 · 9 comments · Fixed by #39189
Assignees
Labels
bug 🐛 Something doesn't work component: dialog This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material

Comments

@mikew
Copy link
Contributor

mikew commented Apr 20, 2023

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Steps to reproduce 🕹

Link to live example: https://codesandbox.io/s/pensive-ride-4ntm36?file=/demo.tsx

Steps:

  1. Add DialogActions
  2. Add a <Button>
  3. Add a <Button component="a">

Current behavior 😯

Since the CSS selector is :first-of-type, if the elements have a different type, spacing between them won't work.

Expected behavior 🤔

There would be spacing between the actions in a DialogActions component

Context 🔦

One of our buttons is a "Close" button, the other is a link to download an asset. Since it's a link, it should be an a tag, not some random onClick handler.

Your environment 🌎

npx @mui/envinfo
  System:
    OS: Linux 6.2 Arch Linux
  Binaries:
    Node: 12.14.0 - [redacted]/v12.14.0/bin/node
    Yarn: 1.22.18 - [redacted]/yarn
    npm: 6.13.4 - [redacted]/node_modules/.bin/npm
  Browsers:
    Chrome: Not Found
    Firefox: Not Found
  npmPackages:
    @emotion/react: ^11.10.6 => 11.10.6 
    @emotion/styled: ^11.10.6 => 11.10.6 
    @mui/base:  5.0.0-alpha.125 
    @mui/core-downloads-tracker:  5.12.0 
    @mui/icons-material: ^5.11.16 => 5.11.16 
    @mui/material: ^5.12.0 => 5.12.0 
    @mui/private-theming:  5.12.0 
    @mui/styled-engine:  5.12.0 
    @mui/system:  5.12.0 
    @mui/types:  7.2.4 
    @mui/utils:  5.12.0 
    @types/react: ^17.0.3 => 17.0.3 
    react: ^17.0.0 => 17.0.2 
    react-dom: ^17.0.0 => 17.0.2 
    styled-components: ^5.0.1 => 5.0.1 
    typescript: ^4.5.4 => 4.5.5 ```
</details>
@mikew mikew added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Apr 20, 2023
@mj12albert mj12albert added the component: dialog This is the name of the generic UI component, not the React module! label Apr 21, 2023
@mj12albert
Copy link
Member

@mikew Thanks for reporting this - we can only use :first-of-type since :first-child isn't safe for SSR

In your case you could manually reapply the spacing using a different selector, example: https://codesandbox.io/s/dialog-actions-spacing-v0iru2?file=/src/App.tsx

@mj12albert mj12albert removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Apr 21, 2023
@mj12albert mj12albert changed the title DialogActions spacing breaks when elements aren't the same type [Dialog] DialogActions spacing breaks when elements aren't the same type Apr 21, 2023
@mikew
Copy link
Contributor Author

mikew commented Apr 21, 2023

dang, I kinda can't wait until we can move past css-in-js 😅 . Thanks for the tip!

@whiteae8 whiteae8 mentioned this issue Apr 24, 2023
1 task
@ZeeshanTamboli
Copy link
Member

I think we've found a solution mentioned here, so we can close this issue.

@mycroes
Copy link

mycroes commented Aug 30, 2023

I think we've found a solution mentioned here, so we can close this issue.

There's no solution, there's a workaround. I propose to at least reopen this issue so the fact it's broken can be tracked.

@ZeeshanTamboli ZeeshanTamboli added bug 🐛 Something doesn't work package: material-ui Specific to @mui/material labels Aug 31, 2023
@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented Aug 31, 2023

I apologize for closing this issue . This is definitely a workaround so I reopened the issue.

One approach might involve looping through the children, using React context or other API to find their positions, and applying styling similar to what's being done for ButtonGroup in #38520. But DialogActions may not contain only Material UI Buttons, but other elements as well so this would not be ideal.

@mycroes
Copy link

mycroes commented Aug 31, 2023

I do think that the solution would work. Basically you only need to add a data attribute to the first element and change the selector to :not(data-first-dialog-action-child).

I'm unaware of a solution to cloning/detecting non-rendering elements (<Tooltip />, <Dialog />), but I guess that's not new for the MUI team?

@mikew
Copy link
Contributor Author

mikew commented Aug 31, 2023

To work around this, we just wrapped each element in a <span> or <div>. Can't remember which, just as long as it's the same element for each

@JulianIV
Copy link

JulianIV commented Sep 4, 2023

Is there a reason why gap: 8px won't work?

@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented Sep 8, 2023

Is there a reason why gap: 8px won't work?

Because of browser support for flexbox layout's gap property. See #35104 (comment). This could work in v6 when we update Safari browser support. 👍

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: dialog This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants