search

maoberlehner / transition-to-height-auto-with-vue

This is an example project for the following article: https://markus.oberlehner.net/blog/transition-to-height-auto-with-vue/
105 stars 37 forks source link

feat: also transition padding and margin #15

Closed WofWca closed 4 years ago

WofWca commented 4 years ago

Closes #14

May be a breaking change. Can't think of an example though. Should bump version to 0.2.0?

WofWca commented 4 years ago

Hey, @yanzijun, do you think it would be a good idea to merge it in your fork? It's been a month already.

whyayen commented 4 years ago

@WofWca In most cases, I just update my fork when this package release new version or merge some pull requests. I think that you can request the author to review your pull request first. The author is busy at work. Maybe he doesn't know you send a pull request yet.

I would review the pull request after work. Because I haven't update the package to the latest version on npm. I think that it's a good idea to update to npm together if the pull request works as expected. Thank you 👍

whyayen commented 4 years ago

@WofWca Thank you for your contributions. I has already test your pull request. You fix the problems of margin and padding. I would merge to my fork and publish to NPM vue-transition-expand package. But I haven't decided which version should I use. I would merge and publish it to NPM tomorrow.

WofWca commented 4 years ago

@yanzijun I think people who manually ensure that their transitioning elements don't have paddings and margins (so their transitions aren't jumpy) would keep doing so after a patch update (0.0.7), so it would not affect them. So we wouldn't lose much by playing it safer at making it 0.1.0.

whyayen commented 4 years ago

@WofWca I appreciated that you spent time on fixing the bugs. I have already merged it to my fork. I published the version of 0.1.0 which included your commits.