Skip to content

Commit

Permalink
✨ Display the publisher domain beneath the header of page attachments…
Browse files Browse the repository at this point in the history
… that contain a form element (#35581)

* Create the domain label element in the page attachment, which is displayed only when the attachment contains a form

* Format fixes and replacing innerHTML with textContent

* More formatting fixes

* Update regex (due to prettier error)

* Undo chnage made for testing

* Remove unnecessary whitespace and clarify some descriptions

* Replace regex with url service and update the domain label's side margins from 10px to 20px

* Update CSS to account for page attachment V1

* Refactor out label creation logic into new createDomainLabelElement_ method

* Prettier changes

* Add tests for building the domain label

* Revert the unit test changes

* Add visual diff tests for the page attachment domain header

* Resolve merge conflicts due to attachment v2 cleanup

* Fix import and undo change made for testing

* Add necessary import

* Test updates

* Undo test changes (they'll be submitted in the next PR)

* Remove unnecessary import and newline

* Added working visual diff tests

* Prettier: Remove whitespace

* Use CSS variables instead of setting the dark theme attribute on the domain label. Also, rename getPublisherDomain_ to getPublisherOrigin_

* Test code

* Couple testing fixes

* Replace after() with parentNode.append()
  • Loading branch information
coreymasanto committed Aug 12, 2021
1 parent b9e41a2 commit ab1dcae
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 5 deletions.
15 changes: 15 additions & 0 deletions examples/visual-tests/amp-story/amp-story-page-attachment.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
<head>
<meta charset="utf-8">
<script async src="https://cdn.ampproject.org/v0.js"></script>
<script async custom-element="amp-form"
src="https://cdn.ampproject.org/v0/amp-form-0.1.js"></script>
<script async custom-element="amp-story"
src="https://cdn.ampproject.org/v0/amp-story-1.0.js"></script>
<script async custom-element="amp-video"
Expand Down Expand Up @@ -188,6 +190,19 @@ <h1>Some really cool video</h1>
href="https://google.com">
</amp-story-page-attachment>
</amp-story-page>

<amp-story-page id="attachment-with-form">
<amp-story-page-attachment layout="nodisplay">
<form>
<fieldset>
<label>
<span>Name:</span>
<input type="text" name="name">
</label>
</fieldset>
</form>
</amp-story-page-attachment>
</amp-story-page>
</amp-story>
</body>
</html>
49 changes: 49 additions & 0 deletions examples/visual-tests/amp-story/amp-story-page-attachment.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
'use strict';

const {
verifySelectorsInvisible,
verifySelectorsVisible,
} = require('../../../build-system/tasks/visual-diff/helpers');

Expand Down Expand Up @@ -190,4 +191,52 @@ module.exports = {
'.i-amphtml-story-page-open-attachment[active]',
]);
},

'form presence - the domain label should display': async (page, name) => {
const url = await page.url();
const pageID = 'attachment-with-form';
await page.goto(`${url}#page=${pageID}`);
await page.waitForSelector(
`amp-story-page#${pageID}[active][distance="0"]`
);

// Open the page attachment.
await page.waitForSelector(
`amp-story-page#${pageID} .i-amphtml-story-inline-page-attachment-chip`
);
await page.tap(
`amp-story-page#${pageID} .i-amphtml-story-inline-page-attachment-chip`
);

// Ensure domain label presence.
await page.waitForSelector('.i-amphtml-story-draggable-drawer-open');
await verifySelectorsVisible(page, name, [
'.i-amphtml-story-draggable-drawer-open ' +
'.i-amphtml-story-page-attachment-domain-label',
]);
},

'form absence - the domain label should not display': async (page, name) => {
const url = await page.url();
const pageID = 'inline-default';
await page.goto(`${url}#page=${pageID}`);
await page.waitForSelector(
`amp-story-page#${pageID}[active][distance="0"]`
);

// Open the page attachment.
await page.waitForSelector(
`amp-story-page#${pageID} .i-amphtml-story-inline-page-attachment-chip`
);
await page.tap(
`amp-story-page#${pageID} .i-amphtml-story-inline-page-attachment-chip`
);

// Ensure domain label absence.
await page.waitForSelector('.i-amphtml-story-draggable-drawer-open');
await verifySelectorsInvisible(page, name, [
'.i-amphtml-story-draggable-drawer-open ' +
'.i-amphtml-story-page-attachment-domain-label',
]);
},
};
13 changes: 13 additions & 0 deletions extensions/amp-story/1.0/amp-story-draggable-drawer-header.css
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,19 @@
background-image: url('data:image/svg+xml;charset=utf-8,<svg xmlns="http://www.w3.org/2000/svg" fill="rgba(255, 255, 255, .4)" viewBox="0 0 14 14"><path d="M13.295 2.115a.997.997 0 10-1.41-1.41L7 5.59 2.115.705a.997.997 0 10-1.41 1.41L5.59 7 .705 11.885a.997.997 0 101.41 1.41L7 8.41l4.885 4.885a.997.997 0 101.41-1.41L8.41 7l4.885-4.885z"/></svg>') !important;
}

.i-amphtml-story-page-attachment-domain-label {
background: var(--i-amphtml-draggable-drawer-background-color) !important;
color: var(--i-amphtml-draggable-drawer-text-color) !important;
display: block !important;
font-family: 'Roboto', sans-serif !important;
font-size: 14px !important;
overflow: hidden !important;
padding: 10px 20px !important;
text-align: center !important;
text-overflow: ellipsis !important;
white-space: nowrap !important;
}

.i-amphtml-story-draggable-drawer-header .i-amphtml-story-draggable-drawer-header-title-and-close {
opacity: 1 !important;
height: 44px !important;
Expand Down
67 changes: 62 additions & 5 deletions extensions/amp-story/1.0/amp-story-page-attachment.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {closest} from '#core/dom/query';
import {dev, devAssert} from '../../../src/log';
import {getHistoryState} from '#core/window/history';
import {getLocalizationService} from './amp-story-localization-service';
import {getSourceOrigin} from '../../../src/url';
import {htmlFor, htmlRefs} from '#core/dom/static-template';
import {removeElement} from '#core/dom';
import {setImportantStyles, toggle} from '#core/dom/style';
Expand Down Expand Up @@ -75,6 +76,12 @@ export class AmpStoryPageAttachment extends DraggableDrawer {
constructor(element) {
super(element);

/**
* The label containing the publisher domain.
* @protected {?Element}
*/
this.domainLabelEl = null;

/** @private @const {!./story-analytics.StoryAnalyticsService} */
this.analyticsService_ = getAnalyticsService(this.win, this.element);

Expand All @@ -91,11 +98,8 @@ export class AmpStoryPageAttachment extends DraggableDrawer {
buildCallback() {
super.buildCallback();

const theme = this.element.getAttribute('theme')?.toLowerCase();
if (theme && AttachmentTheme.DARK === theme) {
this.headerEl.setAttribute('theme', theme);
this.element.setAttribute('theme', theme);
}
this.maybeSetDarkThemeForElement_(this.headerEl);
this.maybeSetDarkThemeForElement_(this.element);

// Outlinks can be an amp-story-page-outlink or the legacy version,
// an amp-story-page-attachment with an href.
Expand Down Expand Up @@ -137,6 +141,13 @@ export class AmpStoryPageAttachment extends DraggableDrawer {
* @private
*/
buildInline_() {
if (this.doesContainFormElement_()) {
// Page attachments that contain forms must display the page's publisher
// domain above the attachment's contents. This enables users to gauge
// the trustworthiness of publishers before sending data to them.
this.headerEl.parentNode.append(this.createDomainLabelElement_());
}

const closeButtonEl = htmlFor(this.element)`
<button class="i-amphtml-story-page-attachment-close-button" aria-label="close"
role="button">
Expand Down Expand Up @@ -445,4 +456,50 @@ export class AmpStoryPageAttachment extends DraggableDrawer {
);
});
}

/**
* Updates the given element with the appropriate class or attribute, if the
* page attachment's theme is 'dark'.
* @param {!Element} element The element upon which to set the dark theme.
* @private
*/
maybeSetDarkThemeForElement_(element) {
const theme = this.element.getAttribute('theme')?.toLowerCase();
if (theme && AttachmentTheme.DARK === theme) {
element.setAttribute('theme', theme);
}
}

/**
* Create the domain label element to be displayed at the top of the page
* attachment.
* @return {!Element} element The domain label element.
* @private
*/
createDomainLabelElement_() {
const domainLabelEl = this.win.document.createElement('div');
domainLabelEl.classList.add('i-amphtml-story-page-attachment-domain-label');
domainLabelEl.textContent = this.getPublisherOrigin_();
return domainLabelEl;
}

/**
* Returns whether a form element exists within this page attachment.
* @return {boolean} True, only if a form element exists as a descendant of
* this page attachment.
* @private
*/
doesContainFormElement_() {
return Boolean(this.element.querySelector('form'));
}

/**
* Returns the publisher origin URL string (e.g., "stories.example.com").
* @return {string} The domain of the publisher.
* @private
*/
getPublisherOrigin_() {
const publisherOrigin = getSourceOrigin(this.getAmpDoc().getUrl());
return publisherOrigin.replace(/https?:\/\//, '');
}
}

0 comments on commit ab1dcae

Please sign in to comment.