Skip to content

Commit 13a2e75

Browse files
gmajouletgmajoulet
authored
Story branching support for sidebar links. (ampproject#24968)
* Squash Pet's commits. * Links in sidebar fixes.
1 parent ce1878f commit 13a2e75

File tree

3 files changed

+82
-30
lines changed

3 files changed

+82
-30
lines changed

‎examples/visual-tests/amp-story/amp-story-sidebar.html‎

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
<style amp-custom>
1515
amp-story-page {
1616
background: white;
17-
color: white;
17+
color: rgba(0, 0, 0, 0.87);
1818
font-family: sans-serif;
1919
}
2020
.i-amphtml-story-spinner-container {
@@ -26,8 +26,8 @@
2626
text-shadow: 3px 3px rgba(0, 0, 0, 0.5);
2727
}
2828
amp-sidebar {
29-
font-size: 2rem;
30-
background-color: black;
29+
font-size: 18px;
30+
background-color: #fff;
3131
}
3232
amp-sidebar div {
3333
border: 3px solid green;
@@ -37,30 +37,29 @@
3737
</head>
3838

3939
<body>
40-
<amp-story standalone publisher="AMP Team" title="Visual Diff Test"
40+
<amp-story id="story" standalone publisher="AMP Team" title="Visual Diff Test"
4141
publisher-logo-src="/examples/visual-tests/photos/120.png"
4242
poster-portrait-src="/examples/visual-tests/picsum.photos/image981_900x1600.jpg"
4343
poster-landscape-src="/examples/visual-tests/picsum.photos/image981_1600x900.jpg"
4444
poster-square-src="/examples/visual-tests/picsum.photos/image981_1600x1600.jpg">
45-
<amp-sidebar id="sidebar1" layout="nodisplay">
46-
<div>
47-
<button id="close-button" on="tap:sidebar1.close"> close </button>
48-
<ul>
49-
<li>Nav item 1</li>
50-
<li><a href="#idTwo" on="tap:idTwo.scrollTo">Nav item 2</a></li>
51-
<li>Nav item 3</li>
52-
<li><a href="#idFour" on="tap:idFour.scrollTo">Nav item 4</a></li>
53-
<li>Nav item 5</li>
54-
<li>Nav item 6</li>
55-
</ul>
56-
</div>
45+
<amp-sidebar id="sidebar1" layout="nodisplay" >
46+
<button id="close-button" on="tap:sidebar1.close">Close</button>
47+
<ul>
48+
<li><span><a href="http://google.com">Out item 1</a></span></li>
49+
<li><span><a href="/examples/visual-tests/amp-story/amp-story-sidebar.html#page=page-2&foo=bar">Absolute fragment link</a></span></li>
50+
<li><a href="/examples/visual-tests/amp-story/amp-story-tooltip.html#page=page-2">Link to another story using fragment</a></li>
51+
<li><a href="#page=page-3&foo=bar">In Link Hash</a></li>
52+
<li on="tap:story.goToPage(id=page-3)">In Link Action</li>
53+
<li>Nav item 5</li>
54+
<li>Nav item 6</li>
55+
</ul>
5756
</amp-sidebar>
5857
<amp-story-page id="cover">
5958
<amp-story-grid-layer template="fill">
6059
</amp-story-grid-layer>
6160
<amp-story-grid-layer template="vertical">
6261
<h1 class="hello-world">Hello world!</h1>
63-
<p>Page one of two</p>
62+
<p>Page one of three</p>
6463
</amp-story-grid-layer>
6564
</amp-story-page>
6665

@@ -69,10 +68,18 @@ <h1 class="hello-world">Hello world!</h1>
6968
</amp-story-grid-layer>
7069
<amp-story-grid-layer template="vertical">
7170
<h1 class="hello-world">Hello world!</h1>
72-
<p>Page two of two</p>
71+
<p>Page two of three</p>
72+
</amp-story-grid-layer>
73+
</amp-story-page>
74+
75+
<amp-story-page id="page-3">
76+
<amp-story-grid-layer template="fill">
77+
</amp-story-grid-layer>
78+
<amp-story-grid-layer template="vertical">
79+
<h1 class="hello-world">Hello world!</h1>
80+
<p>Page three of three</p>
7381
</amp-story-grid-layer>
7482
</amp-story-page>
7583

7684
</amp-story>
7785
</body>
78-

‎extensions/amp-story/1.0/amp-story.js‎

Lines changed: 53 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -103,12 +103,14 @@ import {createPseudoLocale} from '../../../src/localized-strings';
103103
import {debounce} from '../../../src/utils/rate-limit';
104104
import {dev, devAssert, user} from '../../../src/log';
105105
import {dict, map} from '../../../src/utils/object';
106+
import {endsWith} from '../../../src/string';
106107
import {escapeCssSelectorIdent} from '../../../src/css';
107108
import {findIndex} from '../../../src/utils/array';
108109
import {getConsentPolicyState} from '../../../src/consent';
109110
import {getDetail} from '../../../src/event-helper';
110111
import {getMediaQueryService} from './amp-story-media-query-service';
111112
import {getMode} from '../../../src/mode';
113+
import {getState} from '../../../src/history';
112114
import {isExperimentOn} from '../../../src/experiments';
113115
import {parseQueryString} from '../../../src/url';
114116
import {registerServiceBuilder} from '../../../src/service';
@@ -437,13 +439,20 @@ export class AmpStory extends AMP.BaseElement {
437439
if (isExperimentOn(this.win, 'amp-story-branching')) {
438440
this.registerAction('goToPage', invocation => {
439441
const {args} = invocation;
440-
if (args) {
441-
this.storeService_.dispatch(
442-
Action.SET_ADVANCEMENT_MODE,
443-
AdvancementMode.GO_TO_PAGE
444-
);
445-
this.switchTo_(args['id'], NavigationDirection.NEXT);
442+
if (!args) {
443+
return;
446444
}
445+
this.storeService_.dispatch(
446+
Action.SET_ADVANCEMENT_MODE,
447+
AdvancementMode.GO_TO_PAGE
448+
);
449+
// If open, closes the sidebar before navigating.
450+
const promise = this.storeService_.get(StateProperty.SIDEBAR_STATE)
451+
? Services.historyForDoc(this.getAmpDoc()).goBack()
452+
: Promise.resolve();
453+
promise.then(() =>
454+
this.switchTo_(args['id'], NavigationDirection.NEXT)
455+
);
447456
});
448457
}
449458
}
@@ -787,6 +796,29 @@ export class AmpStory extends AMP.BaseElement {
787796

788797
this.getAmpDoc().onVisibilityChanged(() => this.onVisibilityChanged_());
789798

799+
if (isExperimentOn(this.win, 'amp-story-branching')) {
800+
this.win.addEventListener('hashchange', () => {
801+
const maybePageId = parseQueryString(this.win.location.hash)['page'];
802+
if (!maybePageId || !this.isActualPage_(maybePageId)) {
803+
return;
804+
}
805+
this.switchTo_(maybePageId, NavigationDirection.NEXT);
806+
// Removes the page 'hash' parameter from the URL.
807+
let href = this.win.location.href.replace(
808+
new RegExp(`page=${maybePageId}&?`),
809+
''
810+
);
811+
if (endsWith(href, '#')) {
812+
href = href.slice(0, -1);
813+
}
814+
this.win.history.replaceState(
815+
(this.win.history && getState(this.win.history)) || {} /** data */,
816+
this.win.document.title /** title */,
817+
href /** URL */
818+
);
819+
});
820+
}
821+
790822
this.getViewport().onResize(debounce(this.win, () => this.onResize(), 300));
791823
this.installGestureRecognizers_();
792824
}
@@ -1049,27 +1081,38 @@ export class AmpStory extends AMP.BaseElement {
10491081
* @private
10501082
*/
10511083
getInitialPageId_(firstPageEl) {
1052-
const isActualPage = pageId =>
1053-
!!this.element.querySelector(`#${escapeCssSelectorIdent(pageId)}`);
10541084
const historyPage = /** @type {string} */ (getHistoryState(
10551085
this.win,
10561086
HistoryState.PAGE_ID
10571087
));
10581088

10591089
if (isExperimentOn(this.win, 'amp-story-branching')) {
10601090
const maybePageId = parseQueryString(this.win.location.hash)['page'];
1061-
if (maybePageId && isActualPage(maybePageId)) {
1091+
if (maybePageId && this.isActualPage_(maybePageId)) {
10621092
return maybePageId;
10631093
}
10641094
}
10651095

1066-
if (historyPage && isActualPage(historyPage)) {
1096+
if (historyPage && this.isActualPage_(historyPage)) {
10671097
return historyPage;
10681098
}
10691099

10701100
return firstPageEl.id;
10711101
}
10721102

1103+
/**
1104+
* Checks if the amp-story-page for a given ID exists.
1105+
* Note: the `this.pages_` array might not be defined yet.
1106+
* @param {string} pageId
1107+
* @return {boolean}
1108+
* @private
1109+
*/
1110+
isActualPage_(pageId) {
1111+
// TODO(gmajoulet): check from the cached pages array if available, and use
1112+
// the querySelector as a fallback.
1113+
return !!this.element.querySelector(`#${escapeCssSelectorIdent(pageId)}`);
1114+
}
1115+
10731116
/**
10741117
* @param {number} timeoutMs The maximum amount of time to wait, in
10751118
* milliseconds.

‎extensions/amp-story/1.0/test/test-amp-story.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1581,6 +1581,8 @@ describes.realWin(
15811581
element.querySelector('#cover').appendChild(actionButton);
15821582
// Click on the actionButton to trigger the goToPage action.
15831583
actionButton.click();
1584+
// Next tick.
1585+
await Promise.resolve();
15841586
expect(story.activePage_.element.id).to.equal('page-2');
15851587
});
15861588

@@ -1612,7 +1614,7 @@ describes.realWin(
16121614
expect(story.activePage_.element.id).to.equal('cover');
16131615
});
16141616

1615-
it.skip('should navigate back to the correct previous page after advance-to', async () => {
1617+
it('should navigate back to the correct previous page after advance-to', async () => {
16161618
await createStoryWithPages(4, [
16171619
'cover',
16181620
'page-1',

0 commit comments

Comments
 (0)