Skip to content

Commit

Permalink
Fix local navigation history management by handling hash navigations (#…
Browse files Browse the repository at this point in the history 
…5961)
  • Loading branch information
@mkhatib
mkhatib committed Nov 18, 2016
1 parent 4eeb666 commit ff7b32b
Show file tree
Hide file tree
Showing 7 changed files with 437 additions and 126 deletions.
10 changes: 9 additions & 1 deletion extensions/amp-sidebar/0.1/amp-sidebar.js
View file
Expand Up @@ -15,8 +15,9 @@
*/

import {CSS} from '../../../build/amp-sidebar-0.1.css';
import {tryFocus} from '../../../src/dom';
import {closestByTag, tryFocus} from '../../../src/dom';
import {Layout} from '../../../src/layout';
import {dev} from '../../../src/log';
import {historyForDoc} from '../../../src/history';
import {platformFor} from '../../../src/platform';
import {setStyles, toggle} from '../../../src/style';
Expand Down Expand Up @@ -128,6 +129,13 @@ export class AmpSidebar extends AMP.BaseElement {
this.registerAction('toggle', this.toggle_.bind(this));
this.registerAction('open', this.open_.bind(this));
this.registerAction('close', this.close_.bind(this));

this.element.addEventListener('click', e => {
const target = closestByTag(dev().assertElement(e.target), 'A');
if (target && target.href) {
this.close_();
}
}, true);
}

/**
Expand Down
71 changes: 71 additions & 0 deletions extensions/amp-sidebar/0.1/test/test-amp-sidebar.js
View file
Expand Up @@ -41,6 +41,9 @@ describe('amp-sidebar', () => {
list.appendChild(li);
}
ampSidebar.appendChild(list);
const anchor = iframe.doc.createElement('a');
anchor.href = '#section1';
ampSidebar.appendChild(anchor);
if (options.side) {
ampSidebar.setAttribute('side', options.side);
}
Expand Down Expand Up @@ -279,6 +282,74 @@ describe('amp-sidebar', () => {
});
});


it('should close sidebar if clicked on anchor', () => {
return getAmpSidebar().then(obj => {
const sidebarElement = obj.ampSidebar;
const anchor = sidebarElement.getElementsByTagName('a')[0];
const impl = sidebarElement.implementation_;
impl.schedulePause = sandbox.spy();
impl.vsync_ = {
mutate: function(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;
expect(sidebarElement.getAttribute('aria-hidden')).to.equal('false');
const eventObj = document.createEventObject ?
document.createEventObject() : document.createEvent('Events');
if (eventObj.initEvent) {
eventObj.initEvent('click', true, true);
}
anchor.dispatchEvent ?
anchor.dispatchEvent(eventObj) :
anchor.fireEvent('onkeydown', eventObj);
expect(sidebarElement.hasAttribute('open')).to.be.false;
expect(sidebarElement.getAttribute('aria-hidden')).to.equal('true');
expect(sidebarElement.style.display).to.equal('none');
expect(impl.schedulePause.callCount).to.equal(1);
});
});


it('should not close sidebar if clicked on non-anchor', () => {
return getAmpSidebar().then(obj => {
const sidebarElement = obj.ampSidebar;
const li = sidebarElement.getElementsByTagName('li')[0];
const impl = sidebarElement.implementation_;
impl.schedulePause = sandbox.spy();
impl.vsync_ = {
mutate: function(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;
expect(sidebarElement.getAttribute('aria-hidden')).to.equal('false');
const eventObj = document.createEventObject ?
document.createEventObject() : document.createEvent('Events');
if (eventObj.initEvent) {
eventObj.initEvent('click', true, true);
}
li.dispatchEvent ?
li.dispatchEvent(eventObj) :
li.fireEvent('onkeydown', eventObj);
expect(sidebarElement.hasAttribute('open')).to.be.true;
expect(sidebarElement.getAttribute('aria-hidden')).to.equal('false');
expect(sidebarElement.style.display).to.equal('');
expect(impl.schedulePause.callCount).to.equal(0);
});
});

it('should reflect state of the sidebar', () => {
return getAmpSidebar().then(obj => {
const sidebarElement = obj.ampSidebar;
Expand Down
89 changes: 63 additions & 26 deletions src/document-click.js
View file
Expand Up @@ -64,12 +64,13 @@ export class ClickHandler {
/** @private @const {boolean} */
this.isIosSafari_ = platform.isIos() && platform.isSafari();

// Only intercept clicks when iframed.
if (this.viewer_.isIframed() && this.viewer_.isOvertakeHistory()) {
/** @private @const {!function(!Event)|undefined} */
this.boundHandle_ = this.handle_.bind(this);
this.ampdoc.getRootNode().addEventListener('click', this.boundHandle_);
}
/** @private @const {boolean} */
this.isIframed_ = (this.viewer_.isIframed() &&
this.viewer_.isOvertakeHistory());

/** @private @const {!function(!Event)|undefined} */
this.boundHandle_ = this.handle_.bind(this);
this.ampdoc.getRootNode().addEventListener('click', this.boundHandle_);
}

/**
Expand All @@ -89,7 +90,8 @@ export class ClickHandler {
*/
handle_(e) {
onDocumentElementClick_(
e, this.ampdoc, this.viewport_, this.history_, this.isIosSafari_);
e, this.ampdoc, this.viewport_, this.history_, this.isIosSafari_,
this.isIframed_);
}
}

Expand All @@ -106,9 +108,10 @@ export class ClickHandler {
* @param {!./service/viewport-impl.Viewport} viewport
* @param {!./service/history-impl.History} history
* @param {boolean} isIosSafari
* @param {boolean} isIframed
*/
export function onDocumentElementClick_(
e, ampdoc, viewport, history, isIosSafari) {
e, ampdoc, viewport, history, isIosSafari, isIframed) {
if (e.defaultPrevented) {
return;
}
Expand All @@ -119,10 +122,30 @@ export function onDocumentElementClick_(
}
urlReplacementsForDoc(ampdoc).maybeExpandLink(target);

/** @const {!Window} */
const win = ampdoc.win;
const tgtLoc = parseUrl(target.href);
// Handle custom protocols only if the document is iframe'd.
if (isIframed) {
handleCustomProtocolClick_(e, target, tgtLoc, ampdoc, isIosSafari);
}

if (tgtLoc.hash) {
handleHashClick_(e, tgtLoc, ampdoc, viewport, history);
}
}


/**
* Handles clicking on a custom protocol link.
* @param {!Event} e
* @param {!Element} target
* @param {!Location} tgtLoc
* @param {!./service/ampdoc-impl.AmpDoc} ampdoc
* @param {boolean} isIosSafari
* @private
*/
function handleCustomProtocolClick_(e, target, tgtLoc, ampdoc, isIosSafari) {
/** @const {!Window} */
const win = ampdoc.win;
// On Safari iOS, custom protocol links will fail to open apps when the
// document is iframed - in order to go around this, we set the top.location
// to the custom protocol href.
Expand All @@ -141,11 +164,21 @@ export function onDocumentElementClick_(
// in the case where there's no app to handle the custom protocol.
e.preventDefault();
}
}

if (!tgtLoc.hash) {
return;
}

/**
* Handles clicking on a link with hash navigation.
* @param {!Event} e
* @param {!Location} tgtLoc
* @param {!./service/ampdoc-impl.AmpDoc} ampdoc
* @param {!./service/viewport-impl.Viewport} viewport
* @param {!./service/history-impl.History} history
* @private
*/
function handleHashClick_(e, tgtLoc, ampdoc, viewport, history) {
/** @const {!Window} */
const win = ampdoc.win;
/** @const {!Location} */
const curLoc = parseUrl(win.location.href);
const tgtHref = `${tgtLoc.origin}${tgtLoc.pathname}${tgtLoc.search}`;
Expand All @@ -168,7 +201,7 @@ export function onDocumentElementClick_(
let elem = null;

if (hash) {
const escapedHash = escapeCssSelectorIdent(ampdoc.win, hash);
const escapedHash = escapeCssSelectorIdent(win, hash);
elem = (ampdoc.getRootNode().getElementById(hash) ||
// Fallback to anchor[name] if element with id is not found.
// Linking to an anchor element with name is obsolete in html5.
Expand All @@ -177,14 +210,25 @@ export function onDocumentElementClick_(

// If possible do update the URL with the hash. As explained above
// we do `replace` to avoid messing with the container's history.
// The choice of `location.replace` vs `history.replaceState` is important.
// Due to bugs, not every browser triggers `:target` pseudo-class when
// `replaceState` is called. See http://www.zachleat.com/web/moving-target/
// for more details. Do this only if fragment has changed.
if (tgtLoc.hash != curLoc.hash) {
win.location.replace(`#${hash}`);
history.replaceStateForTarget(tgtLoc.hash).then(() => {
scrollToElement(elem, win, viewport, hash);
});
} else {
// If the hash did not update just scroll to the element.
scrollToElement(elem, win, viewport, hash);
}
}


/**
* Scrolls the page to the given element.
* @param {?Element} elem
* @param {!Window} win
* @param {!./service/viewport-impl.Viewport} viewport
* @param {string} hash
*/
function scrollToElement(elem, win, viewport, hash) {
// Scroll to the element if found.
if (elem) {
// The first call to scrollIntoView overrides browsers' default
Expand All @@ -203,11 +247,4 @@ export function onDocumentElementClick_(
dev().warn('HTML',
`failed to find element with id=${hash} or a[name=${hash}]`);
}

if (tgtLoc.hash != curLoc.hash) {
// Push/pop history.
history.push(() => {
win.location.replace(`${curLoc.hash || '#'}`);
});
}
}

0 comments on commit ff7b32b

Please sign in to comment.