Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[material] hover media query issue on mobile #15000

Closed
adipascu opened this issue Mar 22, 2019 · 22 comments
Closed

[material] hover media query issue on mobile #15000

adipascu opened this issue Mar 22, 2019 · 22 comments
Labels
bug 🐛 Something doesn't work component: button This is the name of the generic UI component, not the React module! design This is about UI or UX design, please involve a designer external dependency Blocked by external dependency, we can’t do anything about it package: material-ui Specific to @mui/material

Comments

@adipascu
Copy link
Contributor

adipascu commented Mar 22, 2019

I have noticed that Material UI relies on hover media queries to check how to implement buttons, here is an example:

'&:hover': {
backgroundColor: fade(theme.palette.action.active, theme.palette.action.hoverOpacity),
// Reset on touch devices, it doesn't add specificity
'@media (hover: none)': {
backgroundColor: 'transparent',

Some browsers do not support this feature or they report an incorrect value, this has caused buttons to be "stuck" after touching them.

I'd like to be able to configure Material UI to treat the device input as a touchscreen with no hover support.
As a workaround, I made this JSS plugin that I run before the MUI plugins.

I did not have this issue in Material UI 0.x.

@eps1lon
Copy link
Member

eps1lon commented Mar 22, 2019

Some browsers do not support this feature or they report an incorrect value [...]

Could you be more specific here?

[...] this has caused buttons to be "stuck" after touching them.

Are you sure this isn't the focused state? As far as I know touch devices don't even trigger hover.

@adipascu
Copy link
Contributor Author

adipascu commented Mar 22, 2019

I am talking about the hover CSS media feature.

  • The Android 4.4 webview do not support this feature (this is not such a big issue).
  • Samsung Galaxy Note 9 (SM-N960F) reports that it supports hover.

When this happens, the buttons get stuck as described here and here.
Touch devices trigger the :hover CSS selector for backward compatibility reasons.

@adipascu adipascu changed the title Allow overriding hover media query Allow overriding hover media query (force touch mode) Mar 22, 2019
@eps1lon
Copy link
Member

eps1lon commented Mar 22, 2019

I am talking about the hover CSS media feature.

I understood that. I was asking about concrete device and demos on our page so that I can reproduce this. Chrome devtools won't trigger :hover if I'm in device mode.

@eps1lon eps1lon added waiting for user information component: button This is the name of the generic UI component, not the React module! labels Mar 22, 2019
@adipascu
Copy link
Contributor Author

adipascu commented Mar 22, 2019

It triggers on the material-ui 3.x demo button component doc page. You can test on the galaxy note 9 with chrome 73.

Sorry for the formatting, have a great weekend

@eps1lon
Copy link
Member

eps1lon commented Mar 23, 2019

It triggers on the material-ui 3.x demo button component doc page. You can test on the galaxy note 9 with chrome 73.

Got it. You already implied this in your previous comment, sorry. For anybody wondering how this issue looks:

mui-stuck-hover

After I moved the cursor from the button I click again which removes the hover. You can see this by the slight change in color (a little bit darker when hovered).

@eps1lon eps1lon added the bug 🐛 Something doesn't work label Mar 23, 2019
@adipascu
Copy link
Contributor Author

@eps1lon Yes, this is the exact issue I am facing, the issue is more visible on "flat" buttons like these.

@oliviertassinari
Copy link
Member

oliviertassinari commented May 29, 2019

Oh, I was exploring this same issue in: #15736 without knowing.

@oliviertassinari oliviertassinari changed the title Allow overriding hover media query (force touch mode) hover media query issue on mobile May 29, 2019
@Koniushok
Copy link

I can not solve it at my code, even if I do it I need to correct for all buttons in my code. I have one solution to change @media (hover: none) to @media (hover: none), (pointer: coarse)

@dan8f
Copy link
Contributor

dan8f commented Jun 23, 2019

@Koniushok you can customize your theme to override MuiButtonBase (hover transparent) when the device is mobile. Note this wouldn't solve the issue on hybrid devices...

There is a talk about these issues by Rick Hanlon. No idea if it would be acceptable for Mareial-UI to add mouse enter/ leave to handle hover? @oliviertassinari

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 23, 2019

@dan8f I have jumped and skipped the talk. When the codebase was using inline styles, we used to have this logic.

I think that our first area of investigation should be to dive into why new Samsung touch devices behave differently than the other brands https://www.ctrl.blog/entry/css-media-hover-samsung.html. If they acknowledge that it's a regression, I think that we can wait for their fix.

@adipascu
Copy link
Contributor Author

@oliviertassinari I noticed this is happening with Samsung phones that have a stylus. I think its a limitation because the hover media query prop remains active when the stylus is inactive.

@oliviertassinari
Copy link
Member

The Ionic approach doesn't work: https://codesandbox.io/s/material-demo-s1ux7.

I'm not aware of any good solution to the problem. I would propose that we wait for somebody to explore the possible solutions to the problem and to report back. I don't think that it's something the core team should invest time one. If somebody can find a solution that minimizes override complexity and bundle size, we will consider accepting it. For instance, @media (pointer: ) { is not a valid option.

@oliviertassinari oliviertassinari added the external dependency Blocked by external dependency, we can’t do anything about it label Jun 23, 2019
@dan8f
Copy link
Contributor

dan8f commented Jun 24, 2019

I'm afraid is not just Samsung devices. This problem also surfaces in Xiaomi too (tested on Xiaomi Mi A1), as well as hybrid devices.

@pavel06081991
Copy link

pavel06081991 commented Jun 25, 2019

The Ionic approach doesn't work: https://codesandbox.io/s/material-demo-s1ux7.

I'm not aware of any good solution to the problem. I would propose that we wait for somebody to explore the possible solutions to the problem and to report back. I don't think that it's something the core team should invest time one. If somebody can find a solution that minimizes override complexity and bundle size, we will consider accepting it. For instance, @media (pointer: ) { is not a valid option.

It looks like there is small javascript solution described here http://www.prowebdesign.ro/how-to-deal-with-hover-on-touch-screen-devices/

//test for touch events support and if not supported, attach .no-touch class to the HTML tag.
 
if (!("ontouchstart" in document.documentElement)) {
    document.documentElement.className += " no-touch";
}
`

@vhakulinen
Copy link

vhakulinen commented Jul 28, 2019

I'm hitting the same issue as described here. After spending a few hours debugging, I noticed that one can block the emulated click even from ontouchend by calling e.preventDefault(), which in turn will prevent the hover stuff from happening because the click events wont be emulated. The mdn documentation mentions this: https://developer.mozilla.org/en-US/docs/Web/API/Touch_events/Supporting_both_TouchEvent_and_MouseEvent.

But, with material-ui there is a problem with this. When calling e.preventDefault() from ontouchend, the tripple animation wont complete. Would it be possible to make this animation to complete regardless of any possible e.preventDefault() calls on ontouchend?

@oliviertassinari
Copy link
Member

@vhakulinen The logic initially comes from 7d18a95#diff-07544a6835ff92a7d3d095086e03361dR8. I have no objection with removing the check. Do you want to give it a try? :)

@vhakulinen
Copy link

I kinda do want to give it a try, but I'm working on other stuff for now. Once I get to back to the project where this is a problem, I might give it a shot!

@oliviertassinari
Copy link
Member

@vhakulinen Perfect :)

@oliviertassinari
Copy link
Member

I don't think that this problem is worth investing much more energy on it.

@danielo515
Copy link

danielo515 commented Jan 29, 2020

Why if there is no fix for it? This is still happening on the latest library version.
As I said on the linked issue, here is a screenshot from the project page:

image

That is from a real mobile device, a samsung galaxy s8. If you prefer to just provide a workaround, what about documenting it on the docs?

@OriginLive
Copy link

OriginLive commented Feb 26, 2020

@oliviertassinari In my page, buttons appear gray (disabled), for mobile users if a form throws an error. The same bug happens when testing on chrome and setting it in responsive mode.
Is there a fix for this issue?

For anyone, my fix is:

const theme = createMuiTheme({
  overrides: {
    MuiButton: {
      contained: {
        '@media (hover: none)': {
          '&:hover': {
            backgroundColor: `${primary_color} !important`
          }
        },
}}}});

@Rc85
Copy link

Rc85 commented Sep 9, 2020

@oliviertassinari In my page, buttons appear gray (disabled), for mobile users if a form throws an error. The same bug happens when testing on chrome and setting it in responsive mode.
Is there a fix for this issue?

For anyone, my fix is:

const theme = createMuiTheme({
  overrides: {
    MuiButton: {
      contained: {
        '@media (hover: none)': {
          '&:hover': {
            backgroundColor: `${primary_color} !important`
          }
        },
}}}});

This also overrides the disabled state of the button.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: button This is the name of the generic UI component, not the React module! design This is about UI or UX design, please involve a designer external dependency Blocked by external dependency, we can’t do anything about it package: material-ui Specific to @mui/material
Projects
None yet
Development

No branches or pull requests

10 participants