appsmithorg / appsmith

Platform to build admin panels, internal tools, and dashboards. Integrates with 25+ databases and any API.
https://www.appsmith.com
Apache License 2.0
34.63k stars 3.74k forks source link

[Feature]-[720]:Pressing Escape to close a modal should count as a quick dismissal. #21552

Open ghost opened 1 year ago

ghost commented 1 year ago

Is there an existing issue for this?

Summary

Pressing the Escape button to close a modal should count as a quick dismissal along with clicking outside the modal. Toggling quick dismissal off for the modal should also disable closing the modal with the Escape key.

Why should this be worked on?

We want to control the user experience and not have a user accidentally close a modal when they do not intend to. As such we have specific buttons set up to close the modal and always disable quick dismiss.

If the modal is dismissed prematurely that could result in loss of data when the modal requires the user to input data and that data is not submitted. However incomplete sets of data cannot be submitted, so we cannot save the data to the database on close.

Additionally the modal is contextual based on which button the user presses to show it. For example, we could have 5 orders in a table or a list, and each order has a button to create a shipment. The button pops the same modal, but the modal is populated with data related to the row/list item that it is triggered from.

Pressing escape to close the modal could be done on accident while the user is filling the modal with data (just as clicking outside the modal is an easy mistake to make).

Binding escape to a button makes sense, because then the designer can decide if escape should bind to close modal or some other function.

Barring that, escape should be tied to quick dismiss, so the designer can decide if an easy dismissal is desired (escape or clicking outside) or if they need to make sure the user follows the correct workflow to exit safely.

Front logo Front conversations

somangshu commented 1 year ago

@dilippitchika what would be the impact given 50% of the end users can go about pressing esc for dismissing the modal! This makes the current functionality useless!

dilippitchika commented 1 year ago

So i think the bigger change here is differentiating between a quick dismissal and a confirmed dismissal, there can be different techniques and not just esc, some people might want clicking outside as a quick dismissal as well.

Until then, the developer can choose to use the onClose event to store the values in appsmith store by default and ask the end user to discard before dismissing if they don't want values to be stored

somangshu commented 1 year ago

@dilippitchika the case is different here, the quick dismissal property works for click outside, it only does not work for esc key, both need to have a similar behavior!

extremegf commented 1 year ago

I do not see any use case for

Quick dismiss: Off

If the user can press Esc to close modal, we might as well let him click overlay to dismiss it.

somangshu commented 1 year ago

What do you mean @extremegf ?

One very straight-forward use case we have seem before is where the app devs wants to force the user to take an action on a modal and not be able to dismiss

extremegf commented 1 year ago

One very straight-forward use case we have seem before is where the app devs wants to force the user to take an action on a modal and not be able to dismiss

All the user has to do to dismiss such modal is to press Esc. All modals made via Appsmith can be simply dismissed by pressing Esc. Try it out.

dilippitchika commented 1 year ago

I can see that the data is retained even when user presses esc or explicitly closes the modal. So by default everything is quick dismiss, reset only happens when the developer wants to reset the form.

I did this - Added a table with data, connected it to a modal on row selection, added an input in modal. After closing it irrespective of escape or using close button, the data still persists.

We need to know which input are they using where this issue is happening

somangshu commented 1 year ago

Confused at this point.

The original requirement is

Pressing the Escape button to close a modal should count as a quick dismissal along with clicking outside the modal. Toggling quick dismissal off for the modal should also disable closing the modal with the Escape key.

So the ideal solution is that,

@dilippitchika About the data persisting, we need to manually reset the modal widget with its children to have them in default state.

extremegf commented 1 year ago
  • If quick dismiss is enabled

    • Pressing esc should dismiss the modal
    • clicking on the overlay should dismiss the modal
  • If quick dismiss is disabled

    • Pressing esc should not dismiss the modal
    • clicking on the overlay should not dismiss the modal

Yes, that's right.

somangshu commented 1 year ago

Stats

Stat Values
Reach 360
Effort (months) 0.5

Assuming unique counts in the last 6 months where people have set quick dismiss to false.

melroy-bsk commented 5 months ago

Since there already is a quick dismiss toggle, maybe a toggle "Quick Dismiss - Escape Key", that listens to "onkeypress" for Escape button.

eedwardsSGS commented 4 months ago

Another issue here: if you have a table that has editable fields, a normal habit to cancel out of an edit is to hit escape. However, even if quick dismiss is disabled, hitting escape while editing a field in a table still closes the modal.