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 258 forks source link

Select in Dialog closes Dialog #9484

Closed MarcusNotheis closed 1 month ago

MarcusNotheis commented 1 month ago

Bug Description

When using a ui5-select in a ui5-dialog, the dialog closes when an option is selected.

Affected Component

Dialog, Select

Expected Behaviour

DIalog should stay open

Isolated Example

https://stackblitz.com/edit/vitejs-vite-evkja6?file=package.json,src%2FApp.tsx,src%2Fmain.tsx&terminal=dev

Steps to Reproduce

  1. Open the example
  2. Click on open dialog
  3. Select an option, dialog is closed ...

Log Output, Stack Trace or Screenshots

No response

Priority

High

UI5 Web Components Version

2.0.1

Browser

Chrome, Edge, Firefox, Safari

Operating System

No response

Additional Context

The close event of the select seems to bubble and trigger the close event of the dialog?

Organization

UI5 Web Components for React

Declaration

tsanislavgatev commented 1 month ago

Hi @MarcusNotheis ,

Can you please check this example link, because I currently can't isolate the issue with ui5-webcomponents only?

Can you check if there is something missed while isolating, and if not to confirm that it's only when used with web components for react?

Best Regards, Tsansilav

MarcusNotheis commented 1 month ago

Hi @tsanislavgatev, it only happens if you add an close event listener to the dialog which is closing the dialog. I've made the example in react because there is is common to reset an open state for a dialog on the close event, in vanilla HTML/JS it feels a bit strange. Anyhow, you can reproduce it by adding the following JS snippet to the playground:

dialog.addEventListener("close", () => {
    dialog.open = false;
});
tsanislavgatev commented 1 month ago

Thanks @MarcusNotheis , I am redirecting it to the team!

Hi team @SAP/ui5-webcomponents-topic-p ,

Can you please check this issue? I've isolated it in this example

Best Regards, Tsani

dobrinyonkov commented 1 month ago

Hello @MarcusNotheis,

Thanks for your issue! We had this reported several times already and we are aware of this behavior. However, we aim to keep the components as close to the native behavior as possible. In this case, the select's public event close can be "caught" by a higher level component to perform a desired action, instead of closing the dialog, therefore we still want to keep this possible.

A solution to this on the app side would be to either check if the event target is the dialog and only then close it, or to prevent the event from bubbling up once fired by the select.

ref.current.addEventListener('close', (e) => {
  if (e.target !== selectRef.current) {
    setDialogOpen(false);
  }
});

https://stackblitz.com/edit/vitejs-vite-g82tww?file=package.json,src%2FApp.tsx,src%2Fmain.tsx&terminal=dev

I hope this helps.

Best regards, Dobrin

MarcusNotheis commented 2 weeks ago

Hi @dobrinyonkov, I understand the rationale behind your answer, but I would see this as an unexpected behaviour, especially in the React ecosystem. Every time I would use a ui5-select in any kind of popover or dialog, it would close my modal as long as I'm not prevent the bubbling in the close event handler. This is also somehow contradicting to what @vladitasev answered a couple of years back in this issue.

In addition, in v1 the after-open and after-close events were not bubbling (https://github.com/SAP/ui5-webcomponents/pull/1981), so this would also be worth to be mentioned as breaking change to in the migration guide in case you decide to stick to this behaviour.