BUG: Click outside of the node creation dialog closes the dialog

[mhsdesign]

I think making this decision is to simple and a bit unexpected user behavior. If you fill out a more complex dialog and accidentally click into the darkened space around, it is immediately closed without asking what to do with the pending changes.

I would rather:
A: Do nothing (one should use the escape key or the "back" button)
B: Show an popup warning that we will now proceed to close

... Fun fact, due to my nervous clicking, this bit me in a customer presentation ^^

[patricekaufmann]

Oh yes. It happened a lot of time to me aswell, especially when selecting the text in the text inputs and releasing the mouse outside of the dialog.

I think having the NodeType Selection dialog still be closable by mouse would be nice as sometimes I have the wrong element selected when trying to insert a node. For this dialog, having it only trigger the close action when the MosueDown happened outside of the dialog aswell would be a welcome addition for these cases.

[mhsdesign]

Oh yes. It happened a lot of time to me aswell, especially when selecting the text in the text inputs and releasing the mouse outside of the dialog.

Bildschirmaufnahme.2023-07-08.um.10.11.42.mov

well it happened again in another presentation 😂

[mhsdesign]

@grebaldi do we have the necessary infrastructure for a simple fix? If yes it would be cool if you could point me into the right direction.

Im suggesting a "you have pending changes" overlay which needs to be confirmed.

[Sebobo]

@mhsdesign isn't it enough to add a check inside onRequestClose which is called when pressing escape or clicking outside?

[mhsdesign]

Thanks for the tipp, @Sebobo didnt know we luckily make this distinction. Is this approach as you imagined? #3573

[patricekaufmann]

It would still lead to the popup closing when, for example, the user tries to select text inputs for properties with defaultValue set and then releasing the mouse outside. It would still be annoying occasionaly because the text inputs are so close to the left popup edge that it will happen from time to time.

The origin for this behaviour is the onClick listener in dialog.tsx, because ev.target only refers to the element where the MouseUp event triggered, eg. the target for MouseDown could have been the popup, and not the sorrounding section.

    private readonly handleOverlayClick = (ev: React.MouseEvent) => {
        if (ev.target === ev.currentTarget) {
            this.props.onRequestClose();
        }
    }

[Sebobo]

@patricekaufmann why would that still happen when releasing the mouse? onRequestClose is called and we would have a dirty check in there.

[mhsdesign]

i think we should go with the solution #3573 and display a orange boarder around the nod creation dialog when one tries to escape without pressing the "back" button.

[patricekaufmann]

I refer to a state in which the values haven't changed yet. When the user first interacts with a input field with defaultValue set, the state is not dirty yet. Then again, it's probably more like an edge-case.. hard to tell if people use text inputs with defaultValues in the creation dialog. However it might be good to cover these edge-cases while at it?

[mhsdesign]

Thanks for defending your point ;) But I think we should focus mostly on pending changes as this ist annoying to have to fill out again.

It seems you’re describing a second strongly related problem / improvement. And I do understand your point and also agree that this is discussable behaviour, but let’s discuss this separately so we can fastly move forward with the main objective;)

[patricekaufmann]

To add to the previous post, the MouseUp / MouseDown mechanic also applies to other dialogs:

jfgR0lk4O5.mp4

When the MouseDown event triggers on the Button and MouseUp on the outside overlay section, it should not trigger the close event.

Edit:

You are absolutely right. While related to this issue, it probably needs attention in a separate issue :) Btw, I love that orange border!

[mhsdesign]

I couldn't get the flashing to work, but that would be easy:

Bildschirmaufnahme.2023-07-11.um.10.02.53.mov

(alternatively i could send a flash message as well)

Nested dialogs a doable, but they must be registered deep in redux and it would be a more complex bugfix.

[crydotsnake]

I love the solution! First i thought it would be good to have an alert for the editor too as notice. But i think this is already enough.

[Sebobo]

I don't like that it's called "back" and would prefer "cancel".
I understand, that it leads back to the node selection, but my eyes always look for a "cancel" button 😄

But that's a topic for another day. I'm fine with this visual solution for now!

[bwaidelich]

I love it dearly, this is one of my major UX issues and I accidentally closed many large creation dialogs in the past :)
Will this also avoid the dialog from being closed when hitting backspace or escape? (e.g. when fixing an input or escaping from a date editor overlay etc)

And I agree with @Sebobo that "cancel" would be a better fit

[mhsdesign]

@grebaldi and me found an even better solution:

we introduce the same shake animation as in the login.

Bildschirmaufnahme.2023-07-11.um.11.59.33.mov

(And yes escape is covered / prevented as well)

[crydotsnake]

@grebaldi and me found an even better solution:

we introduce the same shake animation as in the login.

Bildschirmaufnahme.2023-07-11.um.11.59.33.mov
(And yes escape is covered / prevented as well)

Good idea! But I would still make the border orange. The green does not fit so well in this case.

[mhsdesign]

Good idea! But I would still make the border orange. The green does not fit so well in this case.

1. Orange if there are pending changes in general?
2. Or always orange?
3. Or if there are pending changes and one clicks outside?

If you are for approach 1 or 2 i would rather put it into another improvement/fix pr together with sebs proposed renaming:

I don't like that it's called "back" and would prefer "cancel".
I understand, that it leads back to the node selection, but my eyes always look for a "cancel" button 😄
But that's a topic for another day. I'm fine with this visual solution for now!

[crydotsnake]

Good idea! But I would still make the border orange. The green does not fit so well in this case.

1. Orange if there are pending changes in general?

2. Or always orange?

3. Or if there are pending changes and one clicks outside?

If you are for approach 1 or 2 i would rather put it into another improvement/fix pr together with sebs proposed renaming:

3 of course;)

[mhsdesign]

Like this @crydotsnake ?

Bildschirmaufnahme.2023-07-12.um.09.57.42.mov

initially i was about to always prevent any accidental closing independent of pending changes

but @bwaidelich requested on slack

I'd be fine with skipping the "trap", if there were no inputs

so now i only prevent the closing via escape and mouse-click if there are pending changes as shown

[crydotsnake]

Like this @crydotsnake ?

Yes. This looks way better IMO.

[bwaidelich]

but @bwaidelich requested on slack

I wouldn't say I requested it, but I think it makes a lot of sense to be able to quickly close a modal if I accidentally opened it so +100 for the current implementation! <3

[ahaeslich]

I know I'm a bit late to the game but I'm not sure the following part of the change was a wise choice:

prevent the closing via escape

Why? I now don't have the opportunity to simply escape this modal if I really do want to discard my changes. IMHO this use case was overlooked in your discussion.

Imagine we would have more steps than the NodeType selection and the edit form. I know @Sebobo thought about this. How would you cancel your edit mode without clicking the "back" button multiple times in the worst case?

If I'm an impatient person, which I personally can be 😉, I could only escape this situation by reloading the backend. Is that really good UX?

[mhsdesign]

I know I'm a bit late to the game but I'm not sure the following part of the change was a wise choice:

hmm yes indeed. 🤔 ^^

Im fully convinced to not let people off the hook that easily when they have pending changes in the node creation dialog.
(Just fyi - you can create really complex wizards like with 10 properties, where its just super annoying to not be asked)
So i rather sacrifice the holy escape key (which sadly as a mac user doesnt escape always stuff in general - but i get you as a windows user)

As this is released with https://discuss.neos.io/t/neos-ui-bugfix-releases-8-3-3-8-2-11-8-1-11-8-0-14-7-3-19/6362 already i like to propose to move any follow up discussions (and i dont fully object, its just that we had definitely enough votes ^^) into a separate issue. Okay?

[ahaeslich]

As this is released with https://discuss.neos.io/t/neos-ui-bugfix-releases-8-3-3-8-2-11-8-1-11-8-0-14-7-3-19/6362 already i like to propose to move any follow up discussions (and i dont fully object, its just that we had definitely enough votes ^^) into a separate issue. Okay?

Ah I missed it was already released. There you go: #3582