fix(slide-toggle): don't show hover ripples on touch devices #13702
Conversation
googlebot
added
the
cla: yes
| // Disable the hover styling if the user's device doesn't support hovering. | ||
| @media (hover: none) { | ||
| .mat-slide-toggle-bar:hover & { | ||
| display: none; |
There was a problem hiding this comment.
Why not just opacity: 0 as for the other selectors? Also looks like on touch devices the persistent ripple can be hidden without the :hover right?
There was a problem hiding this comment.
I wanted to prevent any selectors with a higher specificity from reintroducing the opacity.
Also looks like on touch devices the persistent ripple can be hidden without the :hover right?
I'm not sure I get it. Do you mean using the focus monitor classes?
There was a problem hiding this comment.
I see, just feels a bit odd to use display: none because I personally just use display: none if something needs to be hidden for a longer time-span. But in this case, it can quickly toggle the display because you're only hiding the persistent ripple if the :hover pseudo element is activated.
I'm not sure I get it. Do you mean using the focus monitor classes?
I've meant that we could just set display: none permanently if @media (hover: none) is activated because in no other scenario on touch, the persistent ripple shows up (after #13562)
There was a problem hiding this comment.
That's what I did initially (always having display: none), but I decided to keep the more specific selector in case the input receives keyboard focus somehow (e.g. programmatically through the FocusMonitor or by pressing the "Next" button on the virtual keyboard).
There was a problem hiding this comment.
Actually thinking about it more, as it is now it allows us to keep the focus indication for hybrid devices that have both a touchscreen and a keyboard.
There was a problem hiding this comment.
Fair enough. Can you add this into a comment? doesn't have to be too detailed.
devversion
added
pr: lgtm
action: merge
On touch devices `:hover` styling persists after the user has tapped. These changes hide the persistent ripple if the user isn't able to hover, in order to avoid confusion with the other ripples. Related to angular#13675.
crisbeto
force-pushed
the
13675/slide-toggle-hover
branch
from
5b59fd4 to
7927d59
Compare
On touch devices `:hover` styling persists after the user has tapped. These changes hide the persistent ripple if the user isn't able to hover, in order to avoid confusion with the other ripples. Related to #13675.
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
On touch devices
:hoverstyling persists after the user has tapped. These changes hide the persistent ripple if the user isn't able to hover, in order to avoid confusion with the other ripples.Related to #13675.
Note: this is the same issue that is fixed in #13700. I'm splitting into separate PRs, because we're blocked on merging style changes for some components, for now.