alvarotrigo / vue-fullpage.js

Official Vue.js wrapper for fullPage.js http://alvarotrigo.com/vue-fullpage/
GNU General Public License v3.0
1.85k stars 255 forks source link

Scrolloverflow breaks Vue - components inside section/slide broken due to DOM changes. #49

Closed kowalej closed 6 years ago

kowalej commented 6 years ago

So I noticed when using vue-fullpage with scrollOverflow: true that sometimes after resizing the screen or explicitly calling the reBuild() event from the API my components would no longer respond to user interaction and could not be found when inspecting them using the Chrome Vue Devtools.

I ended up realizing that the scrollOverflow script basically creates or destroys the scroll wrapper object depending on whether or not it's needed for the current screen/content size. I narrowed down the breaking code to line 2499/2500 (see https://github.com/alvarotrigo/fullPage.js/blob/master/vendors/scrolloverflow.js):

    //unwrapping...
   $('.fp-scroller', element)[0].outerHTML = $('.fp-scroller', element)[0].innerHTML;
   $(SCROLLABLE_SEL, element)[0].outerHTML = $(SCROLLABLE_SEL, element)[0].innerHTML;

This results in the .fp-scroller and .fp-scrollable divs from being removed from the DOM and your main content section will appear back in it's "original" location in the DOM (hence the term "unwrapping"). I think the problem though, is that Vue does not jive well with this unwrapping business, where the component is essentially lost since you are removing it's parent from the DOM.

My (kind of bad) solution I cooked up quickly was to not "unwrap" the sections, but instead swap the scroll related classes for "temp marker classes".

    //unwrapping...
    $('.fp-scroller', element)[0].parentNode.className = 'fp-scrollable-temp';
    $('.fp-scroller', element)[0].parentNode.children[0].className = 'fp-scroller-temp';

Then I simply swap back to the original classes upon the next creation event (if needed) - by modifying the "remove" function (starting line 2486)::

//needs scroll?
                    if ( contentHeight > scrollHeight) {
                        //did we already have an scrollbar ? Updating it

                        if(scrollable != null){
                            if($('.fp-scroller-temp', element)[0] != null) {
                                $('.fp-scroller-temp', element)[0].parentNode.children[0].className = 'fp-scroller';
                                $('.fp-scrollable-temp', element)[0].parentNode.children[0].className = 'fp-scrollable';
                            }
                            scrollOverflowHandler.update(element, scrollHeight);
                        }
                         //creating the scrolling
                        else{
                            if($('.fp-scroller-temp', element)[0] != null) {
                                $('.fp-scroller-temp', element)[0].parentNode.children[0].className = 'fp-scroller';
                                $('.fp-scrollable-temp', element)[0].parentNode.children[0].className = 'fp-scrollable';
                                scrollOverflowHandler.update(element, scrollHeight);
                            }
                            else {
                                if(self.options.verticalCentered){
                                    fp_utils.wrapInner($(TABLE_CELL_SEL, element)[0], wrap.scroller);
                                    fp_utils.wrapInner($(TABLE_CELL_SEL, element)[0], wrap.scrollable);
                                }else{
                                    fp_utils.wrapInner(element, wrap.scroller);
                                    fp_utils.wrapInner(element, wrap.scrollable);
                                }
                            }
                            scrollOverflowHandler.create(element, scrollHeight, self.iscrollOptions);
                        }
                    }

After making these changes I am able to resize my window and call reBuild() all I want and nothing Vue related breaks. The scrolling also works perfectly with overflow items dynamically becoming scrollable where needed. Hopefully you can confirm my issue here and provide a nicer solution then presented here (as I am not a Javascript or Vue expert).

chrtz commented 6 years ago

Not sure if this is 100% related. But I noticed, that my slides won't be scrollable until I resize the viewport (by resizing the browser window or turn my smartphone into landscape).

alvarotrigo commented 6 years ago

Thanks for reporting it!! And apologies for the delay! I was on holidays :)

I think the problem though, is that Vue does not jive well with this unwrapping business, where the component is essentially lost since you are removing it's parent from the DOM.

I'm not quite sure to understand this. The component's parent is always whatever you use on the el property in new Vue({.

Why changing the content of a slide make any difference? (And I'm asking from the ignorance! :) )

alvarotrigo commented 6 years ago

Not sure if this is 100% related.

@chrtz Easy to test then. Remove scrolloverflow by using the option scrollOverflow:false. You still having the issue? => then is not related with this topic.

alvarotrigo commented 6 years ago

@kowalej I'm not able to reproduce your issue tho. Look at this codepen (https://codepen.io/alvarotrigo/pen/yxbbbJ).

Scroll to section 2. Resize it down to create the scroll bar. Then make the the viewport big again to remove it.

It still working as expected?

kowalej commented 6 years ago

@alvarotrigo this sample worked fine, however the problem occurs when your fullpage sections (divs marked with the "section" class) have Vue components inside them. This results in the Vue components being wrapped in fp-scrollable and fp-scroller class divs. These divs are sometimes removed and re-added during resizing which seems to break the Vue components inside.

alvarotrigo commented 6 years ago

Right i see...

Have you considered using the fullpage.js callback afterRebuild in order to initialise those components again? I've noticed it is not documented, but it's available.

kowalej commented 6 years ago

Haven't tried this method, it sounds possible but not very maintainable. I did solve the issue with the code in the original comment, basically instead of unwrapping the component (deleting parent fp-scrollable/scroller divs) I just change their classes temporarily. By the way, check out this thread, it sounds related to what's happening here - https://forum.vuejs.org/t/any-way-to-get-rid-of-root-element-in-component-template/1620/8

alvarotrigo commented 6 years ago

@kowalej mmm yeah, maybe that can do as well... Please feel free to create a pull request with your suggested solution in the fullPage.js repository.

Oh, and if any classname is used more than once, just add it on the part starting by // keeping central set of classnames and selectors in the scrolloverflow.js file.

chrtz commented 6 years ago

@alvarotrigo scrollOverflow: false still doesn't work in my case – so not related. thanks.

alvarotrigo commented 6 years ago

@kowalej I was just thinking... your solution might work when you need to remove the scrollbar. But... you would still have the previous error when you have a section/slide without scrollbar that, when resizing it down, it requieres scrollbar by scrolloverflow.

That would make DOM changes and your components wouldn't work anyways, right?

kowalej commented 6 years ago

I don't think wrapping the content messes anything up, after all, many sections on my site do have the scrollable wrapper appended initially without issue.

alvarotrigo commented 6 years ago

I think the problem though, is that Vue does not jive well with this unwrapping business, where the component is essentially lost since you are removing it's parent from the DOM.

@kowalej so... you can't unwrap the component in the section by removing the parent, but you can wrap it moving the parent to a completely new DOM element?

If that's the case, why don't you just create a new wrapper in the section that will never get removed? For example:

<div class="section">
    <div class="my-never-removed-parent">
        <div> content here </div>
    </div>
</div>

This way you won't have to change a single line of fullpage.js as it will end up like:

<div class="section">
    <div class="fp-scrollable">
        <div class="fp-scroller">
            <div class="my-never-removed-parent">
                <div> content here </div>
            </div>
        </div>
    </div>
</div>