[Button] focus effect padding

[wilav-dev]

Duplicates

• [X] I have searched the existing issues

Latest version

• [X] 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: https://github.com/mui/material-ui/blob/adda22c283b8d6aea46a1be25e2dd18f4d03c7b9/packages/mui-material/src/ButtonBase/TouchRipple.js#L228

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.

[michaldudak]

:+1: Agreed. @alisasanib could you take Olivier's comment into consideration in your PR?

[alisasanib]

@michaldudak Yes sure!

[alisasanib]

@oliviertassinari please let me know if I understood your comment correctly. I checked the latest Material design version for button components, and I think the desired behavior for MUI whenever the button is focused is sth like this:

To achieve this goal, I think we should remove the pulsate related variables, classes, and functions as they won't be necessary anymore. It's also been used by different components such as Button, Tab, and Fab.

[michaldudak]

I believe @oliviertassinari meant to remove the ripple completely when focusing and leave just the changed background color, as shown on the demo: https://material.io/components/buttons#interactive-demo

[mnajdova]

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.

There is also a change in the shadow when focused:

@alisasanib you take this demo as an example: https://material.io/components/buttons#usage (this is what I used in the gif above).

BTW, we should treat this as a breaking change in my opinion, as it differs quite a bit compared to the previous behavior.

[wilav-dev]

Any update or workaround?

[michaldudak]

There is also a change in the shadow when focused

We do move the shadow when focused:

Material UI:

https://user-images.githubusercontent.com/4696105/199466639-652599f9-f80e-4338-969e-558a1bce72a4.mov

Material Design 2:

https://user-images.githubusercontent.com/4696105/199466656-8686139d-206d-4906-8e02-25f7a1fbfa53.mov

As for the breaking change - it's always hard to decide when doing visual changes like this. The change won't literally break any applications but will be perceivable by users and may result in new issues. The safe option here would be to introduce a prop controlling the appearance, but I'm not a fan of inflating the API.

There is a workaround in userland possible. Adding the disableFocusRipple prop and styling the focused state to make the button appear lighter should do the trick.