Skip to content

Commit

Permalink
♻️ Simplify rendering <amp-ima-video> with CSS and static templates (a…
Browse files Browse the repository at this point in the history
…mpproject#34248)

Use static templates and standalone CSS to simplify how we render a modify `<amp-ima-video>`'s internal tree.

This change is unrelated to the Bento effort. Communication between iframe and host is intact.
  • Loading branch information
alanorozco authored and rochapablo committed Aug 30, 2021
1 parent 0fe7b35 commit 7573633
Show file tree
Hide file tree
Showing 8 changed files with 622 additions and 620 deletions.
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] {
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. */

/* 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) {
.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

0 comments on commit 7573633

Please sign in to comment.