Skip to content

Commit

Permalink
Implemented a default separator pill and templating for the separator
Browse files Browse the repository at this point in the history
  • Loading branch information
wassgha committed Jan 20, 2020
1 parent 6fb9245 commit 0a39ed3
Show file tree
Hide file tree
Showing 8 changed files with 121 additions and 47 deletions.
31 changes: 28 additions & 3 deletions extensions/amp-next-page/1.0/amp-next-page.css
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,41 @@
overflow: hidden;
}

.i-amphtml-next-page-document:not(.amp-next-page-document-visible)
[i-amphtml-fixedid] {
.i-amphtml-next-page-document:not(.next-page-visible) [i-amphtml-fixedid] {
display: none;
}

.i-amphtml-next-page-document.amp-next-page-document-visible {
.i-amphtml-next-page-document.next-page-visible {
opacity: 1;
overflow: visible;
}

/** Fixes collapsing margins when switching between the visible and hidden states */
.i-amphtml-next-page-document:before,
.i-amphtml-next-page-document:after {
content: ' ';
display: table;
}

.i-amphtml-next-page-placeholder {
background: #eee;
}

.amp-next-page-default-separator {
display: flex;
flex-direction: row;
justify-content: center;
align-items: center;
background: black;
color: white;
box-shadow: 0px 6px 17px 0px rgba(0, 0, 0, 0.09);
padding: 12px;
border-radius: 32px;
text-align: center;
font-family: 'Roboto', Helvetica, Arial, sans-serif;
font-weight: 400;
font-size: 14px;
width: 160px;
margin: auto;
z-index: 2147483645;
}
2 changes: 1 addition & 1 deletion extensions/amp-next-page/1.0/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export const PageState = {
PAUSED: 5,
};

const VISIBLE_DOC_CLASS = 'amp-next-page-document-visible';
const VISIBLE_DOC_CLASS = 'next-page-visible';

export class Page {
/**
Expand Down
73 changes: 50 additions & 23 deletions extensions/amp-next-page/1.0/service.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
childElementByAttr,
childElementsByTag,
isJsonScriptTag,
removeChildren,
removeElement,
scopedQuerySelector,
} from '../../../src/dom';
Expand Down Expand Up @@ -77,6 +78,9 @@ export class NextPageService {
*/
this.mutator_ = Services.mutatorForDoc(ampdoc);

/** @private @const {!../../../src/service/template-impl.Templates} */
this.templates_ = Services.templatesFor(this.win_);

/** @private {?Element} */
this.separator_ = null;

Expand Down Expand Up @@ -383,13 +387,16 @@ export class NextPageService {
const container = this.win_.document.createElement('div');
container.classList.add(DOC_CONTAINER_CLASS);

// Insert the separator
const separatorInstance = this.separator_.cloneNode(true);
container.appendChild(separatorInstance);
this.maybeRenderSeparatorTemplate_(separatorInstance, page);

// Insert the document
const shadowRoot = this.win_.document.createElement('div');
shadowRoot.classList.add(SHADOW_ROOT_CLASS);
container.appendChild(shadowRoot);

// Insert the separator
container.appendChild(this.separator_.cloneNode(true));

// Insert the container
this.element_.insertBefore(container, this.moreBox_);

Expand All @@ -416,7 +423,7 @@ export class NextPageService {
*/
attachDocumentToPage(page, content, force = false) {
// If the user already scrolled to the bottom, prevent rendering
if (this.getViewportsAway_() <= NEAR_BOTTOM_VIEWPORT_COUNT && !force) {
if (this.getViewportsAway_() < NEAR_BOTTOM_VIEWPORT_COUNT && !force) {
// TODO(wassgha): Append a "load next article" button?
return null;
}
Expand Down Expand Up @@ -533,15 +540,15 @@ export class NextPageService {
}

/**
* Hides or shows elements based on the `amp-next-page-hide` and
* `amp-next-page-replace` attributes
* Hides or shows elements based on the `next-page-hide` and
* `next-page-replace` attributes
* @param {!Document|!ShadowRoot} doc Document of interest
* @param {boolean=} isVisible Whether this page is visible or not
*/
toggleHiddenAndReplaceableElements(doc, isVisible = true) {
// Hide elements that have [amp-next-page-hide] on child documents
// Hide elements that have [next-page-hide] on child documents
if (doc !== this.hostPage_.document) {
toArray(doc.querySelectorAll('[amp-next-page-hide]')).forEach(element =>
toArray(doc.querySelectorAll('[next-page-hide]')).forEach(element =>
toggle(element, false /** opt_display */)
);
}
Expand All @@ -551,14 +558,14 @@ export class NextPageService {
return;
}

// Replace elements that have [amp-next-page-replace]
// Replace elements that have [next-page-replace]
toArray(
doc.querySelectorAll('*:not(amp-next-page) [amp-next-page-replace]')
doc.querySelectorAll('*:not(amp-next-page) [next-page-replace]')
).forEach(element => {
let uniqueId = element.getAttribute('amp-next-page-replace');
let uniqueId = element.getAttribute('next-page-replace');
if (!uniqueId) {
uniqueId = String(Date.now() + Math.floor(Math.random() * 100));
element.setAttribute('amp-next-page-replace', uniqueId);
element.setAttribute('next-page-replace', uniqueId);
}

if (
Expand Down Expand Up @@ -725,11 +732,7 @@ export class NextPageService {
* @private
*/
getSeparatorElement_(element) {
const providedSeparator = childElementByAttr(
element,
'amp-next-page-separator'
);
// TODO(wassgha): Use templates (amp-mustache) to render the separator
const providedSeparator = childElementByAttr(element, 'separator');
if (providedSeparator) {
removeElement(providedSeparator);
}
Expand All @@ -742,20 +745,44 @@ export class NextPageService {
*/
buildDefaultSeparator_() {
const separator = this.win_.document.createElement('div');
separator.classList.add('amp-next-page-separator');
separator.innerText = 'Next article';
separator.classList.add('amp-next-page-default-separator');
return separator;
}

/**
* Renders the template inside the separator element using
* data from the current article (if a template is present)
*
* @param {!Element} separator
* @param {!Page} page
* @return {!Promise}
*/
maybeRenderSeparatorTemplate_(separator, page) {
if (!this.templates_.hasTemplate(separator)) {
return Promise.resolve();
}
this.templates_
.findAndRenderTemplate(separator, {
title: page.title,
url: page.url,
image: page.image,
})
.then(rendered => {
return this.mutator_.mutateElement(separator, () => {
removeChildren(dev().assertElement(separator));
separator.appendChild(rendered);
});
});
}

/**
* @param {!Element} element the container of the amp-next-page extension
* @return {!Element}
* @private
*/
getMoreBoxElement_(element) {
const providedMoreBox = childElementByAttr(
element,
'amp-next-page-more-box'
);
const providedMoreBox = childElementByAttr(element, 'more-box');
// TODO(wassgha): Use templates (amp-mustache) to render the more box
if (providedMoreBox) {
removeElement(providedMoreBox);
Expand All @@ -770,7 +797,7 @@ export class NextPageService {
buildDefaultMoreBox_() {
// TODO(wassgha): Better default more box
const moreBox = this.win_.document.createElement('div');
moreBox.classList.add('amp-next-page-more-box');
moreBox.classList.add('amp-next-page-default-more-box');
return moreBox;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ <h2>Content discovery</h2>
varius est suscipit vitae. Maecenas ut sapien diam. Vivamus viverra nisl
at quam pellentesque posuere. Cras ut nibh non arcu dignissim elementum.
</p>
<div class="hidden" amp-next-page-hide>I should be visible when loaded as a host page but hidden if loaded as a next-page</div>
<div class="hidden" next-page-hide>I should be visible when loaded as a host page but hidden if loaded as a next-page</div>
<p>
Vestibulum a nisi est. Fusce consequat iaculis vehicula. Aliquam luctus
nunc risus, ut congue augue tristique nec. In venenatis aliquam tristique.
Expand All @@ -77,8 +77,8 @@ <h2>Content discovery</h2>
enim, non pellentesque sem. Nam non sem lacinia, lacinia nisi non,
consectetur enim.
</p>
<div class="sticky" amp-next-page-replace="my-sticky-element">I should get replaced by my sibling from each loaded page (page 0)</div>
<div class="fixed" amp-next-page-hide>I am fixed and should be visible when loaded as a host page but hidden if loaded as a next-page (page 0)</div>
<div class="sticky" next-page-replace="my-sticky-element">I should get replaced by my sibling from each loaded page (page 0)</div>
<div class="fixed" next-page-hide>I am fixed and should be visible when loaded as a host page but hidden if loaded as a next-page (page 0)</div>
<p>
Maecenas sed quam nec dolor rhoncus rutrum ut non mauris. Pellentesque
ante dolor, pretium quis enim eu, condimentum volutpat augue. Donec nulla
Expand Down Expand Up @@ -306,10 +306,10 @@ <h2>Content discovery</h2>
]
}
</script>
<div amp-next-page-separator>
<div separator>
Read more
</div>
<div amp-next-page-more-box>
<div more-box>
Here are a few more articles ...
</div>
</amp-next-page>
Expand Down
30 changes: 26 additions & 4 deletions test/manual/amp-next-page/1.0/amp-next-page.amp.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
<script async src="https://cdn.ampproject.org/v0.js"></script>
<script async custom-element="amp-next-page" src="https://cdn.ampproject.org/v0/amp-next-page-1.0.js"></script>
<script async custom-template="amp-mustache" src="https://cdn.ampproject.org/v0/amp-mustache-0.2.js"></script>
<script async custom-element="amp-analytics" src="https://cdn.ampproject.org/v0/amp-analytics-0.1.js"></script>
<script>
(self.AMP=self.AMP||[]).push(function(AMP){
Expand All @@ -19,6 +20,21 @@
.content {
padding: 20px;
}
.amp-next-page-default-separator {
margin-top: 16px;
margin-bottom: 16px;
}
.separator-content {
display: flex;
flex-direction: row;
justify-content: center;
align-items: center;
}
.separator-content amp-img {
display: inline-block;
border-radius: 50%;
margin-right: 6px;
}
</style>
</head>
<body>
Expand Down Expand Up @@ -258,7 +274,8 @@ <h2>Host page</h2>
quis metus in mi pharetra tempus. Etiam dapibus tellus vitae blandit
rhoncus. Fusce commodo risus id sapien ultrices vehicula.
</p>
</div>
</div>
<!-- amp-next-page configuration -->
<amp-next-page>
<script type="application/json">
[
Expand All @@ -284,10 +301,15 @@ <h2>Host page</h2>
}
]
</script>
<div amp-next-page-separator>
Read more
<div separator class="amp-next-page-default-separator">
<template type="amp-mustache">
<div class="separator-content">
<amp-img src={{image}} layout="fixed" height="16" width="16"></amp-img>
<span>Next article: {{title}}</span>
</div>
</template>
</div>
<div amp-next-page-more-box>
<div more-box>
Here are a few more articles ...
</div>
</amp-next-page>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ <h2>Content discovery</h2>
varius est suscipit vitae. Maecenas ut sapien diam. Vivamus viverra nisl
at quam pellentesque posuere. Cras ut nibh non arcu dignissim elementum.
</p>
<div class="hidden" amp-next-page-hide>I should be visible when loaded as a host page but hidden if loaded as a next-page</div>
<div class="hidden" next-page-hide>I should be visible when loaded as a host page but hidden if loaded as a next-page</div>
<p>
Vestibulum a nisi est. Fusce consequat iaculis vehicula. Aliquam luctus
nunc risus, ut congue augue tristique nec. In venenatis aliquam tristique.
Expand All @@ -76,8 +76,8 @@ <h2>Content discovery</h2>
enim, non pellentesque sem. Nam non sem lacinia, lacinia nisi non,
consectetur enim.
</p>
<div class="sticky" amp-next-page-replace="my-sticky-element">I should get replaced by my sibling from each loaded page (page 0)</div>
<div class="fixed" amp-next-page-hide>I am fixed and should be visible when loaded as a host page but hidden if loaded as a next-page (page 0)</div>
<div class="sticky" next-page-replace="my-sticky-element">I should get replaced by my sibling from each loaded page (page 0)</div>
<div class="fixed" next-page-hide>I am fixed and should be visible when loaded as a host page but hidden if loaded as a next-page (page 0)</div>
<p>
Maecenas sed quam nec dolor rhoncus rutrum ut non mauris. Pellentesque
ante dolor, pretium quis enim eu, condimentum volutpat augue. Donec nulla
Expand Down Expand Up @@ -300,10 +300,10 @@ <h2>Content discovery</h2>
}
]
</script>
<div amp-next-page-separator>
<div separator>
Read more
</div>
<div amp-next-page-more-box>
<div more-box>
Here are a few more articles ...
</div>
</amp-next-page>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ <h2>Content discovery</h2>
varius est suscipit vitae. Maecenas ut sapien diam. Vivamus viverra nisl
at quam pellentesque posuere. Cras ut nibh non arcu dignissim elementum.
</p>
<div class="hidden" amp-next-page-hide>I should be visible when loaded as a host page but hidden if loaded as a next-page</div>
<div class="hidden" next-page-hide>I should be visible when loaded as a host page but hidden if loaded as a next-page</div>
<p>
Vestibulum a nisi est. Fusce consequat iaculis vehicula. Aliquam luctus
nunc risus, ut congue augue tristique nec. In venenatis aliquam tristique.
Expand All @@ -70,8 +70,8 @@ <h2>Content discovery</h2>
enim, non pellentesque sem. Nam non sem lacinia, lacinia nisi non,
consectetur enim.
</p>
<div class="sticky" amp-next-page-replace="my-sticky-element">I got replaced by my sibling from page 1</div>
<div class="fixed" amp-next-page-hide>I am fixed and should be visible when loaded as a host page but hidden if loaded as a next-page (page 1)</div>
<div class="sticky" next-page-replace="my-sticky-element">I got replaced by my sibling from page 1</div>
<div class="fixed" next-page-hide>I am fixed and should be visible when loaded as a host page but hidden if loaded as a next-page (page 1)</div>
<p>
Maecenas sed quam nec dolor rhoncus rutrum ut non mauris. Pellentesque
ante dolor, pretium quis enim eu, condimentum volutpat augue. Donec nulla
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ <h2>Content discovery</h2>
varius est suscipit vitae. Maecenas ut sapien diam. Vivamus viverra nisl
at quam pellentesque posuere. Cras ut nibh non arcu dignissim elementum.
</p>
<div class="hidden" amp-next-page-hide>I should be visible when loaded as a host page but hidden if loaded as a next-page</div>
<div class="hidden" next-page-hide>I should be visible when loaded as a host page but hidden if loaded as a next-page</div>
<p>
Vestibulum a nisi est. Fusce consequat iaculis vehicula. Aliquam luctus
nunc risus, ut congue augue tristique nec. In venenatis aliquam tristique.
Expand All @@ -70,8 +70,8 @@ <h2>Content discovery</h2>
enim, non pellentesque sem. Nam non sem lacinia, lacinia nisi non,
consectetur enim.
</p>
<div class="sticky" amp-next-page-replace="my-sticky-element">I got replaced by my sibling from page 2</div>
<div class="fixed" amp-next-page-hide>I am fixed and should be visible when loaded as a host page but hidden if loaded as a next-page (page 2)</div>
<div class="sticky" next-page-replace="my-sticky-element">I got replaced by my sibling from page 2</div>
<div class="fixed" next-page-hide>I am fixed and should be visible when loaded as a host page but hidden if loaded as a next-page (page 2)</div>
<p>
Maecenas sed quam nec dolor rhoncus rutrum ut non mauris. Pellentesque
ante dolor, pretium quis enim eu, condimentum volutpat augue. Donec nulla
Expand Down

0 comments on commit 0a39ed3

Please sign in to comment.