fix(modal): Fix modal.service:show() buttons to be an array.
#150
Conversation
fix(modal): Added more documentation. feat(modal): modal.service:show() can now supports HTML in `modalContent`.
stanislavgeorgiev
requested review from
cal-smith,
magedhennawy and
zvonimirfras
as code owners
| @@ -64,7 +64,7 @@ import Modal from "./modal.decorator"; | |||
| <p class="bx--modal-header__heading bx--type-beta">{{modalTitle}}</p> | |||
| </ibm-modal-header> | |||
| <div class="bx--modal-content"> | |||
| <p>{{modalContent}}</p> | |||
| <p [innerHTML]="modalContent"></p> | |||
There was a problem hiding this comment.
I think this is probably okay (I can certainly see some use cases), but a ng-template output should probably be added as the default way to inject complex html content.
That of course could be added as a feature later, what do you think?
There was a problem hiding this comment.
yea, makes sense! The use case in my head was if I want to show this as content:
Are you sure you want to delete <strong>this item name</strong>
There was a problem hiding this comment.
Yep makes sense, and I think we already support that for banners/notifications (which have a similar usage model)
| @@ -82,7 +86,7 @@ export class ModalService { | |||
| * @returns {ComponentRef<any>} | |||
| * @memberof ModalService | |||
| */ | |||
| show(data: {modalType?: string, modalLabel?: string, modalTitle: string, modalContent: string, buttons?: null}) { | |||
| show(data: {modalType?: string, modalLabel?: string, modalTitle?: string, modalContent?: string, buttons?: []}) { | |||
There was a problem hiding this comment.
nice!
if we wanted to get really fancy we could declare a interface ModalButton and type buttons to buttons?: ModalButton[]
There was a problem hiding this comment.
yeap, would definitely add that in the future!
I'm still an Angular NOOB but also really pressed for our project's release in 3 weeks
There was a problem hiding this comment.
Cool, I'll make an issue to note that 👍
|
🎉 This PR is included in version 1.8.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fix(modal): Added more documentation.
feat(modal): modal.service:show() can now supports HTML in
modalContent.