Skip to content

Commit

Permalink
Update items in viewport before scheduling layout in carousel.
Browse files Browse the repository at this point in the history
  • Loading branch information
mkhatib committed Apr 19, 2016
1 parent 74b9daf commit 46a7865
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 2 deletions.
2 changes: 1 addition & 1 deletion extensions/amp-carousel/0.1/carousel.js
Expand Up @@ -107,9 +107,9 @@ export class AmpCarousel extends BaseCarousel {
st.setStyles(this.container_, {
transform: st.translateX(-newPos),
});
this.updateInViewport_(newPos, oldPos);
this.doLayout_(newPos);
this.preloadNext_(newPos, Math.sign(newPos - oldPos));
this.updateInViewport_(newPos, oldPos);
this.setControlsState();
}

Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-carousel/0.1/slides.js
Expand Up @@ -243,9 +243,9 @@ export class AmpSlides extends BaseCarousel {
transform: '',
opacity: 1,
});
this.scheduleLayout(newSlide);
this.updateInViewport(oldSlide, false);
this.updateInViewport(newSlide, true);
this.scheduleLayout(newSlide);
this.setControlsState();
this.schedulePause(oldSlide);
}
Expand Down
75 changes: 75 additions & 0 deletions extensions/amp-carousel/0.1/test/test-carousel.js
Expand Up @@ -163,3 +163,78 @@ describe('Carousel gestures', () => {
.to.be.false;
});
});

describe('Carousel layout scheduling and viewport updates', () => {
let sandbox;
let element;
let cell0, cell1, cell2;
let carousel;

function setupElements() {
element = document.createElement('div');
element.style.width = '320px';
element.style.height = '200px';
document.body.appendChild(element);

element.appendChild(cell0 = document.createElement('div'));
element.appendChild(cell1 = document.createElement('div'));
element.appendChild(cell2 = document.createElement('div'));
cell0.classList.add('cell0');
cell0.style.width = '300px';
cell1.classList.add('cell1');
cell1.style.width = '300px';
cell2.classList.add('cell2');
cell2.style.width = '300px';
element.getRealChildren = () => [cell0, cell1, cell2];
return element;
}

function setupCarousel() {
carousel = new AmpCarousel(element);
carousel.buildCallback();
carousel.container_.style.width = '320px';
return carousel;
}

function teardownElements() {
document.body.removeChild(element);
}

beforeEach(() => {
sandbox = sinon.sandbox.create();
setupElements();
setupCarousel();
carousel.inViewport_ = true;
carousel.getVsync = function() {
return {
mutate: function(fn) {
fn();
},
};
};
carousel.deferMutate = function(fn) {
fn();
};
carousel.doLayout_ = sandbox.spy();
carousel.updateInViewport_ = sandbox.spy();
carousel.preloadNext_ = sandbox.spy();
carousel.scheduleLayout = sandbox.spy();
carousel.updateInViewport = sandbox.spy();
carousel.schedulePause = sandbox.spy();
carousel.schedulePreload = sandbox.spy();
});
afterEach(() => {
sandbox.restore();
teardownElements();
});

it('should update viewport before scheduling carousel', () => {
carousel.goCallback(1, /*animate*/ false);
expect(
carousel.updateInViewport_.calledBefore(
carousel.doLayout_)).to.be.true;
expect(carousel.updateInViewport_.calledWith(320, 0)).to.be.true;
expect(carousel.doLayout_.calledWith(320)).to.be.true;
});

});
41 changes: 41 additions & 0 deletions extensions/amp-carousel/0.1/test/test-slides.js
Expand Up @@ -750,6 +750,47 @@ describe('Slides functional', () => {
expect(slides[index]).to.equal('e');
});
});

describe('scheduling slides and updating viewport', () => {
beforeEach(() => {
sandbox = sinon.sandbox.create();
setupElements();
setupSlides();
slides.inViewport_ = true;
slides.getVsync = function() {
return {
mutate: function(fn) {
fn();
},
};
};
slides.deferMutate = function(fn) {
fn();
};
slides.scheduleLayout = sandbox.spy();
slides.updateInViewport = sandbox.spy();
slides.schedulePause = sandbox.spy();
slides.schedulePreload = sandbox.spy();
});
afterEach(() => {
sandbox.restore();
teardownElements();
});

it('should update viewport before scheduling slides', () => {
slides.goCallback(1, /*animate*/ false);
expect(slides.updateInViewport.calledBefore(
slides.scheduleLayout)).to.be.true;
expect(slides.updateInViewport.calledWith(slide0, false)).to.be.true;
expect(slides.updateInViewport.calledWith(slide1, true)).to.be.true;

slides.goCallback(-1, /*animate*/ false);
expect(slides.updateInViewport.calledBefore(
slides.scheduleLayout)).to.be.true;
expect(slides.updateInViewport.calledWith(slide0, true)).to.be.true;
expect(slides.updateInViewport.calledWith(slide1, false)).to.be.true;
});
});
});

describe('empty Slides functional', () => {
Expand Down

0 comments on commit 46a7865

Please sign in to comment.