vmware-archive / clarity

Clarity is a scalable, accessible, customizable, open source design system built with web components. Works with any JavaScript framework, built for enterprises, and designed to be inclusive.
http://clarity.design
MIT License
6.43k stars 763 forks source link

fix: refactor and simplify popover outside click listener #6619

Closed Shijir closed 2 years ago

Shijir commented 2 years ago

Signed-off-by: stsogoo stsogoo@vmware.com

PR Checklist

Our current popover outside click listener was implemented in a way too complex way. This was partly due to the fact we tried to listen to outside clicks during the bubbling/bubble-up phase.

This PR sets the outside click listener to the capture phase. With this solution, we don't need to track elements or events we should ignore. It could directly check if the event originates from the host element or one of its children elements (in that case it doesn't try to change toggle.open). Also, we could avoid setTimeout solution when a popover is opened by an outside element.

When an outside element's click event triggers a popover, the event first sets the toggle.open to true, and then the event bubbles up to document (the root of the dom) to which our outside click listener would be attached so it instantly responds and sets toggle.open back to false. With a listener with useCapture, we can capture the event first and then sets the property toggle.open so this could help us avoid the instant switch on-and-off issue of the popover.

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

Other information