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

How to use setState to change custom properties without forcing a refresh #118

Closed
alvarotrigo opened this issue Apr 28, 2019 · 32 comments
Closed

Comments

@alvarotrigo
Copy link
Owner

Taken from this question in stackoverflow..

I assume related with this line in the componentDidUpdate function.

import React from "react";
import ReactFullpage from "@fullpage/react-fullpage";
import AlbumInfo from './AlbumInfo';
import Player from './Player';

class Album extends React.Component {

    constructor(props){
        super(props);

        this.state={
            playing: false
        }
    }

    _initPlayer = (currentIndex, nextIndex) => {
        if(nextIndex.index === 1) {
            this.setState({playing:true})
        }
    }

    render() {

        return (
            <ReactFullpage
                licenseKey='xxxxxxxx-xxxxxxxx-xxxxxxxx-xxxxxxxx'
                sectionsColor={["#000000"]}
                afterLoad={this._initPlayer}
                render={({ state, fullpageApi }) => {
                    return (
                        <div id="fullpage-wrapper">
                            <div className="section">
                                <AlbumInfo />
                            </div>
                            <div className="section">
                                <Player playing={this.state.playing} />
                            </div>
                        </div>
                    );
                }}
            />
            );
        }
    }

export default Album;
@cmswalker
Copy link
Collaborator

Seems like your example would work fine? What issue does a full re-render cause that they are trying to avoid?

@alvarotrigo
Copy link
Owner Author

He says on the stackoverflow issue:

I tried below code, but once the state changes the entire ReactFullpage is re-rendered causing the first slide to be active again so I'm basically stuck in a loop.

I was just wondering. If they just want to update a property that is external to fullpage.js, as it seems the case with this.setState({playing:true}), it shouldn't be re-initialising fullpage.js right?

If the number of sections and slides is the same and the sectionsColor property didn't change, it shouldn't execute the buildOptions right?

@cmswalker
Copy link
Collaborator

So the nature of setting state in react will always trigger a re-render of subcomponents that subscribe to it. And because it's re-rerendering it will cause media to stop (will autoplay once scrolled to again). I'll have to see how other things like react-router have handled this as it's kind of tricky.

how does fullpage.js handle this? I noticed with videos it would stop when i left the section https://codepen.io/anon/pen/QRpOep

@cmswalker
Copy link
Collaborator

could also be a good use case for portals to keep the dom hierarchy separate in the case of continuous media https://reactjs.org/docs/portals.html

@alvarotrigo
Copy link
Owner Author

how does fullpage.js handle this? I noticed with videos it would stop when i left the section https://codepen.io/anon/pen/QRpOep

Using the attribute data-autoplay attribute on the media element. (Docs here).
If you want to left it playing, just use data-keepplaying instead.

@alvarotrigo
Copy link
Owner Author

alvarotrigo commented May 16, 2019

Please let me know if this makes no sense at all :)
But I though, because fullPage.js only requieres a re-rendering when adding new sections/slides, or changing any of its configurable options, I was wondering, couldn't we just filter by that in the componentDidUpdate function and only call this.destroy(); and this.init(this.buildOptions()) when changing any of those?

This way, when changing this.playing (which is not a fullpage.js option) it wouldn't have to re-build fullPage.js?

@cmswalker
Copy link
Collaborator

Yes we can give that a shot but i thought it was already doing that. Why is there a difference in sectionsColor in this scenario? We only look at sectionsColor changes for props and sectionCount and slideCount for internal state to fully rebuild

@alvarotrigo
Copy link
Owner Author

alvarotrigo commented May 16, 2019

@cmswalker here's the reproduction of the issue where I used a non minified version of the script to console.log some stuff in those conditions within the componentDidUpdate function:
https://codesandbox.io/s/g3qyj

We only look at sectionsColor changes for props and sectionCount and slideCount for internal state to fully rebuild

It seems the issue is there. We are comparing two arrays, which can't be done with ===.

Here's a fiddle showing the issue of comparing different array variables with the same content:
https://jsfiddle.net/alvarotrigo/a90zh2cu/5/

var demo = ['hi', 'my', 'friend'];
var demo2 = ['hi', 'my', 'friend'];

alert(demo === demo2);  //will output "false"

alert("ES 6 only--->" + demo.every( e => demo2.includes(e) )); //will output true

Perhaps we should go with the ES6 solution?
Source: https://stackoverflow.com/a/49218423/1081396

@cmswalker
Copy link
Collaborator

cmswalker commented May 16, 2019 via email

@alvarotrigo
Copy link
Owner Author

time, lets iterate to check deep equals

Ok!
Do you give it a go or should I commit the change?

@cmswalker
Copy link
Collaborator

cmswalker commented May 16, 2019 via email

@alvarotrigo
Copy link
Owner Author

Fixed on the dev branch 0.1.15!

@n0umankhan
Copy link

Hi, @alvarotrigo I'm facing the same problem, you already mentioned that it's fixed on dev. Can you please tell when is next deploy expected.

@alvarotrigo
Copy link
Owner Author

alvarotrigo commented Jun 17, 2019

@n0umankhan I'll probably wait until fullPage.js 3.0.6 gets released in a couple of weeks.
But so far you can get it from the dev branch:
https://github.com/alvarotrigo/react-fullpage/tree/dev

Or using npm:

npm install git://github.com/alvarotrigo/react-fullpage.git#dev --save

@n0umankhan
Copy link

Hi, @alvarotrigo I can use dev version but I prefer to wait, Is there any update did you get the chance to merge this in production

@n0umankhan
Copy link

BTW I tried with "npm install git://github.com/alvarotrigo/react-fullpage.git#dev --save" command but get the following error. Can you please help

test

@alvarotrigo
Copy link
Owner Author

Hi, @alvarotrigo I can use dev version but I prefer to wait, Is there any update did you get the chance to merge this in production

Yes, as you might have seen now the react-fullpage version updated to 0.1.15. Which includes fullpage.js 3.0.7.

Give it a try now.

@n0umankhan
Copy link

Hi, the error is still here if I destroy fullpage and then perform setState on resize it cause re-initialize

@alvarotrigo
Copy link
Owner Author

@n0umankhan can you please provide an isolated reproduction of the issue in codesandbox?
Instead of firing the setState on resize make sure to simplify it and make it when clicking a button.

@williamgravel
Copy link

I'm unsure if the following applies to this issue, but I have a hook state in the same component as the <ReactFullpage /> wrapper, which is then passed down to child components with a React context (useContext()). One of those children changes that state on button press, which ended up rebuilding/rerendering the entire DOM and thus leaving the previously active slide. Somehow, by adding an empty callback to the Fullpage wrapper after each render (afterRender={() => {}}), I was able to prevent Fullpage from rebuilding every time a state changes. I can provide more code if it can be helpful.

@alvarotrigo
Copy link
Owner Author

@williamgravel would you be able to provide us with a reproduction in Codesandbox with no CSS or JS files external to fullPage.js and the minimum amount of HTML code. Use empty sections unless strictly necessary for the reproduction.

@williamgravel
Copy link

@alvarotrigo I can't seem to recreate the issue in a controlled sandbox environment and am wondering if there is instead a small bug I am simply missing in my project. Is the afterRender() callback an empty function when initialized by Fullpage.js or if not what is its default functionality?

@alvarotrigo
Copy link
Owner Author

afterRender is a callback, by default it is null, but if it is empty, that's completely fine too.

@vjwilsonl
Copy link

vjwilsonl commented Aug 5, 2020

Hi @alvarotrigo , I am experiencing similar issue, how do I prevent behaviour of reseting to first slide when dynamically adding slide?
Example here
https://codesandbox.io/s/serene-hawking-pdxud

If we are in any section other than first, creating new section will cause jumping to first section.

Thanks in advance!

@alvarotrigo
Copy link
Owner Author

If we are in any section other than first, creating new section will cause jumping to first section.

I guess you would have to add the active class to it? :)

@martymfly
Copy link

Can you please provide an example of adding the active class so that it doesn't jump back to 1st slide upon adding new slides?

I did as below but it doesn't work. Maybe this is not the way it should be.

onLeave={(section, origin, destination, direction) => {
          origin.item.classList.add("active");
          section.item.classList.remove("active");
        }}

@alvarotrigo
Copy link
Owner Author

@martymfly if your problem is with the horizontal slides, then you need to add the active class on the horizontal slide, not just on the section.

Also note the onleave event has 3 params, not 4.

	onLeave: function(origin, destination, direction){},

Only the onSlideLeave has 4:

	onSlideLeave: function(section, origin, destination, direction){}

You can read about callbacks on the the fullpage.js documentation

@martymfly
Copy link

@martymfly if your problem is with the horizontal slides, then you need to add the active class on the horizontal slide, not just on the section.

Also note the onleave event has 3 params, not 4.

	onLeave: function(origin, destination, direction){},

Only the onSlideLeave has 4:

	onSlideLeave: function(section, origin, destination, direction){}

You can read about callbacks on the the fullpage.js documentation

The issue is not with the horizontal slides. My sample would be something like vjwilsonl shared.

Would you be so kind to show on the example below whenever you have time? :)

https://codesandbox.io/s/serene-hawking-pdxud

@alvarotrigo
Copy link
Owner Author

alvarotrigo commented Dec 5, 2020

Would you be so kind to show on the example below whenever you have time? :)

I'm that kind! Hope you purchased the license at least! :)

I'm not a React expert so you probably can do it much better, but here's what I came up with:
https://codesandbox.io/s/unruffled-framework-l9ibi?file=/src/index.js

Basically keeping track of the state of each section, then using it on the classname:

<div key={idx} className={`section ${page.state}`}>

Full code here:

import React from "react";
import ReactDOM from "react-dom";
import ReactFullpage from "@fullpage/react-fullpage";

import "./styles.css";

class App extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      cont: 0,
      fullpages: [
        {
          text: "first section",
          state: "active"
        },
        {
          text: "second section",
          state: ""
        }
      ]
    };
  }

  handleAddSection() {
    console.log("adding new section");
    this.setState((state, props) => {
      let fullpages = JSON.parse(JSON.stringify(state.fullpages));

      fullpages = fullpages.map(function (section) {
        section.state = "";
        return section;
      });

      state.cont++;

      fullpages.push({
        text: "new section " + state.cont,
        state: "active"
      });

      console.log(document.querySelectorAll(".fp-section"));
      return {
        fullpages
      };
    });
  }

  render() {
    return (
      <div className="App">
        <div className="menu">
          <ul>
            <li>
              <button onClick={() => this.handleAddSection()}>+ Section</button>
            </li>
          </ul>
        </div>
        <div className="fp">
          <ReactFullpage
            navigation={true}
            render={(state, fullpageApi) => {
              return (
                <ReactFullpage.Wrapper>
                  {this.state.fullpages.map((page, idx) => {
                    return (
                      <div key={idx} className={`section ${page.state}`}>
                        <p>{page.text}</p>
                      </div>
                    );
                  })}
                </ReactFullpage.Wrapper>
              );
            }}
          />
        </div>
      </div>
    );
  }
}

const rootElement = document.getElementById("root");
ReactDOM.render(<App />, rootElement);

@martymfly
Copy link

martymfly commented Dec 5, 2020

Thanks a lot for your time, but in your sample it goes to the last page added, it doesn't stay on where it was :( I mean if I'm on the 2nd page and added a 3rd one, it should stay on the 2nd page. To be honest I don't have the license yet but if I could get it worked as I wanted and use it somewhere in production, I'd gladly buy a license :))

By the way, I just realized that I might need the license for github support. So please do not waste your time until I get one :) I was just trying the package out to see if it's something that fits my needs :)

@alvarotrigo
Copy link
Owner Author

Ah, i thouhg you wanted to scroll to the new one :)

See the example in : react-fullpage/examples/react/dist/index.html.
I think it needs some improvement but it can give you an idea.

@alvarotrigo
Copy link
Owner Author

Fixed in version 0.1.22

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

No branches or pull requests

6 participants