BUGFIX: Limit click-outside handling for dialogs to their semi-transparent overlay

[grebaldi]

fixes: #2925

The problem

As documented in #2925, our click-outside-handling for dialogs is a tiny bit too aggressive, which leads to problems with stacked dialogs.

The solution

I removed the enhanceWithClickOutside decorator from the <Dialog/>-component and replaced its behavior with a simple onClick-handler that is limited to the semi-transparent overlay that surrounds every dialog.

The other issue described in #2925 (overlays do not cover entire user interface) is being addressed over here: #3491

EDIT: Some more changes were necessary to apply the same behavior to when the escape key gets pressed. A detailed description of the reasoning behind those changes can be found in this comment: #3492 (comment)

[crydotsnake]

It works great with mouse clicks! But if i press the escape (esc) key on my keyboard, it closes all overalys. Can we intercept the escape key as well?

[grebaldi]

Very good point! I'll take a look at that :)

[grebaldi]

Well, interesting: I looked at the Escape-Key handling for the dialogs and found that since 2018 react-close-on-escape was supposed to be replaced by a vanilla solution (see: #2274).

Yet, somehow react-close-on-escape managed to sneak back in. I'll try to restore the intent of #2274 within this PR, but as of yet I have no idea what happened there.

Maybe it has something to do with the large esbuild change (but that's a wild, wild guess).
EDIT: I wasn't thinking straight here... esbuild cannot be the cause, since we're on 7.3 🤦

/cc @mhsdesign @Sebobo @markusguenther

[mhsdesign]

you deserve a medal for b13d470 ;)

we definitely make sure that this doesnt happen with esbuild (despite the different import syntax, but didnt think the webpack stack had any obvious flaws like this one ^^)

---

Some modules referred to the components via

import { /* ... */  } from '@neos-project/react-ui-components';

This caused webpack to pull the components from
'@neos-project/react-ui-components/lib-esm' rather than
'@neos-project/react-ui-components/src', leading to the
component modules being bundled twice.

The solution was to change those reference to an explicit:

import { /* ... */  } from '@neos-project/react-ui-components/src';

Now the react-ui-components are only bundled once.

[grebaldi]

This was more than I thought :)

My plan was to have some global entity that can ensure a global order of stacked dialogs and close them in LIFO order when the escape-key is hit.

Plan didn't work at first try, because it turned out that webpack was bundling the react-components twice. Plugins would receive a different version of the components than the main UI - thus also a different version of the global stack-management entity.

So, I looked into that... Problem was that some modules in the codebase referred to the components via

import { /* ... */  } from '@neos-project/react-ui-components';

This caused webpack to pull the components from '@neos-project/react-ui-components/lib-esm' rather than '@neos-project/react-ui-components/src', leading to the component modules being bundled twice.

The solution was to change those references to an explicit:

import { /* ... */  } from '@neos-project/react-ui-components/src';

Now the react-ui-components are only bundled once.

With that, I introduced the DialogManager class, which takes care of managing the closing behavior of dialogs when the escape key is pressed.

I then proceeded to also remove react-close-on-escape as well as react-portal from the dialog component. The former because it wasn't supposed to be there in the first place and the latter because react-dom has its own Portal-API, so react-portal is no longer needed.

After this, react-portal is only used in @neos-project/neos-ui somewhere in the Inspector implementation. Removing it there should be done in a separate PR though.

Long story short:

But if i press the escape (esc) key on my keyboard, it closes all overalys. Can we intercept the escape key as well?

Yep, that works now :D

[crydotsnake]

Thanks a lot for the improvements! Everything works as expected 💙

[mhsdesign]

Thanks for the adjustments @grebaldi ;) I will try this out locally soon.

of topic a bit, but what do you think about #3531, linking it here because its related.
We can continue discussing that over there ^^

[bwaidelich]

Sorry for being too lazy to test, but will this also affect the behavior of non-stacked dialogs?
So often I fill out a large creation dialog and close it by accident because I click next to it (or hit backspace to fix an input)

[grebaldi]

@bwaidelich
I'm afraid not. I've seen that #3531 has been reported (which I believe includes your description - except for the "backspace"-part). This PR only covers #2925, which came up during Media UI development.

I totally agree that #3531 needs to be addressed. Not sure whether this PR is the place to do so, though.

[bwaidelich]

I'm afraid not. I've seen that #3531 has been reported

Ah right, I didn't see that.
I'll try to reproduce that backspace-issue and comment it there, thanks!

[mhsdesign]

Thanks ;) Works as expected!