Interacting with Grammarly extension in a Dialog registers as an outside click

brianlovin commented 1 year ago

Bug report

Current Behavior

We have a dialog where users compose new posts. In that composer, when people interact with the Grammarly browser extension, those interactions are registered as outside clicks and attempt to close the dialog.

Expected behavior

Grammarly overlay clicks should not register as outside clicks.

Reproducible example

Unfortunately, I'm having a hard time getting a reproducible example in CodeSandbox, for some reason Grammarly doesn't want to work in that context. Here's a video of the problem (note that we show a "Confirm delete draft" dialog if the user tries to close the composer when they have drafted content, so that confirmation dialog is being triggered when the Dialog is being told to close).

To test in production, you can:

Install the Grammarly Chrome extension
Go to https://app.campsite.design → sign in with Google
Create a new post, type some content into the description field
Interact with the Grammarly extension overlay

https://user-images.githubusercontent.com/1923260/228615966-3aa4606a-e2e8-4901-8b2b-e2a78ed2eff8.mp4

Your environment

Software	Name(s)	Version
Radix Package(s)	@radix-ui/react-dialog	1.0.3
React	n/a	18.2.0
Browser	Chromium, Safari
Assistive tech
Node	n/a
npm/yarn
Operating System	macOS

joaom00 commented 1 year ago

In the meantime, you can use this workaround https://github.com/radix-ui/primitives/issues/1280#issuecomment-1319109163

or this https://codesandbox.io/p/sandbox/throbbing-lake-84vkp7 (need to open the preview in a new tab to show the Grammarly)

brianlovin commented 1 year ago

Perfect, that helps a ton! It's not a bulletproof solution, clicks in these red zones still register as "outside" but at least the inline suggestions will be taken care of 🙇‍♂️

Screen Shot 2023-03-29 at 11 52 23@2x

joaom00 commented 1 year ago

Perfect, that helps a ton! It's not a bulletproof solution, clicks in these red zones still register as "outside" but at least the inline suggestions will be taken care of 🙇‍♂️

So I think this workaround will work better, no? https://codesandbox.io/p/sandbox/throbbing-lake-84vkp7

brianlovin commented 1 year ago

This works! TypeScript doesn't like me, but I'll see if I can find a way to type this out or use a different target property.

Edit

Fixes if you cast:

const target = e.target as HTMLElement
const isGrammarly = target && target.hasAttribute('data-grammarly-shadow-root')

kbrgl commented 1 year ago

I can confirm that this issue also occurs using the 1Password extension. When I press the 1Password lock icon at the right of a text field, the dialog is closed.

tomdaniel-it commented 1 year ago

Any update to this?

tomdaniel-it commented 1 year ago

Actually, this seems to have been fixed (at least for the Grammarly extension, unsure about 1Password)

jacobemcken commented 1 year ago

I still experience dialogs being closed in Firefox 116 with the latest Grammarly extension when interacting with the Grammarly extension.

At least Firefox tells me there are no updates for Grammarly. This is the extension I am using: https://addons.mozilla.org/en-US/firefox/addon/grammarly-1/

jacobemcken commented 1 year ago

Chromium 111.0.5563.64 with the latest Grammarly extension also has issues (though as opposed to Grammarly in Firefox), I can choose spelling suggestions without it closing the dialog. But clicking on the green circle icon or the red circle with the number of spelling issues still closes the dialog.

benoitgrelard commented 6 months ago

Related to #1859

anatolzak commented 3 months ago

Hey @benoitgrelard,

I was able to solve this issue in Melt UI https://github.com/melt-ui/melt-ui/pull/1114.

Radix

https://github.com/radix-ui/primitives/assets/53095479/4f3459f1-2acd-4847-8922-b99e2d29a054

Melt

https://github.com/radix-ui/primitives/assets/53095479/c8f49bb7-c5f9-4b38-85a3-e51d172cc785

The issue here is that the DismissableLayer component should allow external JS to stop propagation of an interaction event. In the videos I provided, 1Password actually calls e.stopPropagation() on mouse events but Radix does not currently handle that.

I want to explain what I implemented in Melt and see if it makes sense to you before I create a PR for Radix.

The goal is to allow external JS to intercept any interaction event which should prevent us from calling onDismiss().

The challenge is that JS still triggers various events even if you intercept other events. For example, intercepting pointerdown externally will still not prevent pointerup and click events from triggering. Intercepting a mouse event does not prevent pointer events and click events from triggering.

So the solution I came up with is to keep track of whether any events were intercepted, and only call onDismiss if no events were intercepted.

And here is how I figured out how to check if any event was intercepted.

For each event type (pointerdown, pointerup, mousedown, mouseup, touchstart, touchend, click), I attach 2 event listeners--one for the capturing phase, and the other for the bubbling phase. In the capture phase, I mark the event type as intercepted, and then if the event type is called during the bubbling phase, I mark the event type as not intercepted.

I also only call onDismiss at the end of an interaction to allow intercepting an end-interaction event. Currently Radix calls onDismiss after pointerdown, which would not allow externally intercepting an end-interaction event.

I also debounce the start-interaction event handler to allow other start-interaction event handles to mark an event as non intercepted.

I detailed all of this in the Melt UI PR https://github.com/melt-ui/melt-ui/pull/1114. Let me know if this is something that makes sense to you and I will be happy to create a PR :)

radix-ui / primitives