Clarity: Modal is also dismissed when pressing the ESC key to close a combobox's dropdown panel.

Created on 2 Oct 2020  ·  5Comments  ·  Source: vmware/clarity

Describe the bug

I have a combobox inside a modal but when you try to press the ESC key to close the it's dropdown panel, the modal is also get dismissed.

modals

How to reproduce

https://stackblitz.com/edit/clarity-v4-light-theme-jz6if2?file=src/app/app.component.ts

Steps to reproduce the behavior:

  1. Toggle the dropdown panel of the combo box.
  2. Press ESC key.

Expected behavior

It should only dismiss/close the items panel/dropdown panel of the combo box and not the modal.

Versions

4.0.2

App

  • Angular: 10
  • Node: 12
  • Clarity: 4.0.2

Device:

  • Type: [e.g. MacBook]
  • OS: [e.g. iOS]
  • Browser: Chrome/Edge Chromium
  • Version [e.g. 22]

Additional notes

Also, you will notice that there's a console error when pressing the ESC key.

combobox dropdown modal has workaround 4 medium-high wontfix backlog bug v4 v5

Most helpful comment

I had a research session based on this scenario last week.
Clarity can not know if a component should or should not stop event propagation. But this can (and should) be handled on the user side. Here is a demo, illustrating how this can be achieved for a modal and dropdown:
https://stackblitz.com/edit/pattern-modal-smart-escape-demo?file=src%2Fapp%2Fsmart-escape.directive.ts
It may currently not work with combobox, because the combobox is emitting an extra "open" event, which is a combobox bug and will be logged separately.

All 5 comments

I know that this is a collision between the combobox/popover code and the modal code that dismisses on escape. But I can't find where that collision is happening. It would require a deeper dive than I have timeboxed. I will put this in the backlog.

I know why it's happening but cannot say where it is happening. Any attempt I made to correct it introduced EHCAIWC errors. 🤷‍♂️

I had a research session based on this scenario last week.
Clarity can not know if a component should or should not stop event propagation. But this can (and should) be handled on the user side. Here is a demo, illustrating how this can be achieved for a modal and dropdown:
https://stackblitz.com/edit/pattern-modal-smart-escape-demo?file=src%2Fapp%2Fsmart-escape.directive.ts
It may currently not work with combobox, because the combobox is emitting an extra "open" event, which is a combobox bug and will be logged separately.

The combobox event bug mentioned above:
https://github.com/vmware/clarity/issues/5553

@Jinnie Could you elaborate a bit more conceptually? Why can't a component decide to stop event propagation? In the case of a popped open dropdown, it seams reasonable to assume that event propagation of ESC can be stopped?

ps: Just trying to get a better grasp of Clarity concepts here :-)

@Jinnie Could you elaborate a bit more conceptually? Why can't a component decide to stop event propagation? In the case of a popped open dropdown, it seams reasonable to assume that event propagation of ESC can be stopped?

ps: Just trying to get a better grasp of Clarity concepts here :-)

As a library we cannot stop event bubbling. We need to make that guarantee so that Clarity users have access to events generated inside a Clarity component when they need it. We don't know when they may be listening. If we stop propagation we can prevent the application from having access to user generated events that they may depend on to do other things besides what our components are doing.

In this case, what @Jinnie is recommending is that the application handle listening for the escape event on the application side. This leaves the application in control of when event.stopPropagation() is used. If Clarity does it, a different app that is listening for the esc event will never hear it.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

gperdomor picture gperdomor  ·  3Comments

vzayko picture vzayko  ·  3Comments

amellnik picture amellnik  ·  3Comments

clane picture clane  ·  3Comments

mayesgr picture mayesgr  ·  3Comments