Addepar / ember-widgets

https://opensource.addepar.com/ember-widgets/#/ember-widgets/overview
Other
288 stars 75 forks source link

Close popups in mousedown #285

Closed mixonic closed 5 years ago

mixonic commented 5 years ago

Popups were handling click events after the logic for the page had run. This meant sometimes you clicked on part of the page, Ember might re-render that part of the page and remove the click target, and then the closing logic here would run.

The closing logic confirms that the click target is still on the page, but as mentioned above the click target may not be.

Options here would be to remove the check for the target being on the page, or to handle closing the popovers on capture phase before other logic related to the click happens, or to use a psuedo-before-click event like mousedown.

Removing the check may cause some unexpected new behaviors so I would like to avoid it. Using the capture phase is a big change for this library since it then would not be testable via jQuery's click even triggering.

So using the psuedo-before-click handler of mousedown seems good. This is not compatible with some kinds of non-mouse input, which is the main risk of this approach.

Replaces: https://github.com/Addepar/ember-widgets/pull/284

mixonic commented 5 years ago

This is not viable. There are UIs with cascading popovers and with this logic clicking in a child popover may cause its parent to de-render (and other teardown logic to fire) before the click on the child itself can be run.