Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed dummy section, added checks for dynamic changes #66

Merged
merged 2 commits into from
Nov 19, 2018

Conversation

cmswalker
Copy link
Collaborator

@cmswalker cmswalker commented Nov 10, 2018

@alvarotrigo this should fix #62

and it gets rid of the dummy section

However I'm having issues when it comes to including the scrollOverflow extension, it breaks as it can't find a classList of undefined. Could you look into why this might happen? I left a note here

@alvarotrigo
Copy link
Owner

Thanks for taking a second look at the dummy section issue! 👍

I noticed you are checking for the amount of sections and slides in order to define whether to destroy and re-initialise fullpage.js.

Just one thing, I think you should be checking for .fp-slide or .fp-section in line 61 to 72. Those are the internal classes added by fullPgae.js to identify slides and sections respectively. Bare in mind that .slide and .section are selectors that can be customised for developers by using the fullpage.js options sectionSelector and slideSelector. So .slide or .section won't necessary exist at all.

One question tho. I guess the component wouldn't update automatically when changing other fullpage.js properties like sectionsColor or anchors right?
In that case, is there a way to manually call componentDidUpdate ? (or a new build method?)

We have a similar call in the Vue wrapper for some dynamic changes as well as in the Angular wrapper that still in progress.

@cmswalker
Copy link
Collaborator Author

Ok will update with the internal classnames

One question tho. I guess the component wouldn't update automatically when changing other fullpage.js properties like sectionsColor or anchors right?
In that case, is there a way to manually call componentDidUpdate ? (or a new build method?)

yes, in it's current state it will only fully rebuild if the slide count changes, but you make a good point, perhaps we should also look at all fullpage.js compatible props and perform an update if they've changed. I'll look into it. The alternative would be to perform a destroy() and update() after every render but that could be expensive

@alvarotrigo
Copy link
Owner

The alternative would be to perform a destroy() and update() after every render but that could be expensive

Yeap.

@cmswalker
Copy link
Collaborator Author

@alvarotrigo I've started fixing the root issue for many of these problems. You can see the start of it in this commit f6373aa

If i could get a list of all fullpage.js options and their types, I could fix this fairly quickly. It might be a good separate README to add in general to fullpage.js documentation. Something like:

{
	sectionsColor: Array,
	scrollOverflow: Boolean,
	navigation: Boolean,
	...etc
}

@alvarotrigo
Copy link
Owner

alvarotrigo commented Nov 15, 2018

Looking good!! Thanks for that!

Regarding the types, some can be both String and Array or even String and null. So not sure how you would categorise those. So far, I put them under "Object".
For the callbacks functions I used "Function". Not sure if that's correct either.
And not sure if you need the types of their parameters?

{
   //navigation
    menu: String,
    anchors: Array,
    lockAnchors: Boolean,
    navigation: Boolean,
    navigationPosition: String,
    navigationTooltips: Array, // ['firstSlide', 'secondSlide']
    showActiveTooltip: Boolean,
    slidesNavPosition: String,
    slidesNavigation: Boolean,
    scrollBar: Boolean,
    hybrid: Boolean, 

    //scrolling
    css3: Boolean,
    scrollingSpeed: Number,
    autoScrolling: Boolean,
    fitToSection: Boolean,
    fitToSectionDelay: Number,
    easing: String,
    easingcss3: String,
    loopBottom: Boolean,
    loopTop: Boolean,
    loopHorizontal: Boolean,
    continuousVertical: Boolean,
    continuousHorizontal: Boolean,
    scrollHorizontally: Boolean,
    interlockedSlides: Object, // true, false, [1, 3, 5]
    dragAndMove: Object, //true, false, 'horizontal', 'fingersonly'
    offsetSections: Boolean,
    resetSliders: Boolean,
    fadingEffect: Object, //true, false, 'sections', 'slides'
    normalScrollElements: String, // '#element1, .element2', 
    scrollOverflow: Boolean,
    scrollOverflowReset: Boolean,
    scrollOverflowHandler: Object,
    scrollOverflowOptions: Object,
    touchSensitivity: Number,
    normalScrollElementTouchThreshold: Number,
    bigSectionsDestination: Object, //top, bottom, null 
    
    //Accessibility
    keyboardScrolling: Boolean,
    animateAnchor: Boolean,
    recordHistory: Boolean,


    //Design
    controlArrows: Boolean,
    controlArrowColor: String,
    verticalCentered: Boolean,
    sectionsColor: Array,
    paddingTop: String,
    paddingBottom: String,
    fixedElements: String, // '#header, .footer'
    responsive: Number,
    responsiveWidth: Number,
    responsiveHeight: Number,
    responsiveSlides: Boolean,
    parallax: Object, // true, false, 'sections', 'slides'
    parallaxOptions: {
        percentage: Number,
        property: String,
        type: String,
    },


    //Custom selectors
    sectionSelector: String,
    slideSelector: String,

    //Events
    v2compatible: Boolean,
    afterLoad(): Function,
    onLeave: Function,
    afterRender(): Function,
    afterResize(width: Number, height: Number): Function,
    afterReBuild: Function,
    afterSlideLoad: Function,
    onSlideLeave: Function,
    afterResponsive: Function,


    lazyLoading: Boolean,
    licenseKey: String,

    /* keys for extensions */
    fadingEffectKey: String,
    responsiveSlidesKey: String,
    continuousHorizontalKey: String,
    interlockedSlidesKey: String,
    scrollHorizontallyKey: String,
    resetSlidersKey: String,
    offsetSectionsKey: String,
    dragAndMoveKey: String,
    parallaxKey: String,
}

@alvarotrigo alvarotrigo merged commit 1b28948 into dev Nov 19, 2018
@alvarotrigo
Copy link
Owner

However I'm having issues when it comes to including the scrollOverflow extension, it breaks as it can't find a classList of undefined. Could you look into why this might happen? I left a note here

I get the Maximum call stack size exceeded from #38
But that's an issue in fullPage.js that I've solved on the dev branch. Not an issue.

Working great to me! I've merged it on the dev branch and I'll merge it to the mater one now. I'll be releasing a new version as some people is waiting for these fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants