appuniversum / ember-appuniversum

Ember addon wrapping the appuniversum components.
https://appuniversum.github.io/ember-appuniversum
MIT License
14 stars 11 forks source link

Addition of adjustable close triggers & closable boolean for modal component #411

Closed brenner-company closed 1 year ago

brenner-company commented 1 year ago

Within Kaleidos we could use the following functionality for the AuModal component:

I've made a draft PR as a proof-of-concept that can be checked out if needed.

Windvis commented 1 year ago

This might be related to this issue? https://github.com/appuniversum/ember-appuniversum/issues/265

I find it strange to allow some ways to close the modal, but not all. In my mind it makes more sense to disable closing completely, or allow all the close options, but I haven't researched this. What's the use-case in Kaleidos that requires more fine-grained control?

@Dietr Any opinions?

brenner-company commented 1 year ago

I think this request originated from users accidentally clicking outside the modal and losing input, very similar to the issue (https://github.com/appuniversum/ember-appuniversum/issues/265) you mentioned above.

As mentioned here https://github.com/appuniversum/ember-appuniversum/pull/412#discussion_r1276114793, I think also making the close button (within the modal header) configurable could be a bit of overkill.

The dialog or prompt element seems a possible solution to that, but I'm afraid they will just misuse that component (so that it contains the same content as they would with a modal now).

brenner-company commented 1 year ago

Our team lead mentioned today that there is a bit of urgency regarding this feature as this is blocking the finalization of a new part of Kaleidos.

As I am going to be away until next week, is it ok if I let Tom De Nies know he can contact you for a possible follow-up?

Dietr commented 1 year ago

I would go for the simplest solution and just remove the outside click on all modals. In a native dialog component there's also no outside click by default so might be good to follow that pattern.

We could also refactor the modal to a dialog component in the future.

tdn commented 1 year ago

@Dietr this would be a good solution to start with.

The other issue we're having is that the modal can be closed while we're performing certain save functions that need to be completed sequentially and completely. (It's just the way our backend works)

We already disable the buttons inside the modal, but clicking on the backdrop or the "X" icon still interrupts the process. So removing the background click handler solves part of the issue, especially for accidental closing of the modal, but we'd need to be able to disable the 'X' button & the ESC handler as well. So basically disable closing completely, as @Windvis suggested.

Thanks!

brenner-company commented 1 year ago

FYI: I'll adjust the draft PR accordingly when I get back next Tuesday ✅