Modal component in CoreComponents closes when clicked outside, may result in data loss

[ArthurClemens]

For simple messages, closing the dialog when clicking the backdrop is fine. However, when used with forms, an accidental click can result in data loss. For this reason, dialogs often employ 'modal' behaviour, requiring an explicit action - typically via the Save or Cancel buttons.

The modal component can easily be improved by introducing a modal boolean and using the value in the phx-click-away attribute:

phx-click-away={not @modal and JS.exec("data-cancel", to: "##{@id}")}

[rhcarvalho]

Data loss could also be caused by the Esc key, or accidentally dismissing the changed form in any other way.

I think a key distinction is whether or not there's any unsaved changes in the form. If you click "New Item" and immediately dismiss the modal, nothing is lost. Similarly, clicking "Edit Item" and not changing anything, the modal can be dismissed without data loss.

The problem is when the form state is changed from its original state, do you agree?

If that's the case, then probably a better solution would consider the form state, instead of being statically baked into the modal markup.

What comes to mind are confirmations like "are you sure ... unsaved changes will be lost...", though design-wise a dialog-in-a-dialog is not great UX either, so I don't have a clear proposal here, sorry.

[ArthurClemens]

I agree that the "unsaved changes" warning is a better fit for forms.

To get this warning automatically, simply return true from the beforeunload callback.
The options for displaying this prompt are non-existent so we don't have to concern ourselves with it. Wiring up a custom dialog could be done but is not worth the effort. The prompt text can't be set: you get what the browser makers have decided.

const handleBeforeUnload = (e) => {
  e.preventDefault();
  // return true; 
  e.returnValue = true; // for browser compatibility
};

window.addEventListener("beforeunload", handleBeforeUnload);
// and also: window.removeEventListener("beforeunload", handleBeforeUnload);

This can be implemented with a hook, using Ecto's changeset to conditionally set the return value , as documented in this blog post by Patrick Thompson.

But the big caveat is that push_patch won't trigger beforeunload (nor popstate), so this method won't fly. And destroy can't be prevented. So I don't have a solution yet.

[rhcarvalho]

Connecting the dots, @mxgrn sent this on the Elixir Forum:

https://elixirforum.com/t/would-an-option-for-a-js-confirmation-dialog-on-closing-a-modal-be-useful/60421/2

https://gist.github.com/mxgrn/cb114e52d2a586aeb36241727ec77ac9

[ArthurClemens]

Let's list all scenarios:

Types of dialog

• Dismissable dialog
  • No valuable information gets lost by closing it
  • Can be closed by clicking on the backdrop (next to Escape, Save, Cancel, etc)
  • Example: annoying "subscribe to newsletter"
  • Navigating away closes the dialog
  • Is currently used in core_components for modal
• Confirmation dialog (not a browser prompt)
  • Uses modal behaviour: can only be closed by interacting with the controls on the dialog (Escape, Save, Cancel, etc)
  • Typically used for "Are you sure" messages when deleting a resource
  • Navigating away closes the dialog (counts as a "Cancel")
• Data dialog
  • May suffer data loss when the dialog is closed before saving
  • May optionally use modal behaviour, mainly to prevent an accidental click on the backdrop
  • Navigating away closes the dialog, but additional measures are needed to prevent data loss, typically after detecting that data has been changed (no use to warn if no changes were made).

Data dialogs and data loss

Data loss happens when the user has entered data in the dialog, and through interacting with the dialog or the browser, the dialog with the data gets removed.

Standard dialog interaction:

• Clicking the backdrop when behaviour is not modal
• Using the Escape key
• Any link in the dialog

Browser interaction:

• Using browser buttons (typically "Back")
• With a page reload

Preventing data loss

For standard dialog interaction: adding modal behaviour, not showing navigable links, etc.
For browser interaction: currently limited.

With traditional HTML pages, you would listen for the beforeunload event to prevent navigation away (returning true results in a standard browser prompt). With JS applications, you would listen for router changes to prevent a route change (for example, React Router uses history.block, where history is a custom JS library by remix-run) . In a typical JS application, you would use both.

Currently, listening for beforeunload only works for a page refresh. It doesn't work with other browser interactions. I would love to know if there is a way to intercept and block a route change.

[robkane1]

Just thought I would add here as I recently encountered a similar issue, when rendering an Uppy instance via a hook inside a modal. Because the dom elements interacted with were not present when phoenix loaded the component, interacting with it closed the modal and left the Uppy app in an incomplete state. I suspect that this is an implementation detail of phx_click_away. Having implemented these things myself before, they are a pain to get right and I love the fact that Phoenix provides this out of the box, so I understand that they don't meet all use cases.

The simple work around was just to add an attribute to the modal component that disables click away behaviour for edge cases where this isn;t wanted.

Usually, If I have a complex form, I don't put it in a modal, because one of the best things about reactive UIs is that you don't need to layer things on top of each other.

Edit: Point I am making (poorly) is that core components can be changed as needed to satisfy your needs.