Skip to content

Commit

Permalink
Fix lazy loading (#462)
Browse files Browse the repository at this point in the history
* lazy load non visible views on first render by setTimeout

* fix unit tests to reflect new behavior

* let's merge
  • Loading branch information
Bobgy authored and oliviertassinari committed Sep 22, 2018
1 parent a3f851e commit 248fec1
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 29 deletions.
6 changes: 3 additions & 3 deletions packages/react-swipeable-views-utils/src/virtualize.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ export default function virtualize(MyComponent) {
class Virtualize extends PureComponent {
timer = null;

constructor(props, context) {
super(props, context);
this.state.index = this.props.index || 0;
constructor(props) {
super(props);
this.state.index = props.index || 0;
}

/**
Expand Down
38 changes: 25 additions & 13 deletions packages/react-swipeable-views/src/SwipeableViews.js
Original file line number Diff line number Diff line change
Expand Up @@ -254,25 +254,27 @@ class SwipeableViews extends React.Component {

indexCurrent = null;

constructor(props, context) {
super(props, context);
firstRenderTimeout = null;

constructor(props) {
super(props);

if (process.env.NODE_ENV !== 'production') {
checkIndexBounds(this.props);
checkIndexBounds(props);
}

this.state = {
indexLatest: this.props.index,
indexLatest: props.index,
// Set to true as soon as the component is swiping.
// It's the state counter part of this.isSwiping.
isDragging: false,
// Help with SSR logic and lazy loading logic.
isFirstRender: true,
renderOnlyActive: !props.disableLazyLoading,
heightLatest: 0,
// Let the render method that we are going to display the same slide than previously.
displaySameSlide: true,
};
this.setIndexCurrent(this.props.index);
this.setIndexCurrent(props.index);
}

getChildContext() {
Expand Down Expand Up @@ -315,10 +317,13 @@ class SwipeableViews extends React.Component {
},
);

// eslint-disable-next-line react/no-did-mount-set-state
this.setState({
isFirstRender: false,
});
if (!this.props.disableLazyLoading) {
this.firstRenderTimeout = setTimeout(() => {
this.setState({
renderOnlyActive: false,
});
}, 0);
}

injectStyle();

Expand Down Expand Up @@ -350,6 +355,7 @@ class SwipeableViews extends React.Component {
componentWillUnmount() {
this.transitionListener.remove();
this.touchMoveListener.remove();
clearTimeout(this.firstRenderTimeout);
}

setIndexCurrent(indexCurrent) {
Expand Down Expand Up @@ -739,7 +745,13 @@ class SwipeableViews extends React.Component {
...other
} = this.props;

const { displaySameSlide, heightLatest, isDragging, isFirstRender, indexLatest } = this.state;
const {
displaySameSlide,
heightLatest,
indexLatest,
isDragging,
renderOnlyActive,
} = this.state;
const touchEvents = !disabled
? {
onTouchStart: this.handleTouchStart,
Expand Down Expand Up @@ -793,7 +805,7 @@ So animateHeight is most likely having no effect at all.`,
};

// Apply the styles for SSR considerations
if (disableLazyLoading || !isFirstRender) {
if (!renderOnlyActive) {
const transform = axisProperties.transform[axis](this.indexCurrent * 100);
containerStyle.WebkitTransform = transform;
containerStyle.transform = transform;
Expand All @@ -818,7 +830,7 @@ So animateHeight is most likely having no effect at all.`,
className="react-swipeable-view-container"
>
{React.Children.map(children, (child, indexChild) => {
if (!disableLazyLoading && isFirstRender && indexChild !== indexLatest) {
if (renderOnlyActive && indexChild !== indexLatest) {
return null;
}

Expand Down
33 changes: 20 additions & 13 deletions packages/react-swipeable-views/src/SwipeableViews.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import { mount, shallow } from 'enzyme';
import { assert } from 'chai';
import { spy, stub } from 'sinon';
import { spy, stub, useFakeTimers } from 'sinon';
import SwipeableViews, { findNativeHandler, getDomTreeShapes } from './SwipeableViews';

function simulateSwipeMove(wrapper, event) {
Expand All @@ -17,7 +17,7 @@ describe('SwipeableViews', () => {
describe('prop: children', () => {
it('should render the children', () => {
const wrapper = mount(
<SwipeableViews>
<SwipeableViews disableLazyLoading>
<div>{'slide n°1'}</div>
<div>{'slide n°2'}</div>
<div>{'slide n°3'}</div>
Expand All @@ -26,11 +26,7 @@ describe('SwipeableViews', () => {
</SwipeableViews>,
);

assert.strictEqual(
wrapper.text(),
'slide n°1slide n°2slide n°3slide n°4slide n°5',
'Should render each slide.',
);
assert.strictEqual(wrapper.text(), 'slide n°1slide n°2slide n°3slide n°4slide n°5');
});
});

Expand Down Expand Up @@ -139,7 +135,7 @@ describe('SwipeableViews', () => {
wrapper.simulate('touchStart', {
touches: [{}],
});
assert.strictEqual(handleTouchStart.callCount, 1, 'Should be called');
assert.strictEqual(handleTouchStart.callCount, 1);
});

it('should trigger when we disable the swipe', () => {
Expand All @@ -153,7 +149,7 @@ describe('SwipeableViews', () => {
wrapper.simulate('touchStart', {
touches: [{}],
});
assert.strictEqual(handleTouchStart.callCount, 1, 'Should be called');
assert.strictEqual(handleTouchStart.callCount, 1);
});
});

Expand All @@ -167,17 +163,16 @@ describe('SwipeableViews', () => {
);

wrapper.simulate('touchEnd');
assert.strictEqual(handleTouchEnd.callCount, 1, 'Should be called');
assert.strictEqual(handleTouchEnd.callCount, 1);
});
});

describe('prop: animateTransitions', () => {
it('should use a spring if animateTransitions is true', () => {
const wrapper = mount(
<SwipeableViews>
<SwipeableViews disableLazyLoading>
<div>{'slide n°1'}</div>
</SwipeableViews>,
{ disableLifecycleMethods: true },
);

wrapper.setProps({
Expand Down Expand Up @@ -587,6 +582,16 @@ describe('SwipeableViews', () => {

describe('prop: disableLazyLoading', () => {
describe('false', () => {
let clock;

beforeEach(() => {
clock = useFakeTimers();
});

afterEach(() => {
clock.restore();
});

it('should render the right child', () => {
const wrapper = shallow(
<SwipeableViews index={1}>
Expand Down Expand Up @@ -615,9 +620,11 @@ describe('SwipeableViews', () => {
<div>{'slide n°4'}</div>
<div>{'slide n°5'}</div>
</SwipeableViews>,
{ disableLifecycleMethods: true },
);

assert.strictEqual(wrapper.text(), 'slide n°2');
clock.tick(1);
wrapper.update();
assert.strictEqual(wrapper.text(), 'slide n°1slide n°2slide n°3slide n°4slide n°5');
assert.shallowDeepEqual(
wrapper
Expand Down

0 comments on commit 248fec1

Please sign in to comment.