search

eolant / vuetify-toast-snackbar

Basic Vue toast service that uses Vuetify Snackbar component.
MIT License
128 stars 36 forks source link

Added `classes` property to provide static CSS classes #8

Closed Devessier closed 5 years ago

Devessier commented 5 years ago

This would permit to integrate easily the component in different environments.

I was adding vuetify-toast-snackbar (what a nice plugin !) to a personal web application, when I discovered that the Vuetify default style for toasts was not graphically what I expected for.

Being able to supply such a thing would solve my problem and my web app would look so clean ! 😄

eolant commented 5 years ago

I don't believe it's necessary since if you don't like general style of Vuetify Snackbar component it's possible to style them using existing css classes. Plus in your commit you have two class properties on the component.

Devessier commented 5 years ago

if you don't like general style of Vuetify Snackbar component it's possible to style them using existing css classes Yes, I could do, but, it wouldn't be conformed to good practices : if later I want to use the Vuetify snackbar component not as part of your library, these components would inherit the classes I would have defined globally. Then, yes, I could apply a "countermeasure" by specifying the classes I want to be applied for these components only, but, why make it complicated when it can be simple ? Just adding a list of static classes when calling $toast would be a nice and elegant way to solve this problem.

you have two class properties on the component Furthermore this is a totally legit and valid pattern with Vue.js to have both an attribute class containing static CSS classes and an attribute class which is bound to a variable. Vue.js will merge the both and apply the final result.

Have a nice day ! 😄

eolant commented 5 years ago

@Devessier valid points. Another concern I have is the type of the class property as it can be also an array or an object if I'm not mistaken?

Devessier commented 5 years ago

@eolant I thank you for your reply. Effectively, it's possible. My first commit focused on passing only strings but, according to your suggestions, I am going to push a new commit with these features.

I am waiting for your feedback !