fix(ripple): don't launch ripple for fake mouse events #11997
Conversation
googlebot
added
the
cla: yes
| @@ -246,10 +246,13 @@ export class RippleRenderer { | |||
|
|
|||
| /** Function being called whenever the trigger is being pressed using mouse. */ | |||
| private onMousedown = (event: MouseEvent) => { | |||
| // Screen readers will fire fake mouse events for space/enter. Skip launching a | |||
| // ripple in this case for consistency with the non-screen-reader experience. | |||
| const isFakeMousedown = event.buttons === 0; | |||
There was a problem hiding this comment.
We should use isFakeMousedownFromScreenReader from @angular/cdk/a11y?
There was a problem hiding this comment.
I wanted to avoid pulling in cdk/a11y since the idea is to pull the ripples out into their own module.
There was a problem hiding this comment.
I see. I don't really think though, that this means that we should hesitate using the CDK. Even if the ripples are moved into its own module, they can still have dependencies on the CDK.
The goal is not make ripples independent from anything else. It should be just possible for developers to explicitly use @angular/material/ripple without having all the other stuff.
e.g. see @angular/material/core does have deps on the CDK, and also the ripples already use cdk/platform.
There was a problem hiding this comment.
What I meant is that it seemed like overkill to bring in another CDK package for a one-liner method. I've switched it now.
There was a problem hiding this comment.
Yeah, totally see what you mean. My reasoning is just that the CDK is being pulled by NPM anyway and with tree shaking (which is quite common used) it doesn't really matter how big the a11y package is because only the one function will be pulled in.
There was a problem hiding this comment.
Not sure if I can make that decision though. Let's wait what Jeremy says on that. I'm a huge fan of consistency, and I'm worried that we'll duplicate logic which can be easily forgotten.
crisbeto
force-pushed
the
ripple-fake-mouse-events
branch
from
d57f85d to
06b7a36
Compare
josephperrott
added
the
target: patch
|
Hi @crisbeto! This PR has merge conflicts due to recent upstream merges. |
When using a screen reader the ripples can get fake mouse events when pressing space or enter. With the following changes we skip launching the ripples in these cases, in order to align the experience with the non-screen-reader.
crisbeto
force-pushed
the
ripple-fake-mouse-events
branch
from
06b7a36 to
e0bf841
Compare
|
Hi @crisbeto & @jelbourn, |
When using a screen reader the ripples can get fake mouse events when pressing space or enter. With the following changes we skip launching the ripples in these cases, in order to align the experience with the non-screen-reader.
|
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. |
When using a screen reader the ripples can get fake mouse events when pressing space or enter. With the following changes we skip launching the ripples in these cases, in order to align the experience with the non-screen-reader.
For reference:
