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

Here's how I moved slideRenderer from onIndexChange to onTransitionEnd, so no more DOM update/state change freezes! #633

Open
ajaypillay opened this issue Dec 27, 2020 · 0 comments

Comments

@ajaypillay
Copy link

ajaypillay commented Dec 27, 2020

Here's the issue I faced. I was using a virtualized swiper, and due to heavy DOM rendering and constant state changes while swiping, every time I swiped to a new page, there is a momentary freeze before the transition animation begins (~10ms? or so). I had to set overscanSlideBefore and overscanSlideAfter to 1 to get the bare minimum acceptable performance. Anything more, and the transition would be choppy, and wouldn't even play unless I explicitly defined a springConfig, and made the delay > 0.4s.

In response to my previous question about how to accomplish this, I decided to try to get it work for myself. However, this is DEFINITELY hacky, it works so far, and I just wanted to document it in case anyone else finds it useful. I'm running a Meteor/React/Apollo/Graphql stack for a web app I'm making with full page swiping, for context.

Here's what I did at first, in react-swipeable-views-utils/lib/virtualizer.js, in particular it is in these two function blocks:

_this.handleChangeIndex = function (indexContainer, indexLatest) {
    
    var _this$props = _this.props,
        slideCount = _this$props.slideCount,
        onChangeIndex = _this$props.onChangeIndex;
    var indexDiff = indexContainer - indexLatest;
    var index = _this.state.index + indexDiff;

    if (slideCount) {
        index = (0, _reactSwipeableViewsCore.mod)(index, slideCount);
    } // Is uncontrolled


    if (_this.props.index === undefined) {
        _this.setIndex(index, indexContainer, indexDiff);
    }

    if (onChangeIndex) {
        onChangeIndex(index, _this.state.index);
    }
};

_this.handleTransitionEnd = function () {
    // Delay the update of the window to fix an issue with react-motion.
    _this.timer = setTimeout(function () {
      _this.setWindow();
    }, 0);

    if (_this.props.onTransitionEnd) {
      _this.props.onTransitionEnd();
    }
};

My train of thought was to just move all the code in handleChangeIndex into handleTransitionEnd. If you can catch what may be wrong with this, good job, but unfortunately I didn't. There's a specific case in which this will fail. So, by refactoring the code, I created two new states in handleChangeIndex and just referenced them in handleTransitionEnd. This was too good to be true.

_this.handleChangeIndex = function (indexContainer, indexLatest) {
    _this.state.handleChangeIndexindexContainer = indexContainer;
    _this.state.handleChangeIndexindexLatest = indexLatest;
};

_this.handleTransitionEnd = function () {
    // Delay the update of the window to fix an issue with react-motion.
    _this.timer = setTimeout(function () {
      _this.setWindow();
    }, 0);
    var _this$props = _this.props,
        slideCount = _this$props.slideCount,
        onChangeIndex = _this$props.onChangeIndex;

    var indexDiff = _this.state.handleChangeIndexindexContainer - _this.state.handleChangeIndexindexLatest;
    var index = _this.state.index + indexDiff;
    if (slideCount) {
        index = (0, _reactSwipeableViewsCore.mod)(index, slideCount);
    } // Is uncontrolled

    if (_this.props.index === undefined) {
        _this.setIndex(index, _this.state.handleChangeIndexindexContainer, indexDiff);
    }

    if (onChangeIndex) {
        onChangeIndex(index, _this.state.index);
    }
  
  if (_this.props.onTransitionEnd) {
    _this.props.onTransitionEnd();
  }
};

So I was playing around with this and noticed this worked fine, if you actually swipe away to another page. But if you began the swipe, but cancelled it and remained on the same page, you jump to another page. So I realized that this is because handleTransitionEnd fires off in this case, without the index actually being changed. Now how to fix this was that because handleTransitionEnd does not have access to the current index, or whether the index was successfully changed, I created another state object to keep track of when handleChangeIndex successfully fires off, and handleTransitionEnd will catch this specific case, and then carry on with the rendering. So the final code is as such:

_this.handleChangeIndex = function (indexContainer, indexLatest) {
    _this.state.handleChangeIndexindexContainer = indexContainer;
    _this.state.handleChangeIndexindexLatest = indexLatest;
    _this.state.indexChanged = true;
};

_this.handleTransitionEnd = function () {
      // Delay the update of the window to fix an issue with react-motion.
      _this.timer = setTimeout(function () {
        _this.setWindow();
      }, 0);
      if (_this.state.indexChanged) {
          _this.state.indexChanged = false;
          var _this$props = _this.props,
              slideCount = _this$props.slideCount,
              onChangeIndex = _this$props.onChangeIndex;
    
          var indexDiff = _this.state.handleChangeIndexindexContainer - _this.state.handleChangeIndexindexLatest;
          var index = _this.state.index + indexDiff;
          if (slideCount) {
              index = (0, _reactSwipeableViewsCore.mod)(index, slideCount);
          } // Is uncontrolled
    
          if (_this.props.index === undefined) {
              _this.setIndex(index, _this.state.handleChangeIndexindexContainer, indexDiff);
          }
    
          if (onChangeIndex) {
              onChangeIndex(index, _this.state.index);
          }
      }
      if (_this.props.onTransitionEnd) {
        _this.props.onTransitionEnd();
      }
};

And (so far) it's been working pretty well, without any weird jumps, since I only render the new slides upon a successful index change, and only once the transition has ended. The less than ideal state variable names are because I did not want to interfere with any current ones. indexContainer already exists in state, for example.

Hope this helps someone, and if someone more experienced is able to incorporate this into a PR for this package, that'd be great!

EDIT1: So I managed to break it. If you swipe in opposite directions back and forth within 2 pages, multiple handleChangeIndex events are fired, then it bugs out and skips/jumps pages. A simple fix I made is to make sure the state is true only for the final handleChangeIndex event, so handleTransitionEnd doesn't fire when we don't want it to:

_this.handleChangeIndex = function (indexContainer, indexLatest) {
    _this.state.handleChangeIndexindexContainer = indexContainer;
    _this.state.handleChangeIndexindexLatest = indexLatest;
    if (_this.state.indexChanged) _this.state.indexChanged = false;
    else _this.state.indexChanged = true;
};
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

No branches or pull requests

1 participant