Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

✨ [Amp story] Add amp-story-page-outlink component #34171

Merged
merged 11 commits into from
May 7, 2021
Merged

✨ [Amp story] Add amp-story-page-outlink component #34171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7069a4a
Fix conflicts.
processprocess May 6, 2021
09cbb49
Advancement, test and comment.
processprocess May 3, 2021
a16447f
querySelector
processprocess May 3, 2021
2c7be87
Fix conflicts.
processprocess May 6, 2021
fc5f2ad
Validator, component and template.
processprocess May 3, 2021
a9ce7bc
Fix merge conflicts.
processprocess May 6, 2021
c436de5
Remove duplicate code.
processprocess May 6, 2021
ba5dc64
Outlink validator rule.
processprocess May 6, 2021
3b49e8d
Alphabetize
processprocess May 6, 2021
b009318
Merge branch 'main' of github.com:ampproject/amphtml into outlink
processprocess May 7, 2021
dc0b1b1
Merge branch 'main' of github.com:ampproject/amphtml into outlink
processprocess May 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 16 additions & 16 deletions examples/amp-story/attachment.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,11 +169,11 @@
<span data-text-background-color="#00A3AB">Enable experiment by pasting AMP.toggleExperiment('amp-story-page-attachment-ui-v2', true) in console and refreshing.</span>
</p>
</amp-story-grid-layer>
<amp-story-page-attachment
<amp-story-page-outlink
title="Link Description"
href="https://www.google.com"
layout="nodisplay">
</amp-story-page-attachment>
</amp-story-page-outlink>
</amp-story-page>

<amp-story-page id="outlink-custom-icon-custom-text">
Expand All @@ -186,13 +186,13 @@
<span data-text-background-color="#00A3AB">Enable experiment by pasting AMP.toggleExperiment('amp-story-page-attachment-ui-v2', true) in console and refreshing.</span>
</p>
</amp-story-grid-layer>
<amp-story-page-attachment
<amp-story-page-outlink
title="Link Description"
href="https://www.google.com"
cta-text="Shop at Forbes"
cta-image="img/logo.jpg"
layout="nodisplay">
</amp-story-page-attachment>
</amp-story-page-outlink>
</amp-story-page>

<amp-story-page id="outlink-no-image">
Expand All @@ -205,12 +205,12 @@
<span data-text-background-color="#00A3AB">Enable experiment by pasting AMP.toggleExperiment('amp-story-page-attachment-ui-v2', true) in console and refreshing.</span>
</p>
</amp-story-grid-layer>
<amp-story-page-attachment
<amp-story-page-outlink
title="Link Description"
href="https://www.google.com"
cta-image="none"
layout="nodisplay">
</amp-story-page-attachment>
</amp-story-page-outlink>
</amp-story-page>

<amp-story-page id="outlink-dark-theme">
Expand All @@ -223,12 +223,12 @@
<span data-text-background-color="#00A3AB">Enable experiment by pasting AMP.toggleExperiment('amp-story-page-attachment-ui-v2', true) in console and refreshing.</span>
</p>
</amp-story-grid-layer>
<amp-story-page-attachment
<amp-story-page-outlink
title="Link Description"
href="https://www.google.com"
theme="dark"
layout="nodisplay">
</amp-story-page-attachment>
</amp-story-page-outlink>
</amp-story-page>

<amp-story-page id="outlink-custom-background-color-pink">
Expand All @@ -241,14 +241,14 @@
<span data-text-background-color="#00A3AB">Enable experiment by pasting AMP.toggleExperiment('amp-story-page-attachment-ui-v2', true) in console and refreshing.</span>
</p>
</amp-story-grid-layer>
<amp-story-page-attachment
<amp-story-page-outlink
title="Link Description"
href="https://www.google.com"
theme="custom"
cta-accent-color="pink"
cta-accent-element="background"
layout="nodisplay">
</amp-story-page-attachment>
</amp-story-page-outlink>
</amp-story-page>

<amp-story-page id="outlink-custom-text-color-pink">
Expand All @@ -261,14 +261,14 @@
<span data-text-background-color="#00A3AB">Enable experiment by pasting AMP.toggleExperiment('amp-story-page-attachment-ui-v2', true) in console and refreshing.</span>
</p>
</amp-story-grid-layer>
<amp-story-page-attachment
<amp-story-page-outlink
title="Link Description"
href="https://www.google.com"
theme="custom"
cta-accent-color="pink"
cta-accent-element="text"
layout="nodisplay">
</amp-story-page-attachment>
</amp-story-page-outlink>
</amp-story-page>

<amp-story-page id="outlink-custom-background-color-navy">
Expand All @@ -281,14 +281,14 @@
<span data-text-background-color="#00A3AB">Enable experiment by pasting AMP.toggleExperiment('amp-story-page-attachment-ui-v2', true) in console and refreshing.</span>
</p>
</amp-story-grid-layer>
<amp-story-page-attachment
<amp-story-page-outlink
title="Link Description"
href="https://www.google.com"
theme="custom"
cta-accent-color="navy"
cta-accent-element="background"
layout="nodisplay">
</amp-story-page-attachment>
</amp-story-page-outlink>
</amp-story-page>

<amp-story-page id="outlink-custom-text-color-navy">
Expand All @@ -301,14 +301,14 @@
<span data-text-background-color="#00A3AB">Enable experiment by pasting AMP.toggleExperiment('amp-story-page-attachment-ui-v2', true) in console and refreshing.</span>
</p>
</amp-story-grid-layer>
<amp-story-page-attachment
<amp-story-page-outlink
title="Link Description"
href="https://www.google.com/search?q=why+do+cats+purr+so+loud"
theme="custom"
cta-accent-color="navy"
cta-accent-element="text"
layout="nodisplay">
</amp-story-page-attachment>
</amp-story-page-outlink>
</amp-story-page>

<amp-story-page id="cta-without-data-attributes">
Expand Down
3 changes: 2 additions & 1 deletion extensions/amp-story/1.0/amp-story-page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1739,8 +1739,9 @@ export class AmpStoryPage extends AMP.BaseElement {
* @private
*/
renderOpenAttachmentUI_() {
// AttachmentEl can be either amp-story-page-attachment or amp-story-page-outlink
const attachmentEl = this.element.querySelector(
'amp-story-page-attachment'
'amp-story-page-attachment, amp-story-page-outlink'
);
if (!attachmentEl) {
return;
Expand Down
1 change: 1 addition & 0 deletions extensions/amp-story/1.0/amp-story.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2985,5 +2985,6 @@ AMP.extension('amp-story', '1.0', (AMP) => {
AMP.registerElement('amp-story-grid-layer', AmpStoryGridLayer);
AMP.registerElement('amp-story-page', AmpStoryPage);
AMP.registerElement('amp-story-page-attachment', AmpStoryPageAttachment);
AMP.registerElement('amp-story-page-outlink', AmpStoryPageAttachment); // Shares codepath with amp-story-page-attachment.
AMP.registerServiceForDoc('amp-story-render', AmpStoryRenderService);
});
10 changes: 7 additions & 3 deletions extensions/amp-story/1.0/page-advancement.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,10 @@ export class ManualAdvancement extends AdvancementConfig {
(el) => {
tagName = el.tagName.toLowerCase();

if (tagName === 'amp-story-page-attachment') {
if (
tagName === 'amp-story-page-attachment' ||
tagName === 'amp-story-page-outlink'
) {
shouldHandleEvent = false;
return true;
}
Expand Down Expand Up @@ -502,7 +505,7 @@ export class ManualAdvancement extends AdvancementConfig {

/**
* For an element to trigger a tooltip it has to be descendant of
* amp-story-page but not of amp-story-cta-layer or amp-story-page-attachment.
* amp-story-page but not of amp-story-cta-layer, amp-story-page-attachment or amp-story-page-outlink.
* @param {!Event} event
* @param {!ClientRect} pageRect
* @return {boolean}
Expand Down Expand Up @@ -542,7 +545,8 @@ export class ManualAdvancement extends AdvancementConfig {

if (
tagName === 'amp-story-cta-layer' ||
tagName === 'amp-story-page-attachment'
tagName === 'amp-story-page-attachment' ||
tagName === 'amp-story-page-outlink'
) {
valid = false;
return false;
Expand Down
15 changes: 15 additions & 0 deletions extensions/amp-story/1.0/test/test-amp-story-page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,21 @@ describes.realWin('amp-story-page', {amp: {extensions}}, (env) => {
expect(openAttachmentEl.getAttribute('target')).to.eql('_top');
});

it('should build the open outlink UI with same codepath as page attachment', async () => {
const outlinkEl = win.document.createElement('amp-story-page-outlink');
outlinkEl.setAttribute('layout', 'nodisplay');
element.appendChild(outlinkEl);

page.buildCallback();
await page.layoutCallback();
page.setState(PageState.PLAYING);

const openoutlinkEl = element.querySelector(
'.i-amphtml-story-page-open-attachment'
);
expect(openoutlinkEl).to.exist;
});

it('should build the inline page attachment UI with one image', async () => {
toggleExperiment(win, 'amp-story-page-attachment-ui-v2', true);

Expand Down
88 changes: 69 additions & 19 deletions extensions/amp-story/validator-amp-story.protoascii
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,32 @@ tags: { # <amp-story-page-attachment> with href
spec_name: "amp-story-page-attachment[href]"
mandatory_ancestor: "AMP-STORY-PAGE"
mandatory_last_child: true
attrs: {
name: "cta-accent-color"
}
attrs: {
name: "cta-accent-element"
value: "background"
value: "text"
}
attrs: {
name: "cta-image"
value_url: {
protocol: "http"
protocol: "https"
}
}
attrs: {
name: "cta-text"
}
attrs: {
name: "href"
mandatory: true
value_url: {
protocol: "http"
protocol: "https"
}
}
attrs: {
name: "layout"
mandatory: true
Expand All @@ -859,51 +885,64 @@ tags: { # <amp-story-page-attachment> with href
value: "dark"
value: "light"
}
attrs: {
name: "cta-text"
}
attrs: {
name: "title"
}
child_tags: {
mandatory_num_child_tags: 0
}
}
tags: { # <amp-story-page-attachment> with no href
html_format: AMP
tag_name: "AMP-STORY-PAGE-ATTACHMENT"
spec_name: "amp-story-page-attachment"
mandatory_ancestor: "AMP-STORY-PAGE"
descendant_tag_list: "amp-story-page-attachment-allowed-descendants"
mandatory_last_child: true
attrs: {
name: "cta-accent-color"
name: "cta-text"
}
attrs: {
name: "cta-accent-element"
value: "background"
value: "text"
name: "title"
}
attrs: {
name: "href"
mandatory: true
name: "cta-image"
value_url: {
protocol: "http"
protocol: "https"
}
}
attrs: {
name: "cta-image"
name: "cta-image-2"
value_url: {
protocol: "http"
protocol: "https"
}
}
child_tags: {
mandatory_num_child_tags: 0
attrs: {
name: "layout"
mandatory: true
value: "nodisplay"
}
attrs: {
name: "theme"
value: "dark"
value: "light"
}
}
tags: { # <amp-story-page-attachment> with no href
tags: { # Same validation rules as `<amp-story-page-attachment> with href`
html_format: AMP
tag_name: "AMP-STORY-PAGE-ATTACHMENT"
spec_name: "amp-story-page-attachment"
tag_name: "AMP-STORY-PAGE-OUTLINK"
spec_name: "amp-story-page-outlink"
mandatory_ancestor: "AMP-STORY-PAGE"
descendant_tag_list: "amp-story-page-attachment-allowed-descendants"
mandatory_last_child: true
attrs: {
name: "cta-text"
name: "cta-accent-color"
}
attrs: {
name: "title"
name: "cta-accent-element"
value: "background"
value: "text"
}
attrs: {
name: "cta-image"
Expand All @@ -913,7 +952,11 @@ tags: { # <amp-story-page-attachment> with no href
}
}
attrs: {
name: "cta-image-2"
name: "cta-text"
}
attrs: {
name: "href"
mandatory: true
value_url: {
protocol: "http"
protocol: "https"
Expand All @@ -926,9 +969,16 @@ tags: { # <amp-story-page-attachment> with no href
}
attrs: {
name: "theme"
value: "custom"
value: "dark"
value: "light"
}
attrs: {
name: "title"
}
child_tags: {
mandatory_num_child_tags: 0
}
}
tags: { # <amp-story-animation> <script>
html_format: AMP
Expand Down