BUG: Click outside of the node creation dialog closes the dialog about neos-ui HOT 25 CLOSED

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.

from neos-ui.

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

(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.

from neos-ui.

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!

from neos-ui.

Like this @crydotsnake ?

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

from neos-ui.

Like this @crydotsnake ?

Yes. This looks way better IMO.

from neos-ui.

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.

from neos-ui.

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.

well it happened again in another presentation 😂

from neos-ui.

@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.

from neos-ui.

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

from neos-ui.

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

from neos-ui.

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();
        }
    }

from neos-ui.

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

from neos-ui.

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?

from neos-ui.

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;)

from neos-ui.

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

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!

from neos-ui.

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.

from neos-ui.

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

from neos-ui.

@grebaldi and me found an even better solution:

we introduce the same shake animation as in the login.

(And yes escape is covered / prevented as well)

from neos-ui.

@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.

from neos-ui.

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!

from neos-ui.

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;)

from neos-ui.

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

from neos-ui.

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?

from neos-ui.

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?

from neos-ui.

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

from neos-ui.