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.

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

♻️ Simplify rendering <amp-ima-video> with CSS and static templates #34248

Merged
merged 14 commits into from
May 13, 2021
778 changes: 310 additions & 468 deletions ads/google/ima/ima-video.js

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions build-system/tasks/css/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ const cssEntryPoints = [
outCss: 'amp-story-player-iframe-v0.css',
append: false,
},
{
path: 'amp-ima-video-iframe.css',
outJs: 'amp-ima-video-iframe.css.js',
outCss: 'amp-ima-video-iframe-v0.css',
append: false,
},
];

/**
Expand Down
1 change: 1 addition & 0 deletions build-system/test-configs/dep-check-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ exports.rules = [
'ads/**->src/mode.js',
'ads/**->src/url.js',
'ads/**->src/core/types/array.js',
'ads/**->src/static-template.js',
'ads/**->src/style.js',
'ads/**->src/core/constants/consent-state.js',
'ads/**->src/internal-version.js',
Expand Down
1 change: 1 addition & 0 deletions css/Z_INDEX.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
`.i-amphtml-story-hint-container` | 2 | [extensions/amp-story/1.0/amp-story-hint.css](/extensions/amp-story/1.0/amp-story-hint.css)
`.i-amphtml-story-bookend-active amp-story-page[active]::after` | 2 | [extensions/amp-story/1.0/amp-story.css](/extensions/amp-story/1.0/amp-story.css)
`amp-story-grid-layer` | 2 | [extensions/amp-story/1.0/amp-story.css](/extensions/amp-story/1.0/amp-story.css)
`.controls` | 1 | [css/amp-ima-video-iframe.css](/css/amp-ima-video-iframe.css)
`.amp-story-player-exit-control-button` | 1 | [css/amp-story-player-iframe.css](/css/amp-story-player-iframe.css)
`.i-amphtml-layout-size-defined > [fallback]` | 1 | [css/ampshared.css](/css/ampshared.css)
`.i-amphtml-layout-size-defined > [placeholder]` | 1 | [css/ampshared.css](/css/ampshared.css)
Expand Down
211 changes: 211 additions & 0 deletions css/amp-ima-video-iframe.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,211 @@
/**
* Copyright 2021 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/*
* Styles inserted in iframes rendered by <amp-ima-video>.
*
* .selectorsInThisFile reject naming convention by using camelCase rather than
* dash-case, because they're uniform with Javascript references that are more
* useful when destructured as such (imaVideo.js).
*
* We insert this CSS in a standalone document whose namespace is shared only
* with the IMA SDK. This unconventional naming is not worth converting to
* dash-case during build or runtime, so we keep it.
*/

[hidden] {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now toggle elements with the [hidden] attribute. This helps us from knowing how to restore from display: none, which could be a number of values like block or flex.

display: none !important;
}

body,
.video {
background: black;
}

.video {
width: 100%;
height: 100%;
}

.fill {
position: absolute;
top: 0;
left: 0;
width: 100%;
height: 100%;
}

button {
cursor: pointer;
appearance: none;
padding: 0;
border: none;
background: transparent;
display: block;
}

.controls {
position: absolute;
bottom: 0;
width: 100%;
background-color: rgba(7, 20, 30, 0.7);
background: linear-gradient(
0,
rgba(7, 20, 30, 0.7) 0%,
rgba(7, 20, 30, 0) 100%
);
box-sizing: border-box;
padding: 10px;
padding-top: 60px;
color: white;
display: flex;
font-family: Helvetica, Arial, Sans-serif;
justify-content: center;
align-items: center;
user-select: none;
z-index: 1;
}

.controls > *:not(:last-child) {
margin-right: 20px;
}

button > svg {
width: 100%;
height: 100%;
filter: drop-shadow(0 0 14px rgba(0, 0, 0, 0.4));
fill: #ffffff;
}

.countdownWrapper {
align-items: center;
box-sizing: border-box;
display: none;
flex-grow: 1;
font-size: 12px;
height: 20px;
overflow: hidden;
padding: 5px;
text-shadow: 0 0 10px black;
white-space: nowrap;
}

.time {
margin-right: 20px;
text-align: center;
font-size: 14px;
text-shadow: 0 0 10px black;
}

.progress {
height: 30px;
flex-grow: 1;
position: relative;
margin-right: 20px;
}

.progress::after {
display: block;
content: '';
background-color: rgba(255, 255, 255, 0.45);
height: 2px;
width: 100%;
margin-top: 14px;
}

.progressLine {
background-color: rgb(255, 255, 255);
height: 2px;
margin-top: 14px;
width: 0%;
float: left;
}

.progressMarker {
height: 14px;
width: 14px;
position: absolute;
left: 0%;
top: 50%;
margin-top: -7px;
cursor: pointer;
border-radius: 14px;
background: white;
}

.controls > button {
width: 30px;
height: 30px;
flex-shrink: 0;
}

.overlayButton {
display: flex;
justify-content: center;
align-items: center;
}

.overlayButton > svg {
max-width: 120px;
max-height: 120px;
}

/* Swap button's icons based on root state. */
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of selecting elements and imperatively changeIcon(), we swap the icons with root state attribute selectors [data-playing], [data-muted]. We also toggle controls state while an ad is playing with [data-ad] and [data-skippable].


/* play/pause */
.root:not([data-playing]) .playButton > svg:last-child,
.root[data-playing] .playButton > svg:first-child,
/* mute/unmute */
.root:not([data-muted]) .muteButton > svg:last-child,
.root[data-muted] .muteButton > svg:first-child {
display: none;
}

/* Ad controls */

.root[data-ad] > .controls {
height: 30px;
justify-content: flex-end;
padding: 10px;
}

.root[data-ad] > .controls > button {
height: 22px;
}

.root[data-ad] > .controls .muteButton {
margin-right: 10px;
}

@media screen and (max-width: 400) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Mini" controls are shown when the video is under a certain size an ad is skippable. The latter condition remains in the Javascript logic, while size is just a media query.

.root[data-skippable] > .controls {
height: 20px;
}
.root[data-skippable] > .controls > button {
height: 18px;
}
}

/* Show ad controls */
.root[data-ad] > .controls > .countdownWrapper {
display: flex;
}

/* Hide non-ad controls */
.root[data-ad] > .controls > .time,
.root[data-ad] > .controls > .progress {
display: none;
}
18 changes: 11 additions & 7 deletions extensions/amp-ima-video/0.1/amp-ima-video.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,14 +206,18 @@ class AmpImaVideo extends AMP.BaseElement {
const consentPromise = consentPolicyId
? getConsentPolicyState(element, consentPolicyId)
: Promise.resolve(null);
element.setAttribute(
'data-source-children',
JSON.stringify(this.sourceChildren_)
);
return consentPromise.then((initialConsentState) => {
const iframeContext = {
initialConsentState,
sourceChildren: this.sourceChildren_,
};
const iframe = getIframe(win, element, TYPE, iframeContext, {
allowFullscreen: true,
});
const iframe = getIframe(
win,
element,
TYPE,
{initialConsentState},
{allowFullscreen: true}
);
iframe.title = this.element.title || 'IMA video';

this.applyFillContent(iframe);
Expand Down
5 changes: 3 additions & 2 deletions extensions/amp-ima-video/0.1/test/test-amp-ima-video.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,9 @@ describes.realWin(
);

const parsedName = JSON.parse(iframe.name);
const sourceChildren = parsedName?.attributes?._context?.sourceChildren;
expect(sourceChildren).to.not.be.null;
const sourceChildrenSerialized = parsedName?.attributes?.sourceChildren;
expect(sourceChildrenSerialized).to.not.be.null;
const sourceChildren = JSON.parse(sourceChildrenSerialized);
expect(sourceChildren).to.have.length(2);
expect(sourceChildren[0][0]).to.eql('SOURCE');
expect(sourceChildren[0][1]).to.eql({'data-foo': 'bar', src: 'src'});
Expand Down
Loading