material-ui icon indicating copy to clipboard operation
material-ui copied to clipboard

[material-ui][Button] Negative margin causes start icon and end icon to be outside parent container

Open JaceInglis opened this issue 1 year ago • 6 comments

Steps to reproduce

Steps:

  1. Visit the sandbox reproduction
  2. Open dev tools (Inspect Element) and notice that when you hover over the button that the startIcon is outside the button container
  3. Also notice how when there is no padding on the button the Icon will get cutoff due to the negative margin

Current behavior

Current behaviour: Button Icons fall outside of the parent container due to negative margin.

Expected behavior

Expected behavior: Button Icons stay inside their parent container.

Context

I want to be able to have no padding on my button and still have the content stay inside the components container.

Issue created for: https://github.com/mui/material-ui/pull/40956#pullrequestreview-1869925897

Link to PR: https://github.com/mui/material-ui/pull/40956

Your environment

Using Chrome

npx @mui/envinfo
  System:
    OS: Linux 6.1 Debian GNU/Linux 12 (bookworm) 12 (bookworm)
  Binaries:
    Node: 20.9.0 - /usr/local/bin/node
    npm: 9.8.1 - /usr/local/bin/npm
    pnpm: 8.10.2 - /usr/local/share/npm-global/bin/pnpm
  Browsers:
    Chrome: Not Found
  npmPackages:
    react: 16.8.6 => 16.8.6 
    react-dom: 16.8.6 => 16.8.6 
    typescript: 3.3.3 => 3.3.3 

Search keywords: button negative margin start end icon

JaceInglis avatar Feb 14 '24 23:02 JaceInglis

Hey @JaceInglis, thanks for opening the issue! For the sake of curiosity, do you mind elaborating more on your use case? Why do you want to remove the button's padding entirely?

danilo-leal avatar Feb 15 '24 03:02 danilo-leal

Hey @danilo-leal I want to be able to have a full text button that aligns with the rest of my text other wise text buttons look like they are missaligned due to there internal padding.

Without removed padding image

With removed padding (issue can be seen here) image

Expected (If issue of Icon falling outside of parent container is fixed) image

JaceInglis avatar Feb 15 '24 17:02 JaceInglis

I'd generally advise against messing with spacing from inside the component — maybe a different approach would work for you, and the Card's page introduction demo seems to be pretty close to your use case. Take a look there and let me know if something similar works for you!

danilo-leal avatar Feb 15 '24 18:02 danilo-leal

Hey @danilo-leal the component above doesn't utilize the card component. Putting the removal of the padding aside, the icon within the button still visibly extends beyond the parent container. Could you share your perspective on why addressing this isn't preferred? If the concern is the slight spacing adjustment between the icon and text, perhaps we could consider maintaining the spacing by adding an equivalent amount on the opposite side of the icon. Your insights on this matter would be appreciated.

JaceInglis avatar Feb 15 '24 19:02 JaceInglis

The biggest downside that I see of affecting the inner spacing of the Button component is messing with the style of the different states, primarily the hover and focus states. The demo I linked above is just an illustration of how you can fix the problem by wrapping the content and the button in containers with different padding instead of tweaking the components' inner spacing—no need to necessarily be a Card; you can do it with a simple Box!

Check this out: I put together a quick CodeSandbox using just the Box component and padding to illustrate everything I said above! Let me know if it's helpful!

danilo-leal avatar Feb 15 '24 21:02 danilo-leal

@danilo-leal Your sandbox reproduction uses button size small which actually fixes the issue. image

If you try using medium you will not be able to make a working example without

A. the icon falling outside the line or B. using small pixel adjustments to fix the deeper underlying issue in the button component

For clarification this is the issue I'm trying to solve. image image image

JaceInglis avatar Feb 16 '24 00:02 JaceInglis

@danilo-leal Just making sure you saw this ^

JaceInglis avatar Mar 01 '24 17:03 JaceInglis