aurelia / ux

A user experience framework with higher-level capabilities, designed to bring simplicity and elegance to building cross-device, rich experiences.
MIT License
368 stars 55 forks source link

Modal component and service #255

Closed ben-girardet closed 4 years ago

ben-girardet commented 4 years ago

Here is the PR focused on the modal component and service. I believe that it is ready for final review. It might continue to evolve but I think it's at a state that is robust and useable.

Once merged I would like to use it to finish up the work on the Demo PR.

ben-girardet commented 4 years ago

Regarding Modal VS Dialog comparison. I can point to two things that are different (and might stay different).

1. Dialog has some variants in how we can call the open() method.

Depending on how it is called it will return and handle the returned promise a little differently. I am not a big user of Aurelia Dialog so I don't know how important these variants are. In my opinion I prefer when a service like this has one consistent API. Therefore I would rather stick with a single open() method that works in the same manner all the time.

2. The Dialog attaches the whenClosed() method to the returned Promise<> from the open() api.

This comes handy as it requires one less then() or await but I prefer the way the modalService return a standard Promise pointing to the UxModal ViewModel. When consumed, the open() api requires one more then() or await but then the returned value can do more than just whenClosed() => it gives full access to the ViewModel.

Both of these could lead to breaking changes when switching from Aurelia Dialog to Aurelia Ux Modal but they should not remove any important functionality that I am aware of.

bigopon commented 4 years ago

@ben-girardet looks good to me. I have no further comments for now. Let's go first and continue enhancing this.