fix(Tooltip): remove click stopPropagation

[FrivalszkyP]

Closes #9958

Changelog

Removed

• Remove e.stopPropagation() from the click event handler. In React 17, the stopPropagation prevents other Tooltip's clickoutside handlers from executing. It probably affects other clickoutside event handlers from executing.

Please note: removing it might cause other side effects but I couldn't come up with any scenarios like that.

Testing / Reviewing

See steps to repro in this Issue: #9958

[netlify]

✔️ Deploy Preview for carbon-react-next ready!

🔨 Explore the source changes: 21adaf1

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/61831bf3ef8f93000847986b

😎 Browse the preview: https://deploy-preview-9959--carbon-react-next.netlify.app

[netlify]

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: 21adaf1

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/61831bf33229c100084de4d5

😎 Browse the preview: https://deploy-preview-9959--carbon-elements.netlify.app

[netlify]

✔️ Deploy Preview for carbon-components-react ready!
Built without sensitive environment variables

🔨 Explore the source changes: 21adaf1

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/61831bf3777a94000711147c

😎 Browse the preview: https://deploy-preview-9959--carbon-components-react.netlify.app

[tay1orjones]

Hey @FrivalszkyP! Thanks for submitting this change. Good catch here - the changes to event pooling in React v17 are probably the culprit.

The stopPropagation was added quite a while ago. The example used in the commit message d32424c says it's for tooltips inside buttons.

I'll do some quick tests on this to see if I can find any side effects with tooltips in modals or tooltips.

[tay1orjones]

I couldn't find any side effects 👍

[FrivalszkyP]

@tay1orjones, thanks for being so thorough, I really appreciate it!

[FrivalszkyP]

@tay1orjones I tested it and the Tooltip inside a Button issue could definitely be a problem if we merge this PR. On the other hand, a Tooltip inside a Button is just a button inside another button, in terms of the AOM, so it should be avoided anyway. So the original reason the stopPropagation() was added would have been better handled by refactoring the design and not adding the Tooltip inside a Button.

[tay1orjones]

@FrivalszkyP Yes I agree, tooltips shouldn't be placed inside buttons. We do use a tooltip for the hasIconOnly variant, but when clicking/dismissing the tooltip the same behavior is seen between the deploy preview storybook here and the production storybook.

tooltips.dismiss.and.focus.to.button.mov

[FrivalszkyP]

@tay1orjones interestingly, it works differently for me. Chrome 95 on MacOS Big Sur 11.6. Using the deploy preview, clicking on the tooltip bubble does not close the bubble for me. I'll take a closer look on this, also I'll add storybook actions for the button onclick event and see whether the event fires on my branch. Thanks for bringing this to my attention!

[ZachStowellIBM]

@FrivalszkyP @tay1orjones - I'm actually running into an issue due to these changes.

We have a DataTable, that has sortable headers. One of these headers has a tooltip in it.

The issue we run in to is the exact scenario you were discussing above, about tooltips inside of buttons (since the ability to toggle the sort order is effectively a button).

What's the best course of action here from a Carbon perspective? This seems like a use case where we would want to stopPropagation, however there's no mechanism in the Tooltip component to do so.

[tay1orjones]

@ZachStowellIBM Unfortunately, interactive controls can't have focusable descendants like this. Nested interactive controls are not announced by screen readers. Because of this, placing a Tooltip inside a parent element that is interactive will fail WCAG Success Criterion 4.1.2.

Until this PR, the Tooltip masked this problem for quite some time by calling stopPropagation internally. It's been there all along though. The event propagation can be stopped in an onClick on an element wrapping the Tooltip, but I'd caution that as a temporary workaround.

Rel: #10284

[ZachStowellIBM]

Thanks for the info @tay1orjones! We'll be sure to make changes on our end.