-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(#19): support HTML content #27
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
I think an html
property in notifyOptions
should be better. You keep the given text, and html could be displayed additionaly if html
property is not null.
Something like this :
export type CreateNotifyOptions = {
text: string;
html?: string;
level?: Level;
location?: VSnackbar['$props']['location'];
notifyOptions?: VSnackbar['$props'];
};
Moreover, you need to add html
property to index.d.ts
, types.ts
for allowing autocompletion in projects using this library.
@ThomasLeconte Updated MR according to your comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ramseyfeng, thanks for your work.
In my opinion, notifyInfo
, notifyWarning
, notifyError
and notifySuccess
are fast ways to make notifications. HTML support is not a common use for snackbars. You can keep htmlContent
in CreateNotifyOptions
, but it's not necessary in all shortcuts functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ThomasLeconte The PR has been updated again. :p
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice ! I will test it soon and I will prevent you when next release is available 😉 !
Hi @ramseyfeng, |
@ThomasLeconte That's great! |
Adding support for HTML content display.