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

hover-media-query #25182

Closed
719media opened this issue Jan 3, 2018 · 15 comments
Closed

hover-media-query #25182

719media opened this issue Jan 3, 2018 · 15 comments
Labels

Comments

@719media
Copy link
Contributor

719media commented Jan 3, 2018

I was looking into hover-media-query, which landed in bootstrap 4 at some point in the past. There is even a little snippet of documentation about it here:

https://getbootstrap.com/docs/4.0/getting-started/browsers-devices/#sticky-hoverfocus-on-mobile

However, it appears that this $enable-hover-media-query is commented out in the src scss file and has no effect.

I was hopeful that the hover classes could be controlled with something like .touch on the <body> element, but it doesn't appear that this class works that way, as the mixin is inside of the element and can't be escaped...

Aside from this, as you probably know, the hover media query has two problems:

  1. It isn't supported in Safari (solved by using your shim https://github.com/twbs/mq4-hover-shim)
  2. It doesn't really "work" for multiple-input devices, where a user could touch and mouse

After reading https://codepen.io/MillerTime/post/prepend-body-classes-to-nested-selectors-sass-mixin, I realized the same thing as the author: That you can use the & anywhere in the selector, and easily create body .no-touch classes using the existing hover mixin within bootstrap.

I propose that this could land in bootstrap easily by means of a simple selector like:

#25181

Now all I need to do is set the variable in my own personal custom variables file, e.g.:

$enable-hover-selector: ':not(body.touch)'

Works great!

@patrickhlauke
Copy link
Member

so what's the actual scenario this tries to solve? skimming over the prepend classes article, it seems that all the author achieved is to achieve a DRYer way of having styles for "touch" and "non-touch". That still leaves the fundamental problem of multi-input devices, just in a terser way?

@719media
Copy link
Contributor Author

719media commented Jan 3, 2018

I think I misunderstood that the shim is meant to be run against code transformed during postcss phase, instead of a variable like I am using in this pull request... which after further consideration is probably not a great approach.

So, perhaps in the code the commented out $enable-hover-media-query can just be uncommented?

@patrickhlauke
Copy link
Member

my question really is: what are you actually trying to achieve? what's the end goal?

@719media
Copy link
Contributor Author

719media commented Jan 3, 2018

The end goal is to disable hover classes during touch, but not during actual mouse hover.

This was a worse problem than I initially supposed. Getting an element touched on a mobile device to "unhover" is problematic, so merely setting a body class during touch and then unsetting during touchend doesn't really solve the problem, since the element is still "hovered" on the mobile device even after the touchend. So, fixing the issue for multi-input devices where the hover works during mouse, but not during touch, seems like a goal that I cannot seem to achieve with any reasonable code.

So, the runner-up alternative is to disable hover styles where the device supports hover by either:

  1. using the media query @media (hover: hover) {
    or
  2. using a body selector class that is set for devices supporting hover (which the shim apparently does)

If we use route #2, then I was merely trying to get that class to be written to the bootstrap compiled css... however, it appears that the shim's intention was that this was taken care of during postcss, instead of sass mixin... so perhaps this is meant to be outside of the bootstrap build process? I am unsure of the intention of @mdo when authoring https://getbootstrap.com/docs/4.0/getting-started/browsers-devices/#sticky-hoverfocus-on-mobile, since it implies that settings $enable-hover-media-query actually does something, which it does not appear to do anything because the mixin is commented out

@patrickhlauke
Copy link
Member

as i've not looked at that part of bootstrap previously...do we have any real-world cases where this sticky :hover behavior manifests itself? even a simple test case would do. if anything, in my experience it's usually :focus styling that causes problems (to be addressed in future with the :focus-visible pseudo https://drafts.csswg.org/selectors-4/#focus-visible-pseudo)

@719media
Copy link
Contributor Author

719media commented Jan 3, 2018

Well, in theory it happens on every .btn class. The :focus behavior you speak of also happens, as does :active, but I'm concerned primarily with the :hover effect. If you're asking "does this happen in the default build of bootstrap", the closest I can see is using something like <a class="btn btn-link" href="#">Test</a>, in which case the :focus stays, sure, but so does the :hover (leaving the color: $link-hover-color; color from the :hover intact on mobile after clicking the test button.

I run into it whenever I have .btn styled elements on my page that are really just javascript handlers for toggling something visible/page interaction that I don't want to be sticky hover on touch.

So, I see a few ways forward:

  1. Just remove all the reference to the shim in the bootstrap docs, and get rid of the commented code in the sass mixin to avoid confusion
  2. Uncomment the code, so that at least the @media query can be supported if the $enable-hover-media-query value is set to true. If users want full browser support, they'll have to use the shim (e.g. latest firefox still doesn't support @media hover)
  3. Use something like this pull request, where instead of using the @media hover sass mixin, it just creates the prefix for you as part of the sass mixin. Then the users will still need to control the behavior of applying the prefix however they see fit in their own code.

Personally, it seems to me that the route bootstrap was heading (2 above) makes the most sense, because: 1) it's future compatible for whenever firefox decides to support @media hover, and 2) you can shim it if you want full browser support... and given that solution 3 above still requires custom code to be added to get it to work anyway, it doesn't seem worse. The only advantage solution 3 has is if a developer is clever enough to figure out a way to dynamically apply and remove prefixes based on touch vs. mouse, then the hover solution could be dynamic... but it's a jenky solution at best anyway.

Soooo... if you can just uncomment the code, that would work :)

@719media
Copy link
Contributor Author

719media commented Jan 3, 2018

One downside of the shim is that it requires jquery... now bootstrap itself requires jquery, so most users wouldn't care. I actually just use the css part of bootstrap, however, so I just roll my own solution.

@719media
Copy link
Contributor Author

719media commented Jan 3, 2018

I've trashed the pull request above with this based on our conversation:

#25194

Which almost gives the best of both media query OR prefix, except for one strange issue detailed in the pull request I can't figure out

@patrickhlauke
Copy link
Member

i'll note that testing for (hover: hover) only tests whatever the browser considers the "primary" input. on, say, a touch-enabled laptop it will evaluate to false, even though the user may be using the touchscreen. on a phone/tablet it will evaluate to true, even if the user is actually using a paired bluetooth mouse or a keyboard. so personally, i'm not a huge fan of solutions that make assumptions along those lines, so my personal preference would ideally be option 1 and removing this. i'll have to dig in a bit further to see about any alternative way of working around stuck :hover styles, but for what it's worth i'd like to tackle :focus styles with a proper :focus-visible use once it's supported, and hoping that browsers (i'm guessing mostly iOS nowadays?) stop doing their crappy heuristics-based focus stuff...

anyway, hold your horses with the PR until i've had a chance to look into it a bit more (it's unlikely to make it into 4.0 stable at this stage, so there'd have to be a bit of a wait anyway)

@719media
Copy link
Contributor Author

719media commented Jan 3, 2018

OK, good luck. I spent a little time trying to figure out how to independently control mouse-only hover without some jenky js AND support multi-input devices, but it does not seem possible without doing something like having js clone and re-insert the element into DOM, etc. which makes me cringe.

The best I've come up with is something like (written in jquery for bootstrap use):

$('body').one('touchstart', function() {
  $(this).addClass('touchstart');
});

where there is a slight benefit of not adding the class until the user actually does something that triggers touchstart (as opposed to just checking if touchstart is defined on load). So if the user is on device with mouse + touch, but is just using the mouse, it will maintain hover ability. Obviously if the user starts with touch, and then switches to mouse, then it breaks down.

To be clear, the problem with:

$('body').on('touchstart', function() {
  $(this).addClass('touchstart');
}).on('touchend', function() {
  $(this).removeClass('touchstart');
});

is that the element will show as :hover after the touch is finished, so you can't do that. I guess you could do something like

$('body').on('touchstart', function() {
  $(this).addClass('touchstart touch');
}).on('touchend', function() {
  //I believe the order of events is touchend fires before mouseover, so delay the removal
  setTimeout(function() {
    $(this).removeClass('touch');
  }, 0);
}).on('mouseover', function() {
  if(! $(this).hasClass('touch')) {
    $(this).removeClass('touchstart');
  }
});

@patrickhlauke
Copy link
Member

doing some light testing, it seems the problem of sticky :hover is one that only affects iOS/Safari (and of course any other browser on iOS based on UIWebView/WkWebView). on android, blink-based browsers like chrome/samsung internet/edge, gecko based firefox, and exotics like Maxthon and Puffin (though likely also blink based) don't have the issue.

iOS/Safari seems to completely ignore :focus and :active too, (which is useful for :focus as it doesn't make it appear stuck, but annoying for :active as that doesn't offer an alternative to indicate positive activation - xref #19709).

other browsers tested (see above) do correctly ignore :hover, most of them (blink-based and firefox, but not all of the more exotic ones) show :active during the actual tap, but then leave :focus showing - which is logical, and will be addressed in future with :focus-visible.

long story short, it feels like the odd one out (in the case of sticky hover) is just iOS...so it may be more apt to offer a hack/option involving ... and I hate to say it ... UA sniffing just for that (similar to some other hacky workarounds, like the one to make event delegation/bubbling work, we've had to do for iOS already).

i'll carry on investigating this further, and hopefully come up with something that, while hacky, will only target iOS (which, incidentally, does not allow pairing of a mouse, so has less chance of hitting multi-input scenario problems at this stage)

@patrickhlauke
Copy link
Member

for reference, my hacky test page. https://codepen.io/patrickhlauke/pen/EovRPz

note that the Safari behavior described above only applies to links/buttons. Safari half-behaves for other situations like "faked" <span role="button" tabindex="0"> type controls (:hover doesn't apply/stick, :focus does, but :active is ignored).

confirming Windows 10 Mobile/Edge behaves sanely like blink/gecko on android. more testing/hacking...

@patrickhlauke
Copy link
Member

some more background info: https://gauntface.com/blog/2013/12/09/focusing-on-the-web-today and https://developers.google.com/web/fundamentals/design-and-ux/input/touch/#respond_to_element_states

@719media
Copy link
Contributor Author

719media commented Jan 4, 2018

For what it's worth, here is what I use to determine if the hover is from mouse or touch, realtime:

const handleMouseMoveReset = function() {
  if(document.body.classList.contains('touchstart')) {
    if(! document.body.classList.contains('touch')) {
      document.body.classList.remove('touchstart');
      //remove myself
      this.removeEventListener('mousemove', handleMouseMoveReset, {passive: true});
    }
  }
};

document.addEventListener('touchstart', function() {
  document.removeEventListener('mousemove', handleMouseMoveReset, {passive: true});
  document.body.classList.add('touchstart', 'touch');
}, {passive: true});

document.addEventListener('mouseup', function() {
  if(document.body.classList.contains('touch')) {
    document.body.classList.remove('touch');
    document.addEventListener('mousemove', handleMouseMoveReset, {passive: true});
  }
}, {passive: true});

@mdo
Copy link
Member

mdo commented Apr 1, 2018

We removed the hover-media-query awhile back, so I think this is safe to close. Let me know if it needs to be re-opened to track a particular change for us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants