Skip to content

Commit

Permalink
Revert renaming and fixes (separate PR)
Browse files Browse the repository at this point in the history
  • Loading branch information
wassgha committed Jan 24, 2020
1 parent 909bb9e commit 8625b5a
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 51 deletions.
16 changes: 3 additions & 13 deletions extensions/amp-next-page/1.0/amp-next-page.css
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,16 @@
overflow: hidden;
}

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

.i-amphtml-next-page-document.amp-next-page-visible {
.i-amphtml-next-page-document.amp-next-page-document-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 {
position: relative;
display: flex;
Expand Down
6 changes: 1 addition & 5 deletions extensions/amp-next-page/1.0/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ export const PageState = {
PAUSED: 5,
};

export const VISIBLE_DOC_CLASS = 'amp-next-page-visible';
export const HIDDEN_DOC_CLASS = 'amp-next-page-hidden';
export const VISIBLE_DOC_CLASS = 'amp-next-page-document-visible';

export class Page {
/**
Expand Down Expand Up @@ -159,9 +158,6 @@ export class Page {
this.shadowDoc_.ampdoc
.getBody()
.classList.toggle(VISIBLE_DOC_CLASS, this.isVisible());
this.shadowDoc_.ampdoc
.getBody()
.classList.toggle(HIDDEN_DOC_CLASS, !this.isVisible());
}

if (this.isVisible()) {
Expand Down
31 changes: 16 additions & 15 deletions extensions/amp-next-page/1.0/service.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import {CSS} from '../../../build/amp-next-page-1.0.css';
import {HIDDEN_DOC_CLASS, HostPage, Page, PageState} from './page';
import {HostPage, Page, PageState} from './page';
import {MultidocManager} from '../../../src/multidoc-manager';
import {Services} from '../../../src/services';
import {VisibilityState} from '../../../src/visibility-state';
Expand Down Expand Up @@ -428,7 +428,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 @@ -540,22 +540,20 @@ export class NextPageService {
removeElement(el);
});

doc.body.classList.add(HIDDEN_DOC_CLASS);

// Make sure all hidden elements are initially invisible
this.toggleHiddenAndReplaceableElements(doc, false /** isVisible */);
}

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

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

if (
Expand Down Expand Up @@ -800,7 +798,10 @@ export class NextPageService {
* @private
*/
getMoreBoxElement_(element) {
const providedMoreBox = childElementByAttr(element, 'more-box');
const providedMoreBox = childElementByAttr(
element,
'amp-next-page-more-box'
);
// TODO(wassgha): Use templates (amp-mustache) to render the more box
if (providedMoreBox) {
removeElement(providedMoreBox);
Expand All @@ -815,7 +816,7 @@ export class NextPageService {
buildDefaultMoreBox_() {
// TODO(wassgha): Better default more box
const moreBox = this.win_.document.createElement('div');
moreBox.classList.add('amp-next-page-default-more-box');
moreBox.classList.add('amp-next-page-more-box');
return moreBox;
}
}
6 changes: 3 additions & 3 deletions extensions/amp-next-page/1.0/test/test-amp-next-page.js
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ describes.realWin(
it('adds the hidden class to elements that should be hidden', async () => {
await fetchDocuments(
service,
`${MOCK_NEXT_PAGE} <div next-page-hide id="hidden" />`
`${MOCK_NEXT_PAGE} <div amp-next-page-hide id="hidden" />`
);

expect(
Expand All @@ -333,14 +333,14 @@ describes.realWin(
it('replaces elements with their most recent instance', async () => {
await fetchDocuments(
service,
`${MOCK_NEXT_PAGE} <div next-page-replace="replace-me" instance="1" />`,
`${MOCK_NEXT_PAGE} <div amp-next-page-replace="replace-me" instance="1" />`,
'/document1'
);
service.pages_[1].setVisibility(VisibilityState.VISIBLE);

await fetchDocuments(
service,
`${MOCK_NEXT_PAGE} <div next-page-replace="replace-me" instance="2" />`,
`${MOCK_NEXT_PAGE} <div amp-next-page-replace="replace-me" instance="2" />`,
'/document2'
);
service.pages_[1].relativePos = ViewportRelativePos.INSIDE_VIEWPORT;
Expand Down
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" 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" amp-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" 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>
<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>
<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 @@ -309,7 +309,7 @@ <h2>Content discovery</h2>
<div separator>
Read more
</div>
<div more-box>
<div amp-next-page-more-box>
Here are a few more articles ...
</div>
</amp-next-page>
Expand Down
2 changes: 1 addition & 1 deletion test/manual/amp-next-page/1.0/amp-next-page.amp.html
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ <h2>Host page</h2>
</div>
</template>
</div>
<div more-box>
<div amp-next-page-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" 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" amp-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" 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>
<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>
<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 @@ -303,7 +303,7 @@ <h2>Content discovery</h2>
<div separator>
Read more
</div>
<div more-box>
<div amp-next-page-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" 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" amp-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" 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>
<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>
<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" 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" amp-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" 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>
<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>
<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 8625b5a

Please sign in to comment.