mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.74k stars 32.24k forks source link

[IconButton] Add props to remove hover effect on <Iconbutton />, disableRipple #11108

Closed codewithanugraha closed 12 months ago

codewithanugraha commented 6 years ago

The hover effect on Iconbutton is really unwelcome in my app. Please add some option to disabling it instead of making it permanent.

screenshot from 2018-04-23 11-30-05

cc : @oliviertassinari Thanks in advance

oliviertassinari commented 6 years ago

@Anugraha123 The hover effect was added by @SebastianSchmidt in #10871. I don't think that it needs a specific property for disabling the behavior. However, you can proceeed as follow:

import React from "react";
import IconButton from "material-ui/IconButton";
import DeleteIcon from "@material-ui/icons/Delete";

export default function IconButtons(props) {
  return (
    <IconButton sx={{ "&:hover": { backgroundColor: "transparent" }} } aria-label="Delete">
      <DeleteIcon />
    </IconButton>
  );
}

https://codesandbox.io/s/m393r8rk5y

cyrfer commented 4 years ago

"transparent" did not work for me. I had to take a slightly different approach.

  ButtonUnclickable: {
    '&:hover': {
      backgroundColor: theme.palette.primary.main,
    }
  },
edgar-arroyo-by commented 3 years ago

hello there!

here kindly asking

@oliviertassinari

why is this not optional? sometimes it wont work good, for example, inside tables if the padding is dense.

oliviertassinari commented 3 years ago

@edgar-arroyo-by Why do you need the IconButton for if you don't get the hover style? Why not use ButtonBase directly?

This issue makes me think of #16866

oliviertassinari commented 3 years ago

I would propose this diff:

diff --git a/packages/material-ui/src/IconButton/IconButton.js b/packages/material-ui/src/IconButton/IconButton.js
index 62ae0a7738..da7d2f8380 100644
--- a/packages/material-ui/src/IconButton/IconButton.js
+++ b/packages/material-ui/src/IconButton/IconButton.js
@@ -51,13 +51,15 @@ const IconButtonRoot = styled(ButtonBase, {
     transition: theme.transitions.create('background-color', {
       duration: theme.transitions.duration.shortest,
     }),
-    '&:hover': {
-      backgroundColor: alpha(theme.palette.action.active, theme.palette.action.hoverOpacity),
-      // Reset on touch devices, it doesn't add specificity
-      '@media (hover: none)': {
-        backgroundColor: 'transparent',
+    ...(!styleProps.disableRipple && {
+      '&:hover': {
+        backgroundColor: alpha(theme.palette.action.active, theme.palette.action.hoverOpacity),
+        // Reset on touch devices, it doesn't add specificity
+        '@media (hover: none)': {
+          backgroundColor: 'transparent',
+        },
       },
-    },
+    }),
     ...(styleProps.edge === 'start' && {
       marginLeft: styleProps.size === 'small' ? -3 : -12,
     }),

However, to be aware that when the prop is set, the button is no longer accessible (not more visual feedback o hove and keyboard focus). It's also a bit inconsistent with the <Button> that keeps the hover style when disableRipple is set

milenazuccarelli commented 3 years ago

I'd be happy to take this one on if no one else is working on it @oliviertassinari

adamfitzgibbon commented 3 years ago

I'm taking a look!

adamfitzgibbon commented 2 years ago

Hey @oliviertassinari thanks for the suggestion, I made that change in the above PR, as well as an additional one that was pointed out by a reviewer. I haven't heard back from them in a couple of weeks though. Would you be able to take a look?

lindapaiste commented 2 years ago

However, to be aware that when the prop is set, the button is no longer accessible (not more visual feedback o hove and keyboard focus). It's also a bit inconsistent with the <Button> that keeps the hover style when disableRipple is set

I really hate that the hover effect is now tied to disableRipple!

I came across this issue while trying to figure out why there was no hover effect on one of my IconButtons. I have disableRipple set globally through my theme's defaultProps because I don't want the animation. Now none of my IconButtons have hover effects. Why?

In my opinion it is not intuitive that disabling the ripple animation would also remove the hover background color. Especially since that is not how it works on Button.

Can we have a separate option to control the hover? Like disableHover?

roman-myshchyshyn commented 1 year ago

I would propose this diff:

diff --git a/packages/material-ui/src/IconButton/IconButton.js b/packages/material-ui/src/IconButton/IconButton.js
index 62ae0a7738..da7d2f8380 100644
--- a/packages/material-ui/src/IconButton/IconButton.js
+++ b/packages/material-ui/src/IconButton/IconButton.js
@@ -51,13 +51,15 @@ const IconButtonRoot = styled(ButtonBase, {
     transition: theme.transitions.create('background-color', {
       duration: theme.transitions.duration.shortest,
     }),
-    '&:hover': {
-      backgroundColor: alpha(theme.palette.action.active, theme.palette.action.hoverOpacity),
-      // Reset on touch devices, it doesn't add specificity
-      '@media (hover: none)': {
-        backgroundColor: 'transparent',
+    ...(!styleProps.disableRipple && {
+      '&:hover': {
+        backgroundColor: alpha(theme.palette.action.active, theme.palette.action.hoverOpacity),
+        // Reset on touch devices, it doesn't add specificity
+        '@media (hover: none)': {
+          backgroundColor: 'transparent',
+        },
       },
-    },
+    }),
     ...(styleProps.edge === 'start' && {
       marginLeft: styleProps.size === 'small' ? -3 : -12,
     }),

However, to be aware that when the prop is set, the button is no longer accessible (not more visual feedback o hove and keyboard focus). It's also a bit inconsistent with the <Button> that keeps the hover style when disableRipple is set

So what is they way now, to remove ripple and add corresponding background color when on hover?

ZeeshanTamboli commented 12 months ago

This issue has been solved.