fix(material/core): ripple coords when trigger is not the container

[enten]

This pull request is a proposal to fix coordinates emitted from the trigger element when it's different than the container element.

Actual behavior

When a pointer down event (mousedown: MouseEvent or touchstart: TouchEvent) is emitted from the trigger element, its clientX and clientY properties are used to call RippleRender::fadeInRipple.

In case where the trigger element is the ripple container element, clientX and clientY are inside the container dom rect.

But when the trigger element is different than the ripple container element (a trigger element can be set by using matRippleTrigger input), clientX and clientY are outside the container dom rect.

Expeded behavior

In case where the trigger element is different than the ripple container element, using clientX and clientY properties and calculating an equivalent point in the container element before calling RippleRender::fadeInRipple.

Reproduction

StackBlitz link: https://stackblitz.com/edit/components-issue-gtvxae

• The div "Ripple Container" is the ripple container: <div matRipple matRippleRadius="10" [matRippleTrigger]="trigger"></div>
• The "Trigger Ripple" button is the ripple trigger: <button #trigger>Trigger Button</button>
• When "Trigger Ripple" button is clicked, the ripple element (red dot in screenshot below) is positionned outside the ripple container dom rect (the ripple element is positionned inside the trigger dom rect)
• When "Trigger Ripple with expected behavior" button is clicked, the clientX and clientY properties are fixed and the ripple element (green dot in screenshot below) is positionned correctly inside the ripple container dom rect

  /** Fix coordinates emitted from the trigger element when is not the container element. */
  private _fixCoordinatesEmittedFromTrigger(x: number, y: number): [number, number] {
    if (this._triggerElement && this._triggerElement !== this._containerElement) {
      const containerRect = (this._containerRect =
        this._containerRect || this._containerElement.getBoundingClientRect());
      const triggerRect = (this._triggerRect =
        this._triggerRect || this._triggerElement.getBoundingClientRect());
      x = ((x - triggerRect.left) / triggerRect.width) * containerRect.width + containerRect.left;
      y = ((y - triggerRect.top) / triggerRect.height) * containerRect.height + containerRect.top;
    }
    return [x, y];
  }
}

[devversion]

Thx for the PR (sorry for the delayed review— I'm not working on Material but got assigned)

[devversion]

This assumes that the dimensions of the trigger are translatable to the container right? I wonder if we would rather want to prevent the case where the trigger is outside of the container? What do you think?

[enten]

This assumes that the dimensions of the trigger are translatable to the container right?

That's right. And I don't see any case where the dimensions of the trigger aren't translatable to the container because here we do simple proportion calculations (except when the container height or width equal to 0 but this case is already covered).

I wonder if we would rather want to prevent the case where the trigger is outside of the container?

No, I don't want to prevent the case where the trigger is outside of the container. I want to translate a position clicked in trigger element into the ripple container.

In this stackblitz to reproduce the current behavior, I apply [matRippleRadius]="10" on the ripple container. But if you don't apply a radius, you can see that the ripple element coords is outside the ripple container (I clicked where there is the letter "X" in red) and create a huge radius:

In this example, the experience is not too shocking because the trigger is near to the ripple container. But in case where the trigger is far from the ripple container: the ripple element rendering is strange (when we think about user experience). It's a better user experience when the ripple element coords is inside the ripple container in coords translated from the original coords (where the user clicked on the trigger).

Note: indeed, this translation has no effect when the ripple element is centered.

[devversion]

That's right. And I don't see any case where the dimensions of the trigger aren't translatable to the container because here we do simple proportion calculation

Right, but does it make sense to do a simple proportion calculation? what's the use-case? I.e. in most cases the ripple should originate where the click happens, and different proportions wouldn't necessarily make sense then, right?

No, I don't want to prevent the case where the trigger is outside of the container. I want to translate a position clicked in trigger element into the ripple container.

What's the use-case though? Why wouldn't the trigger not be inside the container? or a sibling that is "positioned over the ripple container"?

In this stackblitz to reproduce the current behavior, I apply [matRippleRadius]="10" on the ripple container. But if you don't apply a radius, you can see that the ripple element coords is outside the ripple container (I clicked where there is the letter "X" in red) and create a huge radius:

Yeah, this is a bug. Good catch, and one solution would be to disallow the trigger to be outside of the boundaries of the container I think.

[enten]

Yeah, this is a bug. Good catch, and one solution would be to disallow the trigger to be outside of the boundaries of the container I think.

I don't think that is a bug. As we can read in ripple.md: it's a feature "to show ripples on interaction with some other element" that the container.

Right, but does it make sense to do a simple proportion calculation? what's the use-case?

Check yourself: I made another stackblitz and compare the user experience with the both triggers (I think that a demonstration can be more explicit than words).

[devversion]

I don't think that is a bug. As we can read in ripple.md: it's a feature "to show ripples on interaction with some other element" that the container.

yeah, I actually created the implementation of ripple a couple of years ago. The intent was to support triggers of different elements, but not necessarily disconnected from the container visually (but to support the trigger overlaying the ripple elements, to e.g. not prevent further clicks).

We can limit this feature, or clarify the intent to address this problem. It's just an option we can consider. We can certainly also fix this if it makes sense to do so.

Check yourself: I made another stackblitz and compare the user experience with the both triggers (I think that a demonstration can be more explicit than words).

I follow what you are trying to achieve, but what is the real-world use-case here? Why would you want to display the ripple in a totally different element? Ripples from the Material specification generally don't seem to mention such a scenario: https://m2.material.io/develop/ios/supporting/ripple

Also cc. @crisbeto / @andrewseguin