[Button] Focus effect padding

[wilav-dev]

Duplicates

• I have searched the existing issues

Latest version

• I have tested the latest version

Current behavior 😯

The focus effect does not cover the hole button

Expected behavior 🤔

Focus must be full width

Steps to reproduce 🕹

Steps:

1. Add a button
2. Focus by using the tab key

Context 🔦

No response

Your environment 🌎

No response

[mlgrn]

I was able to reproduce this problem here: https://stackblitz.com/edit/react-wc86qa?file=src/App.js

[mlgrn]

not sure what the maintainers think is the best approach here, but maybe adding a prop for longer buttons to the ButtonBase since this is only going to be in certain cases? went through the Ripple.js file to see where the styling originates from

[michaldudak]

@oliviertassinari could you shed some light on why this was implemented this way? I haven't found any mentions of circular focus indicators in Material Design docs.

[michaldudak]

Let's just fix it. @mlgrn would you like to take a shot at it?

[alisasanib]

Hi @michaldudak, may I work on this issue? I think the source of this bug is in this line of code, and I couldn't understand the logic behind it:

material-ui/packages/mui-material/src/ButtonBase/TouchRipple.js

Line 228 in adda22c

rippleSize = Math.sqrt((2 * rect.width ** 2 + rect.height ** 2) / 3);

Changing it to rippleSize = Math.max(rect.width, rect.height) will solve this issue.
Do you confirm this proposal?

[oliviertassinari]

Could you shed some light on why this was implemented this way? I haven't found any mentions of circular focus indicators in Material Design docs.

@michaldudak This was implemented to meet the MD v1 spec: https://material.io/archive/guidelines/components/buttons.html#buttons-flat-buttons

Now, looking at the MD v2 and v3. spec, I believe that we should remove the ripple on focus. It should be a simple background change. This would have the added benefit to make Material UI easier to customize.