Closing modal using ModalReference #143
Conversation
There was a problem hiding this comment.
I think this work is awesome @larsk2009! 🎉
I've made a few suggestions around keeping details encapsulated. I think it would be best to have developers only work with the ModalReference and not expose internal details (Close methods) to the outside world.
Also, I noticed you only added the example to the Blazor Server example and not to the Blazor WebAssembly one. If you could add it to both that would be great.
Looking forward to hearing your thoughts.
|
|
||
| /// <summary> | ||
| /// Closes the <paramref name="modal"/> | ||
| /// </summary> | ||
| /// <param name="modal">The <paramref name="modal"/> of the modal to close.</param> | ||
| void Close(ModalReference modal); | ||
|
|
||
| /// <summary> | ||
| /// Closes the <paramref name="modal"/> | ||
| /// </summary> | ||
| /// <param name="modal">The <paramref name="modal"/> of the modal to close.</param> | ||
| /// <param name="result">The result of the modal</param> | ||
| void Close(ModalReference modal, ModalResult result); |
There was a problem hiding this comment.
I don't think we should add these methods to the interface, I think it would be better to keep developers working with the ModalReference only. What do you think?
There was a problem hiding this comment.
I agree with your point here. The reason I added it to the interface is because than the ModalReference can use the interface and doesn't depend on the implementation. But looking at it again, I think there will probably not be another implementation of this interface so than I think it is better to do what you suggested.
|
@chrissainty I forgot to add the example to webassembly. Thanks for reminding me. |
|
@larsk2009 I've just realised, could you add a new page to the docs site for this feature? If you can't for any reason I can add a new issue to do it separately. But it would be great to get it all in together. |
|
I added the docs. First time using docFx so please let me know if it looks good. |
|
The DocFX stuff looks great @larsk2009. I think we're ready to merge! 🥳 Thank you so much for this work. |
This PR closes #120. It adds a
Close()function toModalReference. Optionally aModalResultcan be passed, which I think is handy when having a long-running task that can give some result.I added an example that shows the loading dialog while waiting for 5 seconds, as I think this is what people were trying to make in #120.
I added a new function to the
ModalServicethat can close a modal using theModalReferenceinstead of theModalReference.Id. The reasoning for this function is because I wanted to keep the arguments used between theOnModalInstanceAddedandOnModalCloseRequestedthe same in theModalService. I can understand though if we would rather just use the functions that use theModalReference.Id. Just let me know!