SAP / ui5-webcomponents

UI5 Web Components - the enterprise-flavored sugar on top of native APIs! Build SAP Fiori user interfaces with the technology of your choice.
https://sap.github.io/ui5-webcomponents/
Apache License 2.0
1.5k stars 260 forks source link

[SelectMenu] afterClose event is bubbling to its container when it should not #7799

Closed ybesnehard closed 8 months ago

ybesnehard commented 10 months ago

Bug Description

After https://github.com/SAP/ui5-webcomponents/issues/7637 this should have been fixed but not in all cases.

When you are creating a Dialog with a SelectMenu inside it. The afterClose event of the SelectMenu's Popup is also fired on the containing Dialog. It should not be the case.

In our product (SAP Build Process Automation) specific usecase we remove the dialog from the DOM on afterClose event when dialog are dynamically added in the DOM at opening. So on the afterClose of the Menu that is opened in the dialog, the dialog itself is destroyed from the DOM.

This of course could be a more generic event bubbling issue that create this

Affected Component

Eventing / Dialog / SelectMenu

Expected Behaviour

afterClose event should not bubble from the SelectMenu to its Dialog container

Isolated Example

https://codesandbox.io/s/ui5-webcomponents-forked-ykvz69?file=/index.html

Steps to Reproduce

  1. Open the code sandbox
  2. Click on the Open Dialog Button
  3. Click on Open Menu Button
  4. Select one item of the Menu
  5. The afterClose event of the Dialog is triggered (alert display message) when it should not.

Log Output, Stack Trace or Screenshots

No response

Priority

Very High

UI5 Web Components Version

1.19.0

Browser

Chrome

Operating System

Windows

Additional Context

We waited 1 month for the fix https://github.com/SAP/ui5-webcomponents/issues/7637 But still happening on SelectMenu, we would really need this fix because it is preventing us to deliver some features on our product (SBPA)

Organization

SAP

Declaration

IlianaB commented 10 months ago

Hello @ui5-webcomponents-topic-p, This issue is similar with #7637 , but it is regarding the ui5-select component. afterClose event of select should not bubble to the dialog. Please, have a look.

Regards, Iliana

ybesnehard commented 9 months ago

Hi @jdichev do you have a ETA for this fix because we would need it to release a certain feature for SAP Build Process Automation ? Many Thanks

ilhan007 commented 9 months ago

Hello @ybesnehard although we fixed similar/same issue for the Menu we once more iterated over this in general.

And, we think that stopping events from bubbling is too constraining and decided not to do it and let the events bubble. For cases like yours we recommend (and will have to document it) checking the event target and react accordingly. This is also how you would handle same cases with pure HTML, for example checking on the event target of click event to see where it originates.

What do you think about it, I hope it makes sense for you?

*meanwhile the assigned colleague @dobrinyonkov will create an example how this would look like.

dobrinyonkov commented 9 months ago

Hello @ybesnehard,

We have discussed this issue, and as mentioned we propose not stopping the events from bubbling but doing an event target check. Please find @ilhan007's comment above.

I've modifed your example with the provided solution: https://codesandbox.io/p/sandbox/ui5-webcomponents-forked-cqz7w2

Inside the listener, I have implemented a check to verify if the event's target is directly from the dialog. When the event's origin is the dialog, the alert message is shown.

Kind Regards, Dobrin

ybesnehard commented 8 months ago

Hi again, I find this very constraining to have to do the target check in the afterClose event handler that should not be even triggered in the first place because the dialog is not closing, just another component inside it.

jdichev commented 8 months ago

Hello @ybesnehard,

Sorry for the inconvenience you're experiencing. This is basically an issue that is not specific to particular web component. When multiple components, firing events of the same name, exist in the same component structure, this is going to happen.

We looked once again into this issue and decided not to stop the event bubbling at this point, since this could also be considered a breaking change for anyone who is using event delegation pattern. This pattern is an efficient way to handle multiple events via a single event handler and seems widely used.

Best regards, Jordan