From 26488aa5daa6ab66a9331b4a58a2bdff723ca9f6 Mon Sep 17 00:00:00 2001 From: Alan Orozco Date: Thu, 18 Jan 2018 14:24:41 -0800 Subject: [PATCH 01/13] Implement amp-story dynamic page scaling --- .../amp-story/0.1/amp-story-desktop.css | 13 +- extensions/amp-story/0.1/amp-story-page.js | 14 +- extensions/amp-story/0.1/amp-story.css | 14 +- extensions/amp-story/0.1/amp-story.js | 5 + extensions/amp-story/0.1/page-scaling.js | 252 ++++++++++++++++++ 5 files changed, 290 insertions(+), 8 deletions(-) create mode 100644 extensions/amp-story/0.1/page-scaling.js diff --git a/extensions/amp-story/0.1/amp-story-desktop.css b/extensions/amp-story/0.1/amp-story-desktop.css index 59e6fecf0159..920730d7a195 100644 --- a/extensions/amp-story/0.1/amp-story-desktop.css +++ b/extensions/amp-story/0.1/amp-story-desktop.css @@ -188,16 +188,21 @@ amp-story[standalone][desktop] { } [desktop] > amp-story-page, -.i-amphtml-story-page-sentinel { - left: 50%!important; - right: auto !important; - margin: auto !important; +[desktop] > .i-amphtml-story-page-sizer, +[desktop] .i-amphtml-story-page-sentinel { max-height: 75vh !important; max-width: calc(3/5 * 75vh) !important; min-width: 320px !important; min-height: 533px !important; } +[desktop] > amp-story-page, +[desktop] .i-amphtml-story-page-sentinel { + left: 50%!important; + right: auto !important; + margin: auto !important; +} + [desktop] > amp-story-page { box-shadow: 0 0 15px rgba(0, 0, 0, .4)!important; } diff --git a/extensions/amp-story/0.1/amp-story-page.js b/extensions/amp-story/0.1/amp-story-page.js index 739200fd70d3..92869ad93ef4 100644 --- a/extensions/amp-story/0.1/amp-story-page.js +++ b/extensions/amp-story/0.1/amp-story-page.js @@ -32,9 +32,11 @@ import {upgradeBackgroundAudio} from './audio'; import {EventType, dispatch, dispatchCustom} from './events'; import {AdvancementConfig} from './page-advancement'; import {matches, scopedQuerySelectorAll} from '../../../src/dom'; +import {dev} from '../../../src/log'; import {getLogEntries} from './logging'; import {getMode} from '../../../src/mode'; import {CommonSignals} from '../../../src/common-signals'; +import {PageScalingService} from './page-scaling'; /** @@ -179,7 +181,7 @@ export class AmpStoryPage extends AMP.BaseElement { /** @return {!Promise} */ beforeVisible() { this.rewindAllMediaToBeginning_(); - return this.maybeApplyFirstAnimationFrame(); + return this.scale_().then(() => this.maybeApplyFirstAnimationFrame()); } @@ -327,6 +329,14 @@ export class AmpStoryPage extends AMP.BaseElement { return this.animationManager_.applyFirstFrame(); } + /** + * @return {!Promise} + * @private + */ + scale_() { + const storyEl = dev().assertElement(this.element.parentNode); + return PageScalingService.for(this.win, storyEl).scale(this.element); + } /** * @param {boolean} isActive @@ -356,10 +366,10 @@ export class AmpStoryPage extends AMP.BaseElement { */ setDistance(distance) { this.element.setAttribute('distance', distance); - this.registerAllMedia_(); if (distance > 0 && distance <= 2) { this.preloadAllMedia_(); + this.scale_(); } } diff --git a/extensions/amp-story/0.1/amp-story.css b/extensions/amp-story/0.1/amp-story.css index 8010fa86202f..75b408bb2753 100644 --- a/extensions/amp-story/0.1/amp-story.css +++ b/extensions/amp-story/0.1/amp-story.css @@ -180,14 +180,24 @@ amp-story .amp-video-eq { } /** Page level */ -amp-story-page { +amp-story-page, +.i-amphtml-story-page-sizer { bottom: 0 !important; - display: none !important; height: auto !important; left: 0 !important; position: absolute !important; right: 0 !important; top: 0 !important; +} + +.i-amphtml-story-page-sizer { + opacity: 0 !important; + pointer-events: none !important; + contain: strict !important; +} + +amp-story-page { + display: none !important; transition: none !important; } diff --git a/extensions/amp-story/0.1/amp-story.js b/extensions/amp-story/0.1/amp-story.js index ec25fb87120b..9a6febc055a6 100644 --- a/extensions/amp-story/0.1/amp-story.js +++ b/extensions/amp-story/0.1/amp-story.js @@ -233,6 +233,10 @@ export class AmpStory extends AMP.BaseElement { buildCallback() { this.assertAmpStoryExperiment_(); + if (this.isDesktop_()) { + this.element.setAttribute('desktop',''); + } + if (this.element.hasAttribute(AMP_STORY_STANDALONE_ATTRIBUTE)) { this.getAmpDoc().win.document.documentElement.classList .add('i-amphtml-story-standalone'); @@ -921,6 +925,7 @@ export class AmpStory extends AMP.BaseElement { this.bookend_.hide(); } + /** * Toggle content when bookend is opened/closed. * @param {boolean} display diff --git a/extensions/amp-story/0.1/page-scaling.js b/extensions/amp-story/0.1/page-scaling.js new file mode 100644 index 000000000000..a586c0fa745e --- /dev/null +++ b/extensions/amp-story/0.1/page-scaling.js @@ -0,0 +1,252 @@ +/** + * Copyright 2018 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. + */ +import {Services} from '../../../src/services'; +import {dev, user} from '../../../src/log'; +import {dict} from '../../../src/utils/object'; +import {isExperimentOn} from '../../../src/experiments'; +import {childElementsByTag, matches} from '../../../src/dom'; +import {px, setImportantStyles} from '../../../src/style'; +import {renderAsElement} from './simple-template'; +import {throttle} from '../../../src/utils/rate-limit'; +import {toArray} from '../../../src/types'; + + +/** @private @const {number} */ +const MIN_LAYER_WIDTH_PX = 380; + + +/** @private @const {number} */ +const MAX_LAYER_WIDTH_PX = 520; + + +/** @private @const {string} */ +const SCALING_APPLIED_CLASSNAME = 'i-amphtml-story-scaled'; + + +/** @private @const {!./simple-template.ElementDef} */ +const SIZER_TEMPLATE = { + tag: 'div', + attrs: dict({'class': 'i-amphtml-story-page-sizer'}), +}; + + +/** @struct @typedef {{factor: number, width: number, height: number}} */ +let DimensionsDef; + + +/** @typedef {{scale: function(!Element):!Promise}} */ +let PageScalingServiceInterface; + + +/** + * @param {!Element} sizer + * @return {!DimensionsDef} + */ +function targetDimensionsFor(sizer) { + const {width, height} = sizer./*OK*/getBoundingClientRect(); + + const ratio = width / height; + + const targetWidth = Math.min(MAX_LAYER_WIDTH_PX, + Math.max(width, Math.max(1, ratio) * MIN_LAYER_WIDTH_PX)); + + const targetHeight = (targetWidth / ratio); + + const factor = width / targetWidth; + + return {width: targetWidth, height: targetHeight, factor}; +} + + +/** + * @param {!Element} page + * @param {!DimensionsDef} dimensions + */ +function applyScaling(page, dimensions) { + const {width, height, factor} = dimensions; + // TODO(alanorozco): Calculate relative layer dimensions for cases where + // they won't fill the entire page. + const style = { + 'width': px(width), + 'height': px(height), + 'zoom': `${factor * 100}%`, + 'box-sizing': 'border-box', + }; + scalableChildren(page).forEach(el => { + setImportantStyles(el, style); + }); + markScalingApplied(page); +} + + +/** + * @param {!Element} page + * @return {!Array} + */ +function scalableChildren(page) { + return toArray(childElementsByTag(page, 'amp-story-grid-layer')); +} + + +/** + * @param {!Element} page + * @return {boolean} + */ +function isScalingEnabled(page) { + return page.getAttribute('scaling') != 'disabled'; +} + + +/** + * @param {!Element} page + * @return {boolean} + */ +function isScalingApplied(page) { + return page.classList.contains(SCALING_APPLIED_CLASSNAME); +} + + +/** + * @param {!Element} page + * @param {boolean} isApplied + */ +function markScalingApplied(page, isApplied = true) { + page.classList.toggle(SCALING_APPLIED_CLASSNAME, isApplied); +} + + +/** + * Required for lazy evaluation after resize. + * @param {!Element} page + * @return {boolean} + */ +function withinRange(page) { + return matches(page, '[active], [distance="1"], [desktop] > [distance="2"]'); +} + + +/** + * @param {!Element} page + * @return {boolean} + */ +function needsScaling(page) { + return isScalingEnabled(page) && !isScalingApplied(page) && withinRange(page); +} + + +/** + * @param {!Document} doc + * @param {!Element} container + */ +function createSizer(doc, container) { + const element = renderAsElement(doc, SIZER_TEMPLATE); + container.appendChild(element); + return element; +} + + +/** @private @const {!PageScalingServiceInterface} */ +const MOCK_PAGE_SCALING_SERVICE = { + scale(unusedPage) { + return Promise.resolve(); + }, +}; + + +/** @private {?PageScalingServiceInterface} */ +let pageScalingService = null; + + +/** + * Service for scaling pages dynamically so their layers will be sized within a + * certain pixel range independent of visual dimensions. + */ +// TODO(alanorozco): Make this part of the runtime layout system to prevent +// FOUC-like jump and allow for SSR. +export class PageScalingService { + constructor(win, story) { + /** @private @const {!Element} */ + this.story_ = story; + + /** @private @const */ + this.vsync_ = Services.vsyncFor(win); + + /** @private @const {!Element} */ + this.sizer_ = createSizer(win.document, story); + + /** @private {?DimensionsDef} */ + this.dimensions_ = null; + + Services.viewportForDoc(story).onResize( + throttle(win, () => this.onViewportResize_(), 100)); + } + + /** + * @param {!Window} win + * @param {!Element} story + * @return {!PageScalingServiceInterface} + */ + static for(win, story) { + // Implemented as singleton for now, should be mapped to story element. + // TODO(alanorozco): Implement mapping to support multiple + // instances in one doc. + if (!pageScalingService) { + if (!isExperimentOn(win, 'amp-story-scaling')) { + pageScalingService = MOCK_PAGE_SCALING_SERVICE; + } else if (Services.platformFor(win).isFirefox()) { + // Firefox does not support `zoom`. + // TODO(alanorozco): Use `scale` on Firefox. + user().warn('`amp-story-scaling` ignored: Firefox is not supported.'); + pageScalingService = MOCK_PAGE_SCALING_SERVICE; + } else { + pageScalingService = new PageScalingService(win, story); + } + } + return pageScalingService; + } + + /** + * @param {!Element} page + * @return {!Promise} + */ + scale(page) { + if (!needsScaling(page)) { + return Promise.resolve(); + } + return this.vsync_.runPromise({ + measure: () => { + if (!this.dimensions_) { + this.dimensions_ = targetDimensionsFor(this.sizer_); + } + }, + mutate: () => { + const dimensions = + /** @type {!DimensionsDef} */ (dev().assert(this.dimensions_)); + applyScaling(page, dimensions); + }, + }); + } + + /** @private */ + onViewportResize_() { + const pages = toArray(childElementsByTag(this.story_, 'amp-story-page')); + this.dimensions_ = null; + pages.forEach(page => { + markScalingApplied(page, false); + this.scale(page); + }); + } +} From 600355050b44881ba658302dddb7626c841f1940 Mon Sep 17 00:00:00 2001 From: Alan Orozco Date: Thu, 18 Jan 2018 14:50:04 -0800 Subject: [PATCH 02/13] Register experiment --- extensions/amp-story/0.1/page-scaling.js | 3 ++- tools/experiments/experiments.js | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/extensions/amp-story/0.1/page-scaling.js b/extensions/amp-story/0.1/page-scaling.js index a586c0fa745e..93e0c7532972 100644 --- a/extensions/amp-story/0.1/page-scaling.js +++ b/extensions/amp-story/0.1/page-scaling.js @@ -209,7 +209,8 @@ export class PageScalingService { } else if (Services.platformFor(win).isFirefox()) { // Firefox does not support `zoom`. // TODO(alanorozco): Use `scale` on Firefox. - user().warn('`amp-story-scaling` ignored: Firefox is not supported.'); + user().warn('AMP-STORY', + '`amp-story-scaling` ignored: Firefox is not supported.'); pageScalingService = MOCK_PAGE_SCALING_SERVICE; } else { pageScalingService = new PageScalingService(win, story); diff --git a/tools/experiments/experiments.js b/tools/experiments/experiments.js index ffe5abb2a03d..d5afc7fe5188 100644 --- a/tools/experiments/experiments.js +++ b/tools/experiments/experiments.js @@ -269,6 +269,12 @@ const EXPERIMENTS = [ spec: 'https://github.com/ampproject/amphtml/issues/11329', cleanupIssue: 'https://github.com/ampproject/amphtml/issues/11475', }, + { + id: 'amp-story-scaling', + name: 'Enables dynamic page scaling in amp-story', + spec: 'https://github.com/ampproject/amphtml/issues/12902', + cleanupIssue: 'https://github.com/ampproject/amphtml/issues/12902', + }, { id: 'disable-amp-story-desktop', name: 'Disables responsive desktop experience for the amp-story component', From 8c055875733f3d636bb023fbde70d2d95df2c82c Mon Sep 17 00:00:00 2001 From: Alan Orozco Date: Fri, 26 Jan 2018 08:54:45 -0800 Subject: [PATCH 03/13] Fallback transform implementation + custom css props --- extensions/amp-story/0.1/amp-story-page.js | 2 +- extensions/amp-story/0.1/amp-story.css | 14 +- extensions/amp-story/0.1/amp-story.js | 9 +- extensions/amp-story/0.1/page-scaling.js | 370 ++++++++++++++++----- 4 files changed, 297 insertions(+), 98 deletions(-) diff --git a/extensions/amp-story/0.1/amp-story-page.js b/extensions/amp-story/0.1/amp-story-page.js index 5f345ccfc9d7..8d2d59ad6394 100644 --- a/extensions/amp-story/0.1/amp-story-page.js +++ b/extensions/amp-story/0.1/amp-story-page.js @@ -375,7 +375,7 @@ export class AmpStoryPage extends AMP.BaseElement { */ scale_() { const storyEl = dev().assertElement(this.element.parentNode); - return PageScalingService.for(this.win, storyEl).scale(this.element); + return PageScalingService.for(storyEl).scale(this.element); } /** diff --git a/extensions/amp-story/0.1/amp-story.css b/extensions/amp-story/0.1/amp-story.css index 15591aec9820..6234c450bcdd 100644 --- a/extensions/amp-story/0.1/amp-story.css +++ b/extensions/amp-story/0.1/amp-story.css @@ -185,24 +185,14 @@ amp-story .amp-video-eq { } /** Page level */ -amp-story-page, -.i-amphtml-story-page-sizer { +amp-story-page { bottom: 0 !important; + display: none !important; height: auto !important; left: 0 !important; position: absolute !important; right: 0 !important; top: 0 !important; -} - -.i-amphtml-story-page-sizer { - opacity: 0 !important; - pointer-events: none !important; - contain: strict !important; -} - -amp-story-page { - display: none !important; transition: none !important; } diff --git a/extensions/amp-story/0.1/amp-story.js b/extensions/amp-story/0.1/amp-story.js index 57305d6b257a..abb5d5055a1d 100644 --- a/extensions/amp-story/0.1/amp-story.js +++ b/extensions/amp-story/0.1/amp-story.js @@ -165,8 +165,11 @@ const SHARE_WIDGET_PILL_CONTAINER = { * desktop view. * @private @const {string} */ -const HIDE_ON_BOOKEND_SELECTOR = - 'amp-story-page, .i-amphtml-story-system-layer'; +const HIDE_ON_BOOKEND_SELECTOR = [ + 'amp-story-page[active]', + 'amp-story-page[distance="1"]', + '.i-amphtml-story-system-layer', +].join(); export class AmpStory extends AMP.BaseElement { @@ -254,6 +257,8 @@ export class AmpStory extends AMP.BaseElement { this.element.setAttribute('desktop',''); } + this.element.querySelector('amp-story-page').setAttribute('active', ''); + if (this.element.hasAttribute(AMP_STORY_STANDALONE_ATTRIBUTE)) { this.getAmpDoc().win.document.documentElement.classList .add('i-amphtml-story-standalone'); diff --git a/extensions/amp-story/0.1/page-scaling.js b/extensions/amp-story/0.1/page-scaling.js index 93e0c7532972..27afcd074615 100644 --- a/extensions/amp-story/0.1/page-scaling.js +++ b/extensions/amp-story/0.1/page-scaling.js @@ -14,14 +14,17 @@ * limitations under the License. */ import {Services} from '../../../src/services'; +import { + childElementsByTag, + matches, + scopedQuerySelector, +} from '../../../src/dom'; import {dev, user} from '../../../src/log'; -import {dict} from '../../../src/utils/object'; import {isExperimentOn} from '../../../src/experiments'; -import {childElementsByTag, matches} from '../../../src/dom'; import {px, setImportantStyles} from '../../../src/style'; -import {renderAsElement} from './simple-template'; import {throttle} from '../../../src/utils/rate-limit'; -import {toArray} from '../../../src/types'; +import {toArray, toWin} from '../../../src/types'; +import {unscaledClientRect} from './utils'; /** @private @const {number} */ @@ -36,27 +39,27 @@ const MAX_LAYER_WIDTH_PX = 520; const SCALING_APPLIED_CLASSNAME = 'i-amphtml-story-scaled'; -/** @private @const {!./simple-template.ElementDef} */ -const SIZER_TEMPLATE = { - tag: 'div', - attrs: dict({'class': 'i-amphtml-story-page-sizer'}), -}; - - /** @struct @typedef {{factor: number, width: number, height: number}} */ -let DimensionsDef; +let TargetDimensionsDef; -/** @typedef {{scale: function(!Element):!Promise}} */ -let PageScalingServiceInterface; +/** + * @struct + * @typedef {{ + * relativeWidth: number, + * relativeHeight: number, + * matrix: ?Array, + * }} + */ +let ScalableDimensionsDef; /** * @param {!Element} sizer - * @return {!DimensionsDef} + * @return {!TargetDimensionsDef} */ function targetDimensionsFor(sizer) { - const {width, height} = sizer./*OK*/getBoundingClientRect(); + const {width, height} = unscaledClientRect(sizer); const ratio = width / height; @@ -67,28 +70,30 @@ function targetDimensionsFor(sizer) { const factor = width / targetWidth; - return {width: targetWidth, height: targetHeight, factor}; + return {factor, width: targetWidth, height: targetHeight}; } /** - * @param {!Element} page - * @param {!DimensionsDef} dimensions + * @param {number} factor + * @param {number} width + * @param {number} height + * @param {!Array} matrix + * @return {!Object} */ -function applyScaling(page, dimensions) { - const {width, height, factor} = dimensions; - // TODO(alanorozco): Calculate relative layer dimensions for cases where - // they won't fill the entire page. - const style = { - 'width': px(width), - 'height': px(height), - 'zoom': `${factor * 100}%`, - 'box-sizing': 'border-box', - }; - scalableChildren(page).forEach(el => { - setImportantStyles(el, style); - }); - markScalingApplied(page); +function scaleTransform(factor, width, height, matrix) { + // TODO(alanorozco, #12934): Translate values are not correctly calculated if + // `scale`, `skew` or `rotate` have been user-defined. + const translateX = width * factor / 2 - width / 2; + const translateY = height * factor / 2 - height / 2; + return [ + matrix[0] * factor, + matrix[1], + matrix[2], + matrix[3] * factor, + matrix[4] + translateX, + matrix[5] + translateY, + ]; } @@ -96,7 +101,7 @@ function applyScaling(page, dimensions) { * @param {!Element} page * @return {!Array} */ -function scalableChildren(page) { +function scalableElements(page) { return toArray(childElementsByTag(page, 'amp-story-grid-layer')); } @@ -106,7 +111,14 @@ function scalableChildren(page) { * @return {boolean} */ function isScalingEnabled(page) { - return page.getAttribute('scaling') != 'disabled'; + // TODO(alanorozco, #12902): Clean up experiment flag. + // NOTE(alanorozco): Experiment flag is temporary. No need to clutter the + // signatures in this function path by adding `win` as a parameter. + const win = toWin(page.ownerDocument.defaultView); + if (isExperimentOn(win, 'amp-story-scaling')) { + return true; + } + return page.getAttribute('scaling') == 'outside-range'; } @@ -139,34 +151,25 @@ function withinRange(page) { /** - * @param {!Element} page + * @param {!Window} win * @return {boolean} */ -function needsScaling(page) { - return isScalingEnabled(page) && !isScalingApplied(page) && withinRange(page); +function isCssZoomSupported(win) { + // IE supports `zoom`, but not `CSS.supports`. + return Services.platformFor(win).isIe() || win.CSS.supports('zoom', '1'); } /** - * @param {!Document} doc - * @param {!Element} container + * @param {!Window} win + * @return {boolean} */ -function createSizer(doc, container) { - const element = renderAsElement(doc, SIZER_TEMPLATE); - container.appendChild(element); - return element; +function isCssCustomPropsSupported(win) { + return !Services.platformFor(win).isIe() && win.CSS.supports('(--foo: red)'); } -/** @private @const {!PageScalingServiceInterface} */ -const MOCK_PAGE_SCALING_SERVICE = { - scale(unusedPage) { - return Promise.resolve(); - }, -}; - - -/** @private {?PageScalingServiceInterface} */ +/** @private {?PageScalingService} */ let pageScalingService = null; @@ -177,43 +180,58 @@ let pageScalingService = null; // TODO(alanorozco): Make this part of the runtime layout system to prevent // FOUC-like jump and allow for SSR. export class PageScalingService { - constructor(win, story) { + /** + * @param {!Window} win + * @param {!Element} rootEl + */ + constructor(win, rootEl) { + /** @private @const {!Window} */ + this.win_ = win; + /** @private @const {!Element} */ - this.story_ = story; + this.rootEl_ = rootEl; /** @private @const */ this.vsync_ = Services.vsyncFor(win); /** @private @const {!Element} */ - this.sizer_ = createSizer(win.document, story); + // Assumes active page to be determinant of the target size. + this.sizer_ = dev().assertElement( + scopedQuerySelector(rootEl, 'amp-story-page[active]'), + 'No active page found when initializing scaler.'); + + /** @private {?TargetDimensionsDef} */ + this.targetDimensions_ = null; - /** @private {?DimensionsDef} */ - this.dimensions_ = null; + /** @private {!Object>} */ + this.scalableElsDimensions_ = {}; - Services.viewportForDoc(story).onResize( + Services.viewportForDoc(rootEl).onResize( throttle(win, () => this.onViewportResize_(), 100)); } /** - * @param {!Window} win * @param {!Element} story - * @return {!PageScalingServiceInterface} + * @return {!PageScalingService} */ - static for(win, story) { + static for(story) { // Implemented as singleton for now, should be mapped to story element. // TODO(alanorozco): Implement mapping to support multiple // instances in one doc. + const win = toWin(story.ownerDocument.defaultView); if (!pageScalingService) { - if (!isExperimentOn(win, 'amp-story-scaling')) { - pageScalingService = MOCK_PAGE_SCALING_SERVICE; - } else if (Services.platformFor(win).isFirefox()) { - // Firefox does not support `zoom`. - // TODO(alanorozco): Use `scale` on Firefox. + if (!isCssZoomSupported(win)) { + // TODO(alanorozco, #12934): Combine transform matrix. user().warn('AMP-STORY', - '`amp-story-scaling` ignored: Firefox is not supported.'); - pageScalingService = MOCK_PAGE_SCALING_SERVICE; + '`amp-story-scaling` using CSS transforms as fallback.', + 'Any `amp-story-grid-layer` with user-defined CSS transforms will', + 'break.', + 'See https://github.com/ampproject/amphtml/issues/12934'); + pageScalingService = new TransformScalingService(win, story); + } else if (isCssCustomPropsSupported(win)) { + pageScalingService = new CssPropsZoomScalingService(win, story); } else { - pageScalingService = new PageScalingService(win, story); + pageScalingService = new ZoomScalingService(win, story); } } return pageScalingService; @@ -224,30 +242,216 @@ export class PageScalingService { * @return {!Promise} */ scale(page) { - if (!needsScaling(page)) { - return Promise.resolve(); + return Promise.resolve(this.scale_(page)); + } + + /** + * @param {!Element} page + * @return {!Promise|undefined} + * @private + */ + scale_(page) { + if (isScalingApplied(page)) { + return; + } + if (!isScalingEnabled(page)) { + return; + } + if (!withinRange(page)) { + return; } return this.vsync_.runPromise({ - measure: () => { - if (!this.dimensions_) { - this.dimensions_ = targetDimensionsFor(this.sizer_); - } + measure: state => { + state.targetDimensions = this.measureTargetDimensions_(); + state.scalableElsDimensions = this.measureScalableElements(page); }, - mutate: () => { - const dimensions = - /** @type {!DimensionsDef} */ (dev().assert(this.dimensions_)); - applyScaling(page, dimensions); + mutate: state => { + const {targetDimensions, scalableElsDimensions} = state; + scalableElements(page).forEach((el, i) => { + const elementDimensions = scalableElsDimensions[i]; + const style = this.scalingStyles(targetDimensions, elementDimensions); + + // Required since layer now has a width/height set. + Object.assign(style, {'box-sizing': 'border-box'}); + + setImportantStyles(el, style); + }); + markScalingApplied(page); }, + }, /* state */ {}); + } + + /** + * @param {!TargetDimensionsDef} unusedTargetDimensions + * @param {!ScalableDimensionsDef} unusedScalableElDimensions + * @return {!Object} + * @protected + */ + scalingStyles(unusedTargetDimensions, unusedScalableElDimensions) { + dev().assert(false, 'Empty PageScalingService implementation.'); + return {}; + } + + /** + * @return {!TargetDimensionsDef} + * @private + */ + measureTargetDimensions_() { + if (!this.targetDimensions_) { + const dimensions = targetDimensionsFor(this.sizer_); + this.targetDimensions_ = dimensions; + this.updateRootProps(dimensions); + } + return /** @type {!TargetDimensionsDef} */ ( + dev().assert(this.targetDimensions_)); + } + + /** + * Updates properties on root element when target dimensions have been + * re-measured. + * @param {!TargetDimensionsDef} unusedTargetDimensions + */ + updateRootProps(unusedTargetDimensions) { + // Intentionally left blank. + } + + /** + * @param {!Element} page + * @return {!Array} + * @protected + */ + measureScalableElements(page) { + const pageId = user().assert(page.id, 'No page id.'); + + if (!this.scalableElsDimensions_[pageId]) { + this.scalableElsDimensions_[pageId] = this.measureScalableElsFor(page); + } + + return /** @type {!Array} */ ( + dev().assert(this.scalableElsDimensions_[pageId])); + } + + /** + * Measures scalable elements in a page. + * @param {!Element} page + * @return {!Array} + */ + measureScalableElsFor(page) { + const {width, height} = unscaledClientRect(page); + const pageWidth = width; + const pageHeight = height; + return scalableElements(page).map(el => { + const {width, height} = unscaledClientRect(el); + return { + matrix: this.getTransformMatrix(el), + relativeWidth: width / pageWidth, + relativeHeight: height / pageHeight, + }; }); } + /** + * Gets an element's transform matrix. + * @param {!Element} unusedEl + * @return {?Array} + */ + getTransformMatrix(unusedEl) { + // Calculating a transform matrix is optional depending on scaling + // implementation. + return null; + } + /** @private */ onViewportResize_() { - const pages = toArray(childElementsByTag(this.story_, 'amp-story-page')); - this.dimensions_ = null; + this.targetDimensions_ = null; + this.vsync_.measure(() => { + this.measureTargetDimensions_(); + }); + this.updatePagesOnResize(); + } + + /** @protected */ + updatePagesOnResize() { + const pages = toArray(childElementsByTag(this.rootEl_, 'amp-story-page')); pages.forEach(page => { markScalingApplied(page, false); - this.scale(page); + this.scale_(page); + }); + } +} + + +/** Uses CSS zoom as scaling method. */ +class ZoomScalingService extends PageScalingService { + /** @override */ + scalingStyles(targetDimensions, elementDimensions) { + const {width, height, factor} = targetDimensions; + const {relativeWidth, relativeHeight} = elementDimensions; + return { + 'width': px(width * relativeWidth), + 'height': px(height * relativeHeight), + 'zoom': factor, + }; + } +} + + +/** Uses combined CSS transform as scaling method. */ +class TransformScalingService extends PageScalingService { + /** @override */ + getTransformMatrix(unusedEl) { + // TODO(alanorozco, #12934): Implement. + return [1, 0, 0, 1, 0, 0]; + } + + /** @override */ + scalingStyles(targetDimensions, elementDimensions) { + const {width, height, factor} = targetDimensions; + const {relativeWidth, relativeHeight, matrix} = elementDimensions; + const initialMatrix = /** @type {!Array} */ ( + dev().assert(matrix, 'No initial matrix')); + const transformedMatrix = + scaleTransform(factor, width, height, initialMatrix); + return { + 'width': px(width * relativeWidth), + 'height': px(height * relativeHeight), + 'transform': `matrix(${transformedMatrix.join()})`, + }; + } +} + + +/** Uses CSS zoom and custom CSS properties as scaling method. */ +class CssPropsZoomScalingService extends PageScalingService { + /** @override */ + measureScalableElements(page) { + // Circumvents layer cache as layers are only mutated once per page. + return this.measureScalableElsFor(page); + } + + /** @override */ + updateRootProps() { + const {width, height, factor} = this.targetDimensions_; + this.vsync_.mutate(() => { + this.rootEl_.style.setProperty('--i-amphtml-story-width', px(width)); + this.rootEl_.style.setProperty('--i-amphtml-story-height', px(height)); + this.rootEl_.style.setProperty('--i-amphtml-story-factor', + factor.toString()); }); } + + /** @override */ + updatePagesOnResize() { + // No need to update. + } + + /** @override */ + scalingStyles(unusedTargetDimensions, elementDimensions) { + const {relativeWidth, relativeHeight} = elementDimensions; + return { + 'width': `calc(var(--i-amphtml-story-width) * ${relativeWidth})`, + 'height': `calc(var(--i-amphtml-story-height) * ${relativeHeight})`, + 'zoom': 'var(--i-amphtml-story-factor)', + }; + } } From 0645c6b7f76438d4c553af14dd40568df7849732 Mon Sep 17 00:00:00 2001 From: Alan Orozco Date: Fri, 26 Jan 2018 09:03:32 -0800 Subject: [PATCH 04/13] Revert CSS changes --- extensions/amp-story/0.1/amp-story-desktop.css | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/extensions/amp-story/0.1/amp-story-desktop.css b/extensions/amp-story/0.1/amp-story-desktop.css index 920730d7a195..59e6fecf0159 100644 --- a/extensions/amp-story/0.1/amp-story-desktop.css +++ b/extensions/amp-story/0.1/amp-story-desktop.css @@ -188,21 +188,16 @@ amp-story[standalone][desktop] { } [desktop] > amp-story-page, -[desktop] > .i-amphtml-story-page-sizer, -[desktop] .i-amphtml-story-page-sentinel { +.i-amphtml-story-page-sentinel { + left: 50%!important; + right: auto !important; + margin: auto !important; max-height: 75vh !important; max-width: calc(3/5 * 75vh) !important; min-width: 320px !important; min-height: 533px !important; } -[desktop] > amp-story-page, -[desktop] .i-amphtml-story-page-sentinel { - left: 50%!important; - right: auto !important; - margin: auto !important; -} - [desktop] > amp-story-page { box-shadow: 0 0 15px rgba(0, 0, 0, .4)!important; } From 3e9d4cf7dbebe9f7ac68ebcb50108e99db8d3d45 Mon Sep 17 00:00:00 2001 From: Alan Orozco Date: Fri, 26 Jan 2018 09:05:44 -0800 Subject: [PATCH 05/13] Oops --- extensions/amp-story/0.1/amp-story.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/extensions/amp-story/0.1/amp-story.js b/extensions/amp-story/0.1/amp-story.js index 4862ffd1796c..aa5219dad990 100644 --- a/extensions/amp-story/0.1/amp-story.js +++ b/extensions/amp-story/0.1/amp-story.js @@ -165,11 +165,8 @@ const SHARE_WIDGET_PILL_CONTAINER = { * desktop view. * @private @const {string} */ -const HIDE_ON_BOOKEND_SELECTOR = [ - 'amp-story-page[active]', - 'amp-story-page[distance="1"]', - '.i-amphtml-story-system-layer', -].join(); +const HIDE_ON_BOOKEND_SELECTOR = + 'amp-story-page, .i-amphtml-story-system-layer'; export class AmpStory extends AMP.BaseElement { From ab6b31b399b536cc4369831598ea596c92ff602a Mon Sep 17 00:00:00 2001 From: Alan Orozco Date: Fri, 26 Jan 2018 10:15:50 -0800 Subject: [PATCH 06/13] Fallback to scale on iOs --- extensions/amp-story/0.1/page-scaling.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/extensions/amp-story/0.1/page-scaling.js b/extensions/amp-story/0.1/page-scaling.js index 27afcd074615..a6589184b66e 100644 --- a/extensions/amp-story/0.1/page-scaling.js +++ b/extensions/amp-story/0.1/page-scaling.js @@ -118,7 +118,7 @@ function isScalingEnabled(page) { if (isExperimentOn(win, 'amp-story-scaling')) { return true; } - return page.getAttribute('scaling') == 'outside-range'; + return page.getAttribute('scaling') == 'relative'; } @@ -220,7 +220,8 @@ export class PageScalingService { // instances in one doc. const win = toWin(story.ownerDocument.defaultView); if (!pageScalingService) { - if (!isCssZoomSupported(win)) { + // Falling back to transform on iOS because of font scaling issue. + if (!isCssZoomSupported(win) || Services.platformFor(win).isIos()) { // TODO(alanorozco, #12934): Combine transform matrix. user().warn('AMP-STORY', '`amp-story-scaling` using CSS transforms as fallback.', @@ -274,6 +275,8 @@ export class PageScalingService { // Required since layer now has a width/height set. Object.assign(style, {'box-sizing': 'border-box'}); + console.log(style); + setImportantStyles(el, style); }); markScalingApplied(page); From fb6e748b025512b0ac9b7df29931faff0435c481 Mon Sep 17 00:00:00 2001 From: Alan Orozco Date: Fri, 26 Jan 2018 10:15:59 -0800 Subject: [PATCH 07/13] Rewrite experiment description for clarity --- tools/experiments/experiments.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/experiments/experiments.js b/tools/experiments/experiments.js index baaf281746d3..6e26f845fb43 100644 --- a/tools/experiments/experiments.js +++ b/tools/experiments/experiments.js @@ -267,7 +267,7 @@ const EXPERIMENTS = [ }, { id: 'amp-story-scaling', - name: 'Enables dynamic page scaling in amp-story', + name: 'Scale pages dynamically in amp-story by default', spec: 'https://github.com/ampproject/amphtml/issues/12902', cleanupIssue: 'https://github.com/ampproject/amphtml/issues/12902', }, From 82b61371cd54b6e3bfee6cbc3877b5899500e2a9 Mon Sep 17 00:00:00 2001 From: Alan Orozco Date: Fri, 26 Jan 2018 10:26:19 -0800 Subject: [PATCH 08/13] Add TODO --- extensions/amp-story/0.1/page-scaling.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/amp-story/0.1/page-scaling.js b/extensions/amp-story/0.1/page-scaling.js index a6589184b66e..77c2a4c60b81 100644 --- a/extensions/amp-story/0.1/page-scaling.js +++ b/extensions/amp-story/0.1/page-scaling.js @@ -220,7 +220,7 @@ export class PageScalingService { // instances in one doc. const win = toWin(story.ownerDocument.defaultView); if (!pageScalingService) { - // Falling back to transform on iOS because of font scaling issue. + // TODO(alanorozco, #13064): Falling back to transform on iOS if (!isCssZoomSupported(win) || Services.platformFor(win).isIos()) { // TODO(alanorozco, #12934): Combine transform matrix. user().warn('AMP-STORY', From 6c02375caffa1e7faa96cd276d5b77b6fff6741f Mon Sep 17 00:00:00 2001 From: Alan Orozco Date: Fri, 26 Jan 2018 14:03:50 -0800 Subject: [PATCH 09/13] Whoooooops --- extensions/amp-story/0.1/page-scaling.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/extensions/amp-story/0.1/page-scaling.js b/extensions/amp-story/0.1/page-scaling.js index 77c2a4c60b81..d9329ba0e74a 100644 --- a/extensions/amp-story/0.1/page-scaling.js +++ b/extensions/amp-story/0.1/page-scaling.js @@ -275,8 +275,6 @@ export class PageScalingService { // Required since layer now has a width/height set. Object.assign(style, {'box-sizing': 'border-box'}); - console.log(style); - setImportantStyles(el, style); }); markScalingApplied(page); From 6cd35df6d10e5cf630b8e9e9acc67c5d656b4c1a Mon Sep 17 00:00:00 2001 From: Alan Orozco Date: Fri, 26 Jan 2018 14:26:26 -0800 Subject: [PATCH 10/13] Style --- extensions/amp-story/0.1/page-scaling.js | 111 ++++++++++++----------- 1 file changed, 59 insertions(+), 52 deletions(-) diff --git a/extensions/amp-story/0.1/page-scaling.js b/extensions/amp-story/0.1/page-scaling.js index d9329ba0e74a..cbca3b68dae2 100644 --- a/extensions/amp-story/0.1/page-scaling.js +++ b/extensions/amp-story/0.1/page-scaling.js @@ -198,7 +198,7 @@ export class PageScalingService { // Assumes active page to be determinant of the target size. this.sizer_ = dev().assertElement( scopedQuerySelector(rootEl, 'amp-story-page[active]'), - 'No active page found when initializing scaler.'); + 'No active page found when initializing scaling service.'); /** @private {?TargetDimensionsDef} */ this.targetDimensions_ = null; @@ -264,35 +264,21 @@ export class PageScalingService { return this.vsync_.runPromise({ measure: state => { state.targetDimensions = this.measureTargetDimensions_(); - state.scalableElsDimensions = this.measureScalableElements(page); + state.scalableElsDimensions = this.getOrMeasureScalableElsFor(page); }, mutate: state => { const {targetDimensions, scalableElsDimensions} = state; scalableElements(page).forEach((el, i) => { - const elementDimensions = scalableElsDimensions[i]; - const style = this.scalingStyles(targetDimensions, elementDimensions); - - // Required since layer now has a width/height set. - Object.assign(style, {'box-sizing': 'border-box'}); - - setImportantStyles(el, style); + // `border-box` required since layer now has a width/height set. + setImportantStyles(el, {'box-sizing': 'border-box'}); + setImportantStyles(el, + this.scalingStyles(targetDimensions, scalableElsDimensions[i])); }); markScalingApplied(page); }, }, /* state */ {}); } - /** - * @param {!TargetDimensionsDef} unusedTargetDimensions - * @param {!ScalableDimensionsDef} unusedScalableElDimensions - * @return {!Object} - * @protected - */ - scalingStyles(unusedTargetDimensions, unusedScalableElDimensions) { - dev().assert(false, 'Empty PageScalingService implementation.'); - return {}; - } - /** * @return {!TargetDimensionsDef} * @private @@ -307,21 +293,12 @@ export class PageScalingService { dev().assert(this.targetDimensions_)); } - /** - * Updates properties on root element when target dimensions have been - * re-measured. - * @param {!TargetDimensionsDef} unusedTargetDimensions - */ - updateRootProps(unusedTargetDimensions) { - // Intentionally left blank. - } - /** * @param {!Element} page * @return {!Array} * @protected */ - measureScalableElements(page) { + getOrMeasureScalableElsFor(page) { const pageId = user().assert(page.id, 'No page id.'); if (!this.scalableElsDimensions_[pageId]) { @@ -336,6 +313,7 @@ export class PageScalingService { * Measures scalable elements in a page. * @param {!Element} page * @return {!Array} + * @protected */ measureScalableElsFor(page) { const {width, height} = unscaledClientRect(page); @@ -351,17 +329,6 @@ export class PageScalingService { }); } - /** - * Gets an element's transform matrix. - * @param {!Element} unusedEl - * @return {?Array} - */ - getTransformMatrix(unusedEl) { - // Calculating a transform matrix is optional depending on scaling - // implementation. - return null; - } - /** @private */ onViewportResize_() { this.targetDimensions_ = null; @@ -372,18 +339,59 @@ export class PageScalingService { } /** @protected */ - updatePagesOnResize() { + scaleAll() { const pages = toArray(childElementsByTag(this.rootEl_, 'amp-story-page')); pages.forEach(page => { markScalingApplied(page, false); this.scale_(page); }); } + + /** + * Updates properties on root element when target dimensions have been + * re-measured. + * @param {!TargetDimensionsDef} unusedTargetDimensions + */ + updateRootProps(unusedTargetDimensions) { + // Intentionally left blank. + } + + /** @protected */ + updatePagesOnResize() { + // Intentionally left blank. + } + + /** + * Gets an element's transform matrix. + * @param {!Element} unusedEl + * @return {?Array} + */ + getTransformMatrix(unusedEl) { + // Calculating a transform matrix is optional depending on scaling + // implementation. + return null; + } + + /** + * @param {!TargetDimensionsDef} unusedTargetDimensions + * @param {!ScalableDimensionsDef} unusedScalableElDimensions + * @return {!Object} + * @protected + */ + scalingStyles(unusedTargetDimensions, unusedScalableElDimensions) { + dev().assert(false, 'Empty PageScalingService implementation.'); + return {}; + } } /** Uses CSS zoom as scaling method. */ class ZoomScalingService extends PageScalingService { + /** @protected */ + updatePagesOnResize() { + this.scaleAll(); + } + /** @override */ scalingStyles(targetDimensions, elementDimensions) { const {width, height, factor} = targetDimensions; @@ -399,6 +407,11 @@ class ZoomScalingService extends PageScalingService { /** Uses combined CSS transform as scaling method. */ class TransformScalingService extends PageScalingService { + /** @protected */ + updatePagesOnResize() { + this.scaleAll(); + } + /** @override */ getTransformMatrix(unusedEl) { // TODO(alanorozco, #12934): Implement. @@ -409,10 +422,9 @@ class TransformScalingService extends PageScalingService { scalingStyles(targetDimensions, elementDimensions) { const {width, height, factor} = targetDimensions; const {relativeWidth, relativeHeight, matrix} = elementDimensions; - const initialMatrix = /** @type {!Array} */ ( - dev().assert(matrix, 'No initial matrix')); + const initialMatrix = /** @type {!Array} */ (dev().assert(matrix)); const transformedMatrix = - scaleTransform(factor, width, height, initialMatrix); + scaleTransform(factor, width, height, initialMatrix); return { 'width': px(width * relativeWidth), 'height': px(height * relativeHeight), @@ -425,8 +437,8 @@ class TransformScalingService extends PageScalingService { /** Uses CSS zoom and custom CSS properties as scaling method. */ class CssPropsZoomScalingService extends PageScalingService { /** @override */ - measureScalableElements(page) { - // Circumvents layer cache as layers are only mutated once per page. + getOrMeasureScalableElsFor(page) { + // Circumvents element dimensions cache as layers are only mutated once. return this.measureScalableElsFor(page); } @@ -441,11 +453,6 @@ class CssPropsZoomScalingService extends PageScalingService { }); } - /** @override */ - updatePagesOnResize() { - // No need to update. - } - /** @override */ scalingStyles(unusedTargetDimensions, elementDimensions) { const {relativeWidth, relativeHeight} = elementDimensions; From ccdf4a71679d8a00e6621ca6686a6f2c04167986 Mon Sep 17 00:00:00 2001 From: Alan Orozco Date: Fri, 26 Jan 2018 15:29:59 -0800 Subject: [PATCH 11/13] Don't assert until scale time --- extensions/amp-story/0.1/page-scaling.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/extensions/amp-story/0.1/page-scaling.js b/extensions/amp-story/0.1/page-scaling.js index cbca3b68dae2..756ec727a0c8 100644 --- a/extensions/amp-story/0.1/page-scaling.js +++ b/extensions/amp-story/0.1/page-scaling.js @@ -194,11 +194,9 @@ export class PageScalingService { /** @private @const */ this.vsync_ = Services.vsyncFor(win); - /** @private @const {!Element} */ + /** @private @const {?Element} */ // Assumes active page to be determinant of the target size. - this.sizer_ = dev().assertElement( - scopedQuerySelector(rootEl, 'amp-story-page[active]'), - 'No active page found when initializing scaling service.'); + this.sizer_ = scopedQuerySelector(rootEl, 'amp-story-page[active]'); /** @private {?TargetDimensionsDef} */ this.targetDimensions_ = null; @@ -285,7 +283,8 @@ export class PageScalingService { */ measureTargetDimensions_() { if (!this.targetDimensions_) { - const dimensions = targetDimensionsFor(this.sizer_); + const sizer = dev().assertElement(this.sizer_, 'No sizer.'); + const dimensions = targetDimensionsFor(sizer); this.targetDimensions_ = dimensions; this.updateRootProps(dimensions); } From 9c509b07770da550400833b84191831c5ddde164 Mon Sep 17 00:00:00 2001 From: Alan Orozco Date: Fri, 26 Jan 2018 16:55:49 -0800 Subject: [PATCH 12/13] Update amp-story.js --- extensions/amp-story/0.1/amp-story.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/amp-story/0.1/amp-story.js b/extensions/amp-story/0.1/amp-story.js index aa5219dad990..3b4627390562 100644 --- a/extensions/amp-story/0.1/amp-story.js +++ b/extensions/amp-story/0.1/amp-story.js @@ -166,7 +166,7 @@ const SHARE_WIDGET_PILL_CONTAINER = { * @private @const {string} */ const HIDE_ON_BOOKEND_SELECTOR = - 'amp-story-page, .i-amphtml-story-system-layer'; + 'amp-story-page, .i-amphtml-story-system-layer'; export class AmpStory extends AMP.BaseElement { From 6a6c478ac5cf9c77a2f2910eab99719d2f2b0911 Mon Sep 17 00:00:00 2001 From: Alan Orozco Date: Fri, 26 Jan 2018 16:56:10 -0800 Subject: [PATCH 13/13] Update amp-story.js --- extensions/amp-story/0.1/amp-story.js | 1 - 1 file changed, 1 deletion(-) diff --git a/extensions/amp-story/0.1/amp-story.js b/extensions/amp-story/0.1/amp-story.js index 3b4627390562..ddde08cc3853 100644 --- a/extensions/amp-story/0.1/amp-story.js +++ b/extensions/amp-story/0.1/amp-story.js @@ -984,7 +984,6 @@ export class AmpStory extends AMP.BaseElement { this.bookend_.hide(); } - /** * Toggle content when bookend is opened/closed. * @param {boolean} display