search

Zulko / eagle.js

A hackable slideshow framework built with Vue.js
https://zulko.github.io/eaglejs-demo/
ISC License
4.08k stars 221 forks source link

#26 breaks all `slide` conditional rendering #33

Closed yaodingyd closed 6 years ago

yaodingyd commented 6 years ago

26 introduced a fix for #17: if user didn't have a wrapper element for slideshow, when it's mounted, because all slide is set to active: false by default, so its DOM node will be a Comment node, thus slideshow's $el would not have fontSize property.

The fix is simple but dumb: by adding a div for every slide, we can ensure that slideshow will always have a real DOM node, but this also means all slide would be rendered, even if it's not active. This is critial issue 1. Also because all slide s are always rendered, slide would have no transition effect at all. This would be critial issue 2.

The original issue still needs to be fixed. We can note the documentation for now that slideshow must have one wrapper element.

I didn't think it though and I apologize for this...

Zulko commented 6 years ago

So, would you suggest to revert the whole pull request, or only the commit that fixes #17 ?

yaodingyd commented 6 years ago

This commit bd14f5b83c4713c7da1936912e5dfade5339cec9 fixes the issue and I have added notes in slideshow section in README. Although we do need a new npm publish.