[ButtonBase] Inconsistent ripple style with the material specs

[arsen]

If you compare the ripple effect from material specs, it feels like the one here is a bit slower than the one at Google, animation duration should be a little shorter.

You can see it yourself.

• Google version: https://material.io/components/buttons
  

• MUI version: https://material-ui.com/components/buttons/
  

[mbrookes]

MCW isn't the spec, it's another Google team's interpretation of it, although ironically material.io now uses MCW for their "Interactive demo" giving the sense that it is definitive, when often it deviates from the spec.

Could you reference the spec at material.io (but not an "interactive demo" 🙂)?

[Studio384]

The List specs show the ripple effect in various demos that are part of the specs and not another interpretation by a Google team. They do seem faster than the ripple effect that MUI uses today.

[mbrookes]

It's a shame the ripple doesn't have specific guidelines, as it seems to vary from one component to another, depending on the designer who created the video.

I can't work out why from the code, but comparing the ripple on the Card component with that on the List, it appears that the Card ripple is much faster, despite the two components being the same width.

I wondered if the speed was in some way proportional to the area, but DURATION is a fixed value. 🤷‍♂

[arsen]

I think that it should at least be configurable, the speed of the ripple effect.
Looks like the duration is a fixed const.

https://github.com/mui-org/material-ui/blob/c06a88d24235685dd769c1a3df82152ded17a6ca/packages/material-ui/src/ButtonBase/TouchRipple.js#L8

[eps1lon]

I think making the speed configurable as suggest by @arsen in #19664 (comment) is justified.

[oliviertassinari]

@arsen Why do you need to configure the duration?

[arsen]

@oliviertassinari
It may sound weird but having the DURATION 550ms (plus DELAY_RIPPLE 80ms) on mobile devices feels that the app is running slow.

[oliviertassinari]

@arsen What would be a better value? Would it make sense to apply such value by default?

[oliviertassinari]

The current implementation of the ripple doesn't seem aligned anymore with how Google implements it on its product, or on the Material Design spec. Nowadays, the ripple seems to start with a wider surface and to move faster. I have added a screen recorder on the issue's description to better compare the "feeling".

[aeharding]

Just wanted to say it would be awesome if the ripple became a first class component like in Google's material-design. It would make the ripple much more versatile for things like large ripple surfaces (see #24310) - I closed that as a duplicate of this issue because aligning the ripple style with Material's spec would fix that. :)

[mbrookes]

The current implementation of the ripple doesn't seem aligned [with] the Material Design spec

I think you missed the comment* at the beginning that the live examples in the spec (such as the one you included the screen recording for) are MCW. Compare instead the list videos that @Studio384 mentioned.

*This time round at least. You previously upvoted it.

[oliviertassinari]

@mbrookes I have seen Google Android applications go in the same direction. I also can't find anything about the ripple on https://material.io/.

[mbrookes]

I also can't find anything about the ripple on https://material.io/.

Yes, like I said here: #19664 (comment)

[ghost]

@arsen What would be a better value? Would it make sense to apply such value by default?

Is there any development plan for making the speed configurable

[oliviertassinari]

@codewangshuhao Now that we have dynamic styles, it's technically easier. But, how and why would you like to change the value?

Regarding solving this issue, how about we move toward making the ripple less noticeable, closer to material.io?

diff --git a/packages/material-ui/src/ButtonBase/TouchRipple.js b/packages/material-ui/src/ButtonBase/TouchRipple.js
index 3d219bb956..16d0564949 100644
--- a/packages/material-ui/src/ButtonBase/TouchRipple.js
+++ b/packages/material-ui/src/ButtonBase/TouchRipple.js
@@ -8,18 +8,22 @@ import useThemeProps from '../styles/useThemeProps';
 import Ripple from './Ripple';
 import touchRippleClasses from './touchRippleClasses';

-const DURATION = 550;
+const DURATION = 400;
 export const DELAY_RIPPLE = 80;

 const enterKeyframe = keyframes`
   0% {
-    transform: scale(0);
-    opacity: 0.1;
+    transform: scale(0.15);
+    opacity: 0.01;
+  }
+
+  33% {
+    opacity: 0.13;
   }

   100% {
     transform: scale(1);
-    opacity: 0.3;
+    opacity: 0.24;
   }
 `;

@@ -74,7 +78,7 @@ export const TouchRippleRipple = experimentalStyled(
   position: absolute;

   &.${touchRippleClasses.rippleVisible} {
-    opacity: 0.3;
+    opacity: 0.24;
     transform: scale(1);
     animation-name: ${enterKeyframe};
     animation-duration: ${DURATION}ms;

This is a change that I think can only be reviewed by applying it and trying it out.

[ghost]

@codewangshuhao Now that we have dynamic styles, it's technically easier. But, how and why would you like to change the value?

Regarding solving this issue, how about we move toward making the ripple less noticeable, closer to material.io?

diff --git a/packages/material-ui/src/ButtonBase/TouchRipple.js b/packages/material-ui/src/ButtonBase/TouchRipple.js
index 3d219bb956..16d0564949 100644
--- a/packages/material-ui/src/ButtonBase/TouchRipple.js
+++ b/packages/material-ui/src/ButtonBase/TouchRipple.js
@@ -8,18 +8,22 @@ import useThemeProps from '../styles/useThemeProps';
 import Ripple from './Ripple';
 import touchRippleClasses from './touchRippleClasses';

-const DURATION = 550;
+const DURATION = 400;
 export const DELAY_RIPPLE = 80;

 const enterKeyframe = keyframes`
   0% {
-    transform: scale(0);
-    opacity: 0.1;
+    transform: scale(0.15);
+    opacity: 0.01;
+  }
+
+  33% {
+    opacity: 0.13;
   }

   100% {
     transform: scale(1);
-    opacity: 0.3;
+    opacity: 0.24;
   }
 `;

@@ -74,7 +78,7 @@ export const TouchRippleRipple = experimentalStyled(
   position: absolute;

   &.${touchRippleClasses.rippleVisible} {
-    opacity: 0.3;
+    opacity: 0.24;
     transform: scale(1);
     animation-name: ${enterKeyframe};
     animation-duration: ${DURATION}ms;

This is a change that I think can only be reviewed by applying it and trying it out.

thanks for reply，For my usage scenario, I’m just confused about DURATION by the inconsistency between material-ui and material.io, and when DURATION is equal to 500ms, I feel a bit stuck when clicking the button. If material-ui can lower the DURATION value, then the configurable DURATION will not Apply to my usage scenario

[oliviertassinari]

@codewangshuhao Do you think that you could: 1. fork the repo 2. run yarn 3. run yarn start 4. apply the patch 5. open http://0.0.0.0:3000/components/buttons to see if my proposed patch feels better or worse?

[oliviertassinari]

They have updated the implementation: https://next.vuetifyjs.com/en/components/buttons/#flat.

[ghost]

@codewangshuhao Do you think that you could: 1. fork the repo 2. run yarn 3. run yarn start 4. apply the patch 5. open http://0.0.0.0:3000/components/buttons to see if my proposed patch feels better or worse?

I have tried this solution, and I think the stuttering feeling has improved significantly, but the click feel of the button is still not the same as that of material.io[just for myself，i will only use it on pc]

but as the ui framework，we should discuss the final realization plan with more developer，and change it more carefully.

[oliviertassinari]

@codewangshuhao Your description matches my experience with the diff. It feels better and it's not matching material.io, for instance, they have a much faster transition (I didn't have the time to aim for it).

[ghost]

Personally think that the best effect is from the latest material-components-web-components，especially for button with router.push
@oliviertassinari