Skip to content

Commit 4a98ed8

Browse files
authored
πŸ› amp-story: Remove duplicate replaceState on navigation (ampproject#26624)
* throttle replaceState and get rid of page_id usage * fix test * move throttle to amp-story * apply throttle to all replace * use history buffer * make caller provide own throttle * clear settimeout after each test * Revert "make caller provide own throttle" This reverts commit 7213df4. * remove throttle
1 parent 19cedca commit 4a98ed8

File tree

3 files changed

+8
-26
lines changed

3 files changed

+8
-26
lines changed

β€Žextensions/amp-story/1.0/amp-story.jsβ€Ž

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ import {dev, devAssert, user} from '../../../src/log';
104104
import {dict, map} from '../../../src/utils/object';
105105
import {endsWith} from '../../../src/string';
106106
import {escapeCssSelectorIdent} from '../../../src/css';
107-
import {findIndex} from '../../../src/utils/array';
107+
import {findIndex, lastItem} from '../../../src/utils/array';
108108
import {getConsentPolicyState} from '../../../src/consent';
109109
import {getDetail} from '../../../src/event-helper';
110110
import {getMediaQueryService} from './amp-story-media-query-service';
@@ -738,10 +738,6 @@ export class AmpStory extends AMP.BaseElement {
738738
this.onBookendStateUpdate_(isActive);
739739
});
740740

741-
this.storeService_.subscribe(StateProperty.CURRENT_PAGE_ID, pageId => {
742-
this.onCurrentPageIdUpdate_(pageId);
743-
});
744-
745741
this.storeService_.subscribe(StateProperty.PAUSED_STATE, isPaused => {
746742
this.onPausedStateUpdate_(isPaused);
747743
});
@@ -1068,16 +1064,17 @@ export class AmpStory extends AMP.BaseElement {
10681064
* @private
10691065
*/
10701066
getInitialPageId_(firstPageEl) {
1071-
const historyPage = /** @type {string} */ (getHistoryState(
1072-
this.win,
1073-
HistoryState.PAGE_ID
1074-
));
1075-
10761067
const maybePageId = parseQueryString(this.win.location.hash)['page'];
10771068
if (maybePageId && this.isActualPage_(maybePageId)) {
10781069
return maybePageId;
10791070
}
10801071

1072+
const pages =
1073+
/** @type {!Array} */ (getHistoryState(
1074+
this.win,
1075+
HistoryState.NAVIGATION_PATH
1076+
) || []);
1077+
const historyPage = lastItem(pages);
10811078
if (historyPage && this.isActualPage_(historyPage)) {
10821079
return historyPage;
10831080
}
@@ -1682,20 +1679,6 @@ export class AmpStory extends AMP.BaseElement {
16821679
}
16831680
}
16841681

1685-
/**
1686-
* @param {string} pageId new current page id
1687-
* @private
1688-
* */
1689-
onCurrentPageIdUpdate_(pageId) {
1690-
// Never save ad pages to history as they are unique to each visit.
1691-
const page = this.getPageById(pageId);
1692-
if (page.isAd()) {
1693-
return;
1694-
}
1695-
1696-
setHistoryState(this.win, HistoryState.PAGE_ID, pageId);
1697-
}
1698-
16991682
/**
17001683
* Handle resize events and set the story's desktop state.
17011684
* @visibleForTesting

β€Žextensions/amp-story/1.0/test/test-amp-story.jsβ€Ž

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ describes.realWin(
352352

353353
await story.layoutCallback();
354354
expect(replaceStateStub).to.have.been.calledWith(
355-
{ampStoryPageId: firstPageId},
355+
{ampStoryNavigationPath: [firstPageId]},
356356
''
357357
);
358358
});

β€Žextensions/amp-story/1.0/utils.jsβ€Ž

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,6 @@ export function resolveImgSrc(win, url) {
256256
export const HistoryState = {
257257
ATTACHMENT_PAGE_ID: 'ampStoryAttachmentPageId',
258258
BOOKEND_ACTIVE: 'ampStoryBookendActive',
259-
PAGE_ID: 'ampStoryPageId',
260259
NAVIGATION_PATH: 'ampStoryNavigationPath',
261260
};
262261

0 commit comments

Comments
Β (0)