UI that cancels click events prevents popovers from auto-closing

[nwhittaker]

Check existing issues

• I have checked for existing issues to avoid duplicates

Actual Behavior

Given a popover with auto-close enabled, clicking an element that prevents propagation of the click event causes the popover to remain open.

Expected Behavior

Clicking an element that prevents propagation of the click event does not cause the popover to remain open.

Reproduction Sample

https://codepen.io/nwhittaker-esri/pen/MWRvPzQ

Reproduction Steps

1. Visit the repro sample and click the Layer information button to open the popover.
2. With the popover still open, click the Button with event cancelation button and observe the popover remains opened.
3. Optionally click a blank area in the document body and see the popover closes.

Reproduction Version

2.7.0

Relevant Info

Possibly regressed by #8983. Is it possible to restore the handler to the capture phase and inspect the event's target there to see if it's disabled or aria-disabled instead?

Other components that exhibit the same regressed behavior: combobox, dropdown, and input-time-zone.

Regression?

2.6.0

Priority impact

p2 - want for current milestone

Impact

This change in behavior is breaking our tests in ways that are not easily solvable and preventing us from upgrading to Calcite 2.7. Going forward, it also prevents us from implementing UI with any sort of click cancelation.

A workaround could be to have our handlers relay a new click event to the window, but that doesn't solve the problem where the cancelation was intended to prevent bubbling up to another handler that is also on the window.

Calcite package

• @esri/calcite-components
• @esri/calcite-components-angular
• @esri/calcite-components-react
• @esri/calcite-design-tokens
• @esri/eslint-plugin-calcite-components

Esri team

ArcGIS Field Apps

[timmorey]

This issue is currently blocking us from adopting 2.7+ in field-maps-designer. Over the last 5+ yrs of development, we have accumulated a number of stopPropagation() calls on click events in various parts of the UI. Each of these is now effectively a "dead spot" in the app where a click interaction will fail to close a popover/dropdown/etc.

Maybe we could find better solutions than doing stopPropagations() in some of these places, but the effort to re-evaluate each would be quite large. I have to presume that a number of other apps are in a similar situation, since stopPropagation() is a common tool to control how events are handled.

[jcfranco]

@nwhittaker @timmorey Apologies for the delayed response. We are exploring alternative solutions and potential workarounds. Worst-case scenario, we may need to revert #8983 for the upcoming release and aim to reinstall in May.

It's important to note that we want all components to use the target/bubbling event phase for consistency and to meet developer expectations. Currently, relying on the capture phase is problematic as it complicates other fixes we need to implement.

Maybe we could find better solutions than doing stopPropagations() in some of these places, but the effort to re-evaluate each would be quite large.

Agreed. It'll take some effort to re-evaluate stopPropagation() usage, but it seems like a worthwhile task to help prevent disruptions and bugs.

cc @geospatialem

[nwhittaker]

It's important to note that we want all components to use the target/bubbling event phase for consistency and to meet developer expectations. Currently, relying on the capture phase is problematic as it complicates other fixes we need to implement.

@jcfranco is it possible to make a distinction: Events targeting elements inside a Calcite shadow-dom vs. events targeting elements outside a Calcite shadow-dom? Where the former events use bubbling and the latter use capturing?

Alternatively, is going back to pointer events¹ for these types of "outside click" handlers an option? It's not perfect, but might be less likely to conflict with developer flows that are listening for mouse events.

Footnotes

1. https://github.com/Esri/calcite-design-system/pull/8943 ↩

[jcfranco]

is it possible to make a distinction: Events targeting elements inside a Calcite shadow-dom vs. events targeting elements outside a Calcite shadow-dom? Where the former events use bubbling and the latter use capturing?

I could be missing something, but this might not help with eventing expectation concern. It'll be simpler to use the same phase. Component composition might also make this tricky as the targeted component could be used within either light or shadow DOM.

Alternatively, is going back to pointer events1 for these types of "outside click" handlers an option?

I'd like to keep click for these contexts.

---

Workaround-wise, would it be possible to set up something like the following into your environments? The key parts of the workaround are:

1. hijack Event.prototype.stopPropagation to extend default behavior 👇 (a custom helper to stop propagation could also be used, but went with the simplest setup)
2. emits custom click event directly on window (should help bypass any event handlers in between and toggles popovers)
   • alternatively, this could emit a KeyboardEvent of type keydown with Escape as the pressed key

[nwhittaker]

I could be missing something, but this might not help with eventing expectation concern. It'll be simpler to use the same phase.

My point was there could be a distinction between events that can be canceled and ones that cannot (e.g. outside clicks). For the ones that cannot, they should be listened for in the capture phase to mitigate cancelation. There's the added complexity of checking for disabled reference elements, but it's with significant benefit to the consumer.

Component composition might also make this tricky as the targeted component could be used within either light or shadow DOM.

Wondering what you're thinking of here because the event's composedPath() function gets everything including elements in shadow DOMs.

Workaround-wise, would it be possible to set up something like the following into your environments?

I did address this in the issue description. As a general workaround, it fails for the case where I have other click handlers on the window that I want to stop the event for. I also suspect monkey patching native classes is something the team wants to avoid.

---

Ideally the outside-click handling just works without impact to the consumer code. Workarounds aside, I guess I'm left wondering what are the technical blockers that make that not achievable?

[wei8123]

Hi, this issue effects the click event in ArcGIS Experience Builder Shadow cast tool.

This is also reproducible with a plain Calcite app. Here's a simple app which reproduces the problem.
https://codepen.io/hccampos/pen/WNBvLqK

[jcfranco]

I meant to follow up on this sooner, but we are not considering this a regression and will proceed to close for the following reasons:

1. The event phases our components use are not publicly documented and are subject to change.
2. Our components aim to follow developer expectations and therefore use the bubbling/target phase for event handling.
   1. Popover and tooltip were the only outliers using the capture phase due to the initial implementation. We recently refactored popover to use a different phase and will do the same for tooltip.
3. Stopping event propagation can have unintended side effects and break expectations of how events normally work across components (this is important for a low-level component library).
   1. This will also cause issues with other components currently (e.g., dropdown).
   2. It is very likely to encounter the same issue with other component libraries.

I synced up with @nwhittaker a while back, and their team will pursue an app-level workaround. Not sure if the approach is ready to be shared at this time.

@wei8123 In your example, the stopImmediatePropagation is preventing all click handlers from processing the event (including the popover). Would any of the following workarounds help?

[nwhittaker]

Not sure if the approach is ready to be shared at this time.

Here's our current working workaround. I don't know how comprehensive it is, but it seems to work well so far. The basic approach is to create a click capture listener on the document whenever a target opens and remove it when the target closes. If the listener is invoked, it closes the target if the click occurred outside of the target or any reference-element.

EDIT: To clarify, this workaround only takes effect once the target is open. It does not endeavor to guess whether or not stopping the opening event was intended to prevent the target from opening, or to stop something else.

https://gist.github.com/nwhittaker/e07e4e0ec4dfb0ec0a52a1fe98dfff30

[jcfranco]

Thanks for sharing your workaround @nwhittaker!

Closing per #9013 (comment).

[qlqllu]

ExB has a complex layout system. In the builder, we need to stop the propagation of click events when selecting a widget. And the workaround seems not work in ExB, do we miss something?

[wei8123]

I meant to follow up on this sooner, but we are not considering this a regression and will proceed to close for the following reasons:

1. The event phases our components use are not publicly documented and are subject to change.

2. Our components aim to follow developer expectations and therefore use the bubbling/target phase for event handling.

   1. Popover and tooltip were the only outliers using the capture phase due to the initial implementation. We recently refactored popover to use a different phase and will do the same for tooltip.

3. Stopping event propagation can have unintended side effects and break expectations of how events normally work across components (this is important for a low-level component library).

   1. This will also cause issues with other components currently (e.g., dropdown).
   2. It is very likely to encounter the same issue with other component libraries.

I synced up with @nwhittaker a while back, and their team will pursue an app-level workaround. Not sure if the approach is ready to be shared at this time.

@wei8123 In your example, the stopImmediatePropagation is preventing all click handlers from processing the event (including the popover). Would any of the following workarounds help?

Hi @jcfranco, as @qlqllu mentioned above, executing this workaround file in EXB's init does not solve the problem:

Because clicking the problematic button in the 3D-Toolbox widget, does not trigger the events listened in the workaround file (such as 'calciteDropdownBeforeOpen', 'calciteDropdownBeforeClose').
Any idea?

[jcfranco]

@qlqllu @wei8123 Could you please share more details about your issue? I'm afraid I can't help without additional context. We can also schedule a meeting or call if you’d prefer.

[qlqllu]

@jcfranco We call the patchComponentsWithOutsideClickHandlers function in an ExB app's init process; however, we find neither the onBeforeOpen nor onBeforeClose callback gets invoked. Do we miss anything?

Here are more details on why we need to stop event propagation. ExB supports nested layout structure. For example, we can add a Button widget inside a Fix panel widget, and both of these widgets need to be selected. So, when the user clicks the Button widget to select it, we have to stop the click event propagation because if not, the user will select the Fix panel widget.

[ruantao1989]

Hi, this issue effects the click event in ArcGIS Experience Builder Shadow cast tool.

Hi @jcfranco,
I created a quick demo to show how we use it in EXB: https://codepen.io/ruantao1989/pen/zYVOqwV
Could you please take a look?

Thanks!

[ruantao1989]

I added a demo to show our specific issue: @jcfranco
https://codepen.io/ruantao1989/pen/qBzOWvg?editors=1000

stopPropagationForCalcite.mp4

1. Outside the map and the calcite DOM, we have a container DOM:

    <div id="container" style="border: 2px solid red">
      <div id="mapDiv"> </div>
    </div>

2. Due to the functionality of EXB, we need to prevent event propagation in the container,
   However, this causes the ShadowCast calcite component (color-picker) to fail to open

       /******************************************************
       stopping propagation on click on a container DOM:
       The color picker at the bottom of the ShadowCast component cannot be opened
       ******************************************************/
       document.querySelector("#container").addEventListener("click", (e) => {
         e.stopPropagation();
       });

[jcfranco]

@ruantao1989 If you're able to redirect fake clicks to window without any issue, you could apply a modified version of on of the earlier workarounds:

document.querySelector("#container").addEventListener("click", (e) => {
  e.stopPropagation();
  
  // workaround: redirect a fake click event to window for popover to listen to
  const fakeEvent = new CustomEvent("click", { detail: 1 });
  fakeEvent.composedPath = () => [e.target];
  window.dispatchEvent(fakeEvent);
});

See updated demo.

[qlqllu]

@jcfranco The fake event workaround works. However, I think this solution does not seem very clean, and it may have side effects if an app code listens to the click events on the window. Whenever an app stops the event propagation, it may have this issue and need to find this workaround solution. I still prefer the "capture" phase solution.

Nevertheless, thanks for helping figure out this workaround!

[jcfranco]

@qlqllu Glad to hear the workaround will work for you.

Regarding your other concerns, please refer to point 3 in the above discussion.