Missing confirm to exit unsaved binding changes

[oliviertassinari]

Duplicates

• [X] I have searched the existing issues

Latest version

• [X] I have tested the latest version

Steps to reproduce 🕹

Steps:

1. Open https://master--toolpad.mui.com/_toolpad/app/cld06azhx0001k59tpypy3aoy/pages/mu03mbo
2. Select the Text element
3. Click on "Update value binding"
4. Make a change
5. Press Escape

Same behavior for:

• custom components editor
• page module editor

Current behavior 😯

No confirmation to exit.

Expected behavior 🤔

Confirmation before exiting.

Context 🔦

I lost some of my changes because of this behavior. I press escape to hide this popup:

expecting that it would hide it? But instead, it exited the binding modal. This is how I found this regression (I assume it's one). And yes, it would be great for an escape to not trigger the exit action in this case, as it does with this different popup:

Your environment 🌎

v0.0.35

[Janpot]

Possible road to a solution: disableEscapeKeyDown on the dialog, when the monaco editor has focus, or, if possible, only when it's displaying an autocomplete menu.

We could potentially also think about an extra confirmation when the dialog closes without saving.

[oliviertassinari]

confirmation when the dialog closes without saving

I have added the "regression" label because I thought it was behaving like this before. So I think that this one would be a great start since the data isn't saved live.

[Janpot]

I have added the "regression" label because I thought it was behaving like this before.

This dialog hasn't had a confirmation before. it's not a regression, but worthy of an integration test anyway

[prakhargupta1]

The binding editor also disappears when you click on the background. During the meeting, we also saw that somehow the alert box was not working correctly.

[apedroferreira]

the alert box was not working correctly

Can you specify more about this please? I couldn't get some parts of the discussion because my connection has been so bad :D

Edit: Ok I just noticed that while leaving a code component editor with unsaved changes, the confirmation popup has a weird behavior when we press "Cancel". Might be related with the recent changes to navigation.

[Janpot]

Known problems:

• The code component editor navigates away after pressing cancel
• After going back to the component editor, the cancel alert comes twice, even when no modifications are made
• The query editor: I should be able to press cancel in the dialog without getting the confirmation
• The page module editor: missing confirmation
• The url parameters editor: missing confirmation
• The json prop editor: missing confirmation
• ?

[apedroferreira]

~~Thanks! My current approach should be able to fix all of these. But I'm also seeing if I can split the DomLoader in two separate files/concepts, because since we added undo/redo it's also handling "app state" (selections, current view) instead of just the DOM save state.~~

Edit: Oops, I meant this comment in the PR, it's a bit unrelated to the issue itself.

[apedroferreira]

The query editor: I should be able to press cancel in the dialog without getting the confirmation

It feels useful to me that a draft query (one that hasn't been saved for the first time yet) shows a confirmation to the user if there are unsaved changes? So I won't change anything for queries unless I misunderstood.

[Janpot]

My reasoning would be that by pressing "cancel", the user did a conscious action to discard the unsaved work and close the editor. Unlike clicking the background, or a browser gesture, or the Esc key. Which may have been activated inadvertedly. So a confirmation on that specific action only adds friction in most of the cases. But this could be opinionated.

[apedroferreira]

My reasoning would be that by pressing "cancel", the user did a conscious action to discard the unsaved work and close the editor. Unlike clicking the background, or a browser gesture, or the Esc key. Which may have been activated inadvertedly. So a confirmation on that specific action only adds friction in most of the cases. But this could be opinionated.

Ah, I see your point, because clicking "Cancel" is a bit more explicit than just switching to a different view or clicking the background. Ok, we can start by assuming than clicking "Cancel" is always intentional, I think that's fair.