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.

Sign up for GitHub

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

Jump to bottom

Remove unused timer delay to scheduleLayout #10989

Merged
merged 4 commits into from Aug 18, 2017
Merged

Remove unused timer delay to scheduleLayout #10989

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0a840e2
remove unused timer delay
zhouyx Aug 18, 2017
e04a4c1
wait for transitionend on cloase
zhouyx Aug 18, 2017
db91e34
change timeout time
zhouyx Aug 18, 2017
89eb012
fix test
zhouyx Aug 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
62 changes: 28 additions & 34 deletions extensions/amp-sidebar/0.1/amp-sidebar.js
View file
Open in desktop
Expand Up @@ -31,7 +31,7 @@ import {toArray} from '../../../src/types';
const TAG = 'amp-sidebar toolbar';

/** @const */
const ANIMATION_TIMEOUT = 550;
const ANIMATION_TIMEOUT = 350;

/** @const */
const IOS_SAFARI_BOTTOMBAR_HEIGHT = '10vh';
Expand Down Expand Up @@ -79,18 +79,12 @@ export class AmpSidebar extends AMP.BaseElement {
/** @private {boolean} */
this.bottomBarCompensated_ = false;

/** @private @const {!../../../src/service/timer-impl.Timer} */
this.timer_ = Services.timerFor(this.win);

/** @private {number|string|null} */
this.openOrCloseTimeOut_ = null;

/** @const {function()} */
this.boundReschedule_ = debounce(this.win, () => {
const children = this.getRealChildren();
this.scheduleLayout(children);
this.scheduleResume(children);
}, 500);
this.boundOnAnimationEnd_ =
debounce(this.win, this.onAnimationEnd_.bind(this), ANIMATION_TIMEOUT);
}

/** @override */
Expand Down Expand Up @@ -191,6 +185,9 @@ export class AmpSidebar extends AMP.BaseElement {
}
}
}, true);

this.element.addEventListener('transitionend', this.boundOnAnimationEnd_);
this.element.addEventListener('animationend', this.boundOnAnimationEnd_);
}

/** @override */
Expand Down Expand Up @@ -256,19 +253,8 @@ export class AmpSidebar extends AMP.BaseElement {
this.vsync_.mutate(() => {
this.openMask_();
this.element.setAttribute('open', '');
this.boundOnAnimationEnd_();
this.element.setAttribute('aria-hidden', 'false');
if (this.openOrCloseTimeOut_) {
this.timer_.cancel(this.openOrCloseTimeOut_);
}
this.openOrCloseTimeOut_ = this.timer_.delay(() => {
const children = this.getRealChildren();
this.scheduleLayout(children);
this.scheduleResume(children);
this.element.addEventListener('transitionend', this.boundReschedule_);
this.element.addEventListener('animationend', this.boundReschedule_);
// Focus on the sidebar for a11y.
tryFocus(this.element);
}, ANIMATION_TIMEOUT);
});
});
this.getHistory_().push(this.close_.bind(this)).then(historyId => {
Expand All @@ -285,23 +271,11 @@ export class AmpSidebar extends AMP.BaseElement {
return;
}
this.viewport_.leaveOverlayMode();
this.element.removeEventListener('transitionend', this.boundReschedule_);
this.element.removeEventListener('animationend', this.boundReschedule_);
this.vsync_.mutate(() => {
this.closeMask_();
this.element.removeAttribute('open');
this.boundOnAnimationEnd_();
this.element.setAttribute('aria-hidden', 'true');
if (this.openOrCloseTimeOut_) {
this.timer_.cancel(this.openOrCloseTimeOut_);
}
this.openOrCloseTimeOut_ = this.timer_.delay(() => {
if (!this.isOpen_()) {
this.vsync_.mutate(() => {
toggle(this.element, /* display */false);
this.schedulePause(this.getRealChildren());
});
}
}, ANIMATION_TIMEOUT);
});
if (this.historyId_ != -1) {
this.getHistory_().pop(this.historyId_);
Expand Down Expand Up @@ -380,6 +354,26 @@ export class AmpSidebar extends AMP.BaseElement {
getHistory_() {
return Services.historyForDoc(this.getAmpDoc());
}

/**
* Get called when animation/transition end when open/close sidebar
* @private
*/
onAnimationEnd_() {
if (this.isOpen_()) {
// On open sidebar
const children = this.getRealChildren();
this.scheduleLayout(children);
this.scheduleResume(children);
tryFocus(this.element);
} else {
// On close sidebar
this.vsync_.mutate(() => {
toggle(this.element, /* display */false);
this.schedulePause(this.getRealChildren());
});
}
}
}

AMP.extension('amp-sidebar', '0.1', AMP => {
Expand Down
77 changes: 34 additions & 43 deletions extensions/amp-sidebar/0.1/test/test-amp-sidebar.js
View file
Open in desktop
Expand Up @@ -23,6 +23,7 @@ import {assertScreenReaderElement} from '../../../../testing/test-helper';
import {toggleExperiment} from '../../../../src/experiments';
import * as sinon from 'sinon';
import '../amp-sidebar';
import * as lolex from 'lolex';

adopt(window);

Expand All @@ -38,6 +39,7 @@ describes.realWin('amp-sidebar 0.1 version', {
}, () => {
let sandbox;
let platform;
let clock;
let timer;

function getAmpSidebar(options) {
Expand Down Expand Up @@ -180,9 +182,6 @@ describes.realWin('amp-sidebar 0.1 version', {
callback();
},
};
sandbox.stub(timer, 'delay', function(callback) {
callback();
});
impl.open_();
expect(iframe.doc.querySelectorAll('.i-amphtml-sidebar-mask').length)
.to.equal(1);
Expand Down Expand Up @@ -210,6 +209,7 @@ describes.realWin('amp-sidebar 0.1 version', {
return getAmpSidebar().then(obj => {
const sidebarElement = obj.ampSidebar;
const impl = sidebarElement.implementation_;
clock = lolex.install(impl.win, 0, ['Date', 'setTimeout']);
const historyPushSpy = sandbox.spy();
const historyPopSpy = sandbox.spy();
impl.scheduleLayout = sandbox.spy();
Expand All @@ -230,23 +230,22 @@ describes.realWin('amp-sidebar 0.1 version', {
callback();
},
};
sandbox.stub(timer, 'delay', function(callback) {
callback();
});
timer.cancel = sandbox.spy();
impl.openOrCloseTimeOut_ = 10;

impl.open_();
expect(sidebarElement.hasAttribute('open')).to.be.true;
expect(sidebarElement.getAttribute('aria-hidden')).to.equal('false');
expect(sidebarElement.getAttribute('role')).to.equal('menu');
expect(obj.iframe.doc.activeElement).to.equal(sidebarElement);
expect(sidebarElement.style.display).to.equal('');
expect(timer.cancel).to.be.calledOnce;
expect(impl.scheduleLayout).to.be.calledOnce;

expect(historyPushSpy).to.be.calledOnce;
expect(historyPopSpy).to.have.not.been.called;
expect(impl.historyId_).to.not.equal('-1');
expect(impl.scheduleLayout).to.not.be.called;

clock.tick(600);
expect(obj.iframe.doc.activeElement).to.equal(sidebarElement);
expect(sidebarElement.style.display).to.equal('');
expect(impl.scheduleLayout).to.be.calledOnce;

// second call to open_() should be a no-op and not increase call counts.
impl.open_();
Expand All @@ -261,6 +260,7 @@ describes.realWin('amp-sidebar 0.1 version', {
return getAmpSidebar({'open': true}).then(obj => {
const sidebarElement = obj.ampSidebar;
const impl = sidebarElement.implementation_;
clock = lolex.install(impl.win, 0, ['Date', 'setTimeout']);
impl.schedulePause = sandbox.spy();
const historyPushSpy = sandbox.spy();
const historyPopSpy = sandbox.spy();
Expand All @@ -283,17 +283,13 @@ describes.realWin('amp-sidebar 0.1 version', {
callback();
},
};
sandbox.stub(timer, 'delay', function(callback) {
callback();
});

timer.cancel = sandbox.spy();
impl.openOrCloseTimeOut_ = 10;
impl.close_();
expect(sidebarElement.hasAttribute('open')).to.be.false;
expect(sidebarElement.getAttribute('aria-hidden')).to.equal('true');
clock.tick(600);
expect(sidebarElement.style.display).to.equal('none');
expect(timer.cancel).to.be.calledOnce;
expect(impl.schedulePause).to.be.calledOnce;
expect(historyPopSpy).to.be.calledOnce;
expect(impl.historyId_).to.equal(-1);
Expand All @@ -309,29 +305,30 @@ describes.realWin('amp-sidebar 0.1 version', {
return getAmpSidebar().then(obj => {
const sidebarElement = obj.ampSidebar;
const impl = sidebarElement.implementation_;
clock = lolex.install(impl.win, 0, ['Date', 'setTimeout']);
impl.scheduleLayout = sandbox.spy();
impl.schedulePause = sandbox.spy();
impl.vsync_ = {
mutate(callback) {
callback();
},
};
sandbox.stub(timer, 'delay', function(callback) {
callback();
});

expect(sidebarElement.hasAttribute('open')).to.be.false;
expect(sidebarElement.getAttribute('aria-hidden')).to.equal('true');
expect(sidebarElement.getAttribute('role')).to.equal('menu');
expect(obj.iframe.doc.activeElement).to.not.equal(sidebarElement);
impl.toggle_();
expect(sidebarElement.hasAttribute('open')).to.be.true;
expect(sidebarElement.getAttribute('aria-hidden')).to.equal('false');
clock.tick(600);
expect(obj.iframe.doc.activeElement).to.equal(sidebarElement);
expect(sidebarElement.style.display).to.equal('');
expect(impl.scheduleLayout).to.be.calledOnce;
impl.toggle_();
expect(sidebarElement.hasAttribute('open')).to.be.false;
expect(sidebarElement.getAttribute('aria-hidden')).to.equal('true');
clock.tick(600);
expect(sidebarElement.style.display).to.equal('none');
expect(impl.schedulePause).to.be.calledOnce;
});
Expand All @@ -342,15 +339,13 @@ describes.realWin('amp-sidebar 0.1 version', {
const iframe = obj.iframe;
const sidebarElement = obj.ampSidebar;
const impl = sidebarElement.implementation_;
clock = lolex.install(impl.win, 0, ['Date', 'setTimeout']);
impl.schedulePause = sandbox.spy();
impl.vsync_ = {
mutate(callback) {
callback();
},
};
sandbox.stub(timer, 'delay', function(callback) {
callback();
});
expect(sidebarElement.hasAttribute('open')).to.be.false;
impl.open_();
expect(sidebarElement.hasAttribute('open')).to.be.true;
Expand All @@ -367,6 +362,7 @@ describes.realWin('amp-sidebar 0.1 version', {
el.dispatchEvent(eventObj) : el.fireEvent('onkeydown', eventObj);
expect(sidebarElement.hasAttribute('open')).to.be.false;
expect(sidebarElement.getAttribute('aria-hidden')).to.equal('true');
clock.tick(600);
expect(sidebarElement.style.display).to.equal('none');
expect(impl.schedulePause).to.be.calledOnce;
});
Expand All @@ -376,33 +372,37 @@ describes.realWin('amp-sidebar 0.1 version', {
return getAmpSidebar().then(obj => {
const sidebarElement = obj.ampSidebar;
const impl = sidebarElement.implementation_;
clock = lolex.install(impl.win, 0, ['Date', 'setTimeout']);
impl.schedulePause = sandbox.spy();
impl.scheduleResume = sandbox.spy();
impl.vsync_ = {
mutate(callback) {
callback();
},
};
sandbox.stub(timer, 'delay', function(callback) {
callback();
});

expect(impl.isOpen_()).to.be.false;
clock.tick(600);
expect(impl.schedulePause).to.have.not.been.called;
expect(impl.scheduleResume).to.have.not.been.called;
impl.toggle_();
expect(impl.isOpen_()).to.be.true;
clock.tick(600);
expect(impl.schedulePause).to.have.not.been.called;
expect(impl.scheduleResume).to.be.calledOnce;
impl.toggle_();
expect(impl.isOpen_()).to.be.false;
clock.tick(600);
expect(impl.schedulePause).to.be.calledOnce;
expect(impl.scheduleResume).to.be.calledOnce;
impl.toggle_();
expect(impl.isOpen_()).to.be.true;
clock.tick(600);
expect(impl.schedulePause).to.be.calledOnce;
expect(impl.scheduleResume).to.have.callCount(2);
impl.toggle_();
expect(impl.isOpen_()).to.be.false;
clock.tick(600);
expect(impl.schedulePause).to.have.callCount(2);
expect(impl.scheduleResume).to.have.callCount(2);
});
Expand Down Expand Up @@ -459,15 +459,13 @@ describes.realWin('amp-sidebar 0.1 version', {
const anchor = sidebarElement.getElementsByTagName('a')[0];
anchor.href = '#newloc';
const impl = sidebarElement.implementation_;
clock = lolex.install(impl.win, 0, ['Date', 'setTimeout']);
impl.schedulePause = sandbox.spy();
impl.vsync_ = {
mutate(callback) {
callback();
},
};
sandbox.stub(timer, 'delay', function(callback) {
callback();
});
expect(sidebarElement.hasAttribute('open')).to.be.false;
impl.open_();
expect(sidebarElement.hasAttribute('open')).to.be.true;
Expand All @@ -491,6 +489,7 @@ describes.realWin('amp-sidebar 0.1 version', {
anchor.fireEvent('onkeydown', eventObj);
expect(sidebarElement.hasAttribute('open')).to.be.false;
expect(sidebarElement.getAttribute('aria-hidden')).to.equal('true');
clock.tick(600);
expect(sidebarElement.style.display).to.equal('none');
expect(impl.schedulePause).to.be.calledOnce;
});
Expand Down Expand Up @@ -623,34 +622,26 @@ describes.realWin('amp-sidebar 0.1 version', {
return getAmpSidebar().then(obj => {
const sidebarElement = obj.ampSidebar;
const impl = sidebarElement.implementation_;
impl.boundReschedule_ = sandbox.spy();
clock = lolex.install(impl.win, 0, ['Date', 'setTimeout']);
impl.boundOnAnimationEnd_ = sandbox.spy();
impl.buildCallback();
impl.vsync_ = {
mutate(callback) {
callback();
},
};
sandbox.stub(timer, 'delay', function(callback) {
callback();
});
impl.toggle_();
const animationEndEventObj = new Event(
const animationEndEvent = new Event(
'animationend',
{bubbles: true}
);
sidebarElement.firstChild.dispatchEvent(animationEndEventObj);
expect(impl.boundReschedule_).to.be.calledOnce;
impl.boundReschedule_.reset();
sidebarElement.firstChild.dispatchEvent(animationEndEvent);
expect(impl.boundOnAnimationEnd_).to.be.calledOnce;
const transitionEndEvent = new Event(
'transitionend',
{bubbles: true}
);
sidebarElement.firstChild.dispatchEvent(transitionEndEvent);
expect(impl.boundReschedule_).to.be.calledOnce;
impl.boundReschedule_.reset();
impl.toggle_();
sidebarElement.firstChild.dispatchEvent(animationEndEventObj);
sidebarElement.firstChild.dispatchEvent(transitionEndEvent);
expect(impl.boundReschedule_).to.not.be.called;
expect(impl.boundOnAnimationEnd_).to.be.calledTwice;
});
});
});
Expand Down