search

jofftiquez / vue-croppie

Vue wrapper for croppie
https://jofftiquez.github.io/vue-croppie/
MIT License
260 stars 42 forks source link

Add croppieInitialized method call #59

Closed papa-zulu closed 5 years ago

papa-zulu commented 5 years ago

The croppieInitilaized() method can be used to resolve the issue with server side rendering. https://github.com/jofftiquez/vue-croppie/issues/38

The passed method will fire when the croppie instance and reference are created.

nicod-pc commented 5 years ago

That's a feature we also like to have. It would be nice when this is merged soon.

jofftiquez commented 5 years ago

Thanks for this @papa-zulu

dobromir-hristov commented 5 years ago

Why a method? An event would have been more than enough here...

papa-zulu commented 5 years ago

In my opinion, methods are a much cleaner solution than events.

dobromir-hristov commented 5 years ago

Depends, this is usually a more of a React'ish pattern, than a Vue one. As you know in Vue the preferred way is events, unless you want to pass data from the function, but that is not the case.

Plus events can be debugged in the Vue devtools.

Just my two cents here.

nicod-pc commented 5 years ago

@dobromir-hristov in your expectation it should be available like this: <vue-croppie v-on:croppieInitilaized="myFunction"> or <vue-croppie @croppieInitilaized="myFunction"> And after the component was initialized this.$emit('croppieInitilaized', something); is called? That looks actually better in my opinion, if it is working.

dobromir-hristov commented 5 years ago

Correct, its the same behavior.

I am not saying the upper is wrong, just asked why it was done like that, if there was an intention or was it just preference. As we saw, it was preference, which is OK, as long as the library author is fine with it :)

jofftiquez commented 5 years ago

@dobromir-hristov @immondu @papa-zulu I read your discussion. I agree that using a method is not wrong, however, an event would comply more on Vue's standards. Perhaps we could do another PR for this? Thank you all.

dobromir-hristov commented 5 years ago

Changing it would cause a breaking change, not sure how many people use it now.

jofftiquez commented 5 years ago

Or perhaps we could add an event counterpart then deprecate the method later?

dobromir-hristov commented 5 years ago

Technically that is the proper way, but its funny because it has been out for what, 1 week? 😆

jofftiquez commented 5 years ago

@dobromir-hristov I know! :laughing: :rofl: