Keep removing

ampproject · mszylkowski · Nov 11, 2021 · Nov 8, 2021 · Nov 8, 2021 · Nov 9, 2021
commit fc37c1503c7a1ef8de7c549d7d64d433748ab1ac
diff --git a/extensions/amp-story/1.0/amp-story-embedded-component.js b/extensions/amp-story/1.0/amp-story-embedded-component.js
@@ -32,14 +32,8 @@ import {isProtocolValid, parseUrlDeprecated} from '../../../src/url';
 import {resetStyles, setImportantStyles} from '#core/dom/style';
 import objstr from 'obj-str';
 
-/**
- * Action icons to be placed in tooltip.
- * @enum {string}
- * @private
- */
-const ActionIcon = {
-  LAUNCH: 'i-amphtml-tooltip-action-icon-launch',
-};
+/** @private @const {string} */
+const LAUNCH_ICON_CLASS = 'i-amphtml-tooltip-action-icon-launch';
 
 /** @private @const {number} */
 const TOOLTIP_CLOSE_ANIMATION_MS = 100;
@@ -55,17 +49,13 @@ const TooltipTheme = {
   DARK: 'dark',
 };
 
-/**
- * Components that can be expanded.
- * @const {!Object}
- * @package
- */
-export const EXPANDABLE_COMPONENTS = {
+/** @private @const {!Object} list of embedded components that are click shielded */
+const EMBEDDED_COMPONENTS_SELECTORS = {
   'amp-twitter': {
     customIconClassName: 'amp-social-share-twitter-no-background',
-    actionIcon: ActionIcon.LAUNCH,
+    actionIcon: LAUNCH_ICON_CLASS,
     localizedStringId: LocalizedStringId.AMP_STORY_TOOLTIP_EXPAND_TWEET,
-    selector: 'amp-twitter',
+    selector: 'amp-twitter[interactive]',
   },
 };
 
@@ -76,70 +66,45 @@ export const EXPANDABLE_COMPONENTS = {
  */
 const LAUNCHABLE_COMPONENTS = {
   'a': {
-    actionIcon: ActionIcon.LAUNCH,
+    actionIcon: LAUNCH_ICON_CLASS,
     selector: 'a[href]:not([affiliate-link-icon])',
   },
-};
-
-/**
- * Union of expandable and launchable components.
- * @private
- * @const {!Object}
- */
-const INTERACTIVE_COMPONENTS = {
-  ...EXPANDABLE_COMPONENTS,
-  ...LAUNCHABLE_COMPONENTS,
+  ...EMBEDDED_COMPONENTS_SELECTORS,
 };
 
 /**
  * Gets the list of components with their respective selectors.
  * @param {!Object} components
- * @param {string=} opt_predicate
  * @return {!Object<string, string>}
  */
-function getComponentSelectors(components, opt_predicate) {
+function getComponentSelectors(components) {
   const componentSelectors = {};
 
   Object.keys(components).forEach((componentName) => {
-    componentSelectors[componentName] = opt_predicate
-      ? components[componentName].selector + opt_predicate
-      : components[componentName].selector;
+    componentSelectors[componentName] = components[componentName].selector;
   });
 
   return componentSelectors;
 }
 
-/** @const {string} */
-const INTERACTIVE_EMBED_SELECTOR = '[interactive]';
-
 /**
- * Selectors of elements that can go into expanded view.
+ * Selectors of elements that are embedded.
  * @return {!Object}
  */
-export function expandableElementsSelectors() {
+export function embeddedElementsSelectors() {
   // Using indirect invocation to prevent no-export-side-effect issue.
-  return getComponentSelectors(
-    EXPANDABLE_COMPONENTS,
-    INTERACTIVE_EMBED_SELECTOR
-  );
+  return Object.values(
+    getComponentSelectors(EMBEDDED_COMPONENTS_SELECTORS)
+  ).join(',');
 }
 
-/**
- * Contains all interactive component CSS selectors.
- * @type {!Object}
- */
-const interactiveSelectors = {
-  ...getComponentSelectors(LAUNCHABLE_COMPONENTS),
-  ...getComponentSelectors(EXPANDABLE_COMPONENTS, INTERACTIVE_EMBED_SELECTOR),
-};
-
 /**
  * All selectors that should delegate to the AmpStoryEmbeddedComponent class.
  * @return {!Object}
  */
 export function interactiveElementsSelectors() {
   // Using indirect invocation to prevent no-export-side-effect issue.
-  return interactiveSelectors;
+  return Object.values(getComponentSelectors(LAUNCHABLE_COMPONENTS)).join(',');
 }
 
 /**
@@ -484,10 +449,7 @@ export class AmpStoryEmbeddedComponent {
    * @private
    */
   updateTooltipBehavior_(target) {
-    if (
-      matches(target, LAUNCHABLE_COMPONENTS['a'].selector) ||
-      matches(target, EXPANDABLE_COMPONENTS['amp-twitter'].selector)
-    ) {
+    if (matches(target, interactiveElementsSelectors())) {
       dev()
         .assertElement(this.tooltip_)
         .setAttribute('href', this.getElementHref_(target));
@@ -522,7 +484,7 @@ export class AmpStoryEmbeddedComponent {
    * @return {?Object}
    */
   getEmbedConfigFor_(target) {
-    const config = INTERACTIVE_COMPONENTS[target.tagName.toLowerCase()];
+    const config = LAUNCHABLE_COMPONENTS[target.tagName.toLowerCase()];
     if (config && matches(target, config.selector)) {
       return config;
     }

diff --git a/extensions/amp-story/1.0/amp-story-page.js b/extensions/amp-story/1.0/amp-story-page.js
@@ -21,7 +21,6 @@ import {
   getStoreService,
 } from './amp-story-store-service';
 import {AdvancementConfig} from './page-advancement';
-import {EXPANDABLE_COMPONENTS} from './amp-story-embedded-component';
 import {AnimationManager, hasAnimations} from './animation';
 import {CommonSignals} from '#core/constants/common-signals';
 import {Deferred} from '#core/data-structures/promise';
@@ -59,6 +58,7 @@ import {whenUpgradedToCustomElement} from '#core/dom/amp-element-helpers';
 
 import {toArray} from '#core/types/array';
 import {upgradeBackgroundAudio} from './audio';
+import {embeddedElementsSelectors} from './amp-story-embedded-component';
 
 /**
  * CSS class for an amp-story-page that indicates the entire page is loaded.
@@ -93,11 +93,6 @@ export const Selectors = {
   ALL_TABBABLE: 'a, amp-twitter > iframe',
 };
 
-/** @private @const {string} */
-const EMBEDDED_COMPONENTS_SELECTORS = Object.keys(EXPANDABLE_COMPONENTS).join(
-  ', '
-);
-
 /** @private @const {number} */
 const RESIZE_TIMEOUT_MS = 1000;
 
@@ -716,7 +711,7 @@ export class AmpStoryPage extends AMP.BaseElement {
    */
   addClickShieldToEmbeddedComponents_() {
     const componentEls = toArray(
-      scopedQuerySelectorAll(this.element, EMBEDDED_COMPONENTS_SELECTORS)
+      scopedQuerySelectorAll(this.element, embeddedElementsSelectors())
     );
 
     if (componentEls.length <= 0) {

diff --git a/extensions/amp-story/1.0/amp-story-tooltip.css b/extensions/amp-story/1.0/amp-story-tooltip.css
@@ -149,18 +149,10 @@
   background-image: url('data:image/svg+xml;charset=utf-8,<svg xmlns="http://www.w3.org/2000/svg" width="18px" height="18px" viewBox="0 0 48 48" fill="%23757575"><path d="M0 0h48v48H0z" fill="none"/><path d="M38 38H10V10h14V6H10c-2.21 0-4 1.79-4 4v28c0 2.21 1.79 4 4 4h28c2.21 0 4-1.79 4-4V24h-4v14zM28 6v4h7.17L15.51 29.66l2.83 2.83L38 12.83V20h4V6H28z"/></svg>') !important;
 }
 
-.i-amphtml-tooltip-action-icon-expand {
-  background-image: url('data:image/svg+xml;charset=utf-8,<svg xmlns="http://www.w3.org/2000/svg" width="18px" height="18px" viewBox="0 0 48 48" fill="#757575"><path d="M17.17 32.92l9.17-9.17-9.17-9.17L20 11.75l12 12-12 12z"/><path d="M0-.25h48v48H0z" fill="none"/></svg>') !important;
-}
-
 .i-amphtml-story-tooltip-theme-dark .i-amphtml-tooltip-action-icon-launch {
   background-image: url('data:image/svg+xml;charset=utf-8,<svg xmlns="http://www.w3.org/2000/svg" width="18px" height="18px" viewBox="0 0 48 48" fill="rgba(255, 255, 255, 0.54)"><path d="M0 0h48v48H0z" fill="none"/><path d="M38 38H10V10h14V6H10c-2.21 0-4 1.79-4 4v28c0 2.21 1.79 4 4 4h28c2.21 0 4-1.79 4-4V24h-4v14zM28 6v4h7.17L15.51 29.66l2.83 2.83L38 12.83V20h4V6H28z"/></svg>') !important;
 }
 
-.i-amphtml-story-tooltip-theme-dark .i-amphtml-tooltip-action-icon-expand {
-  background-image: url('data:image/svg+xml;charset=utf-8,<svg xmlns="http://www.w3.org/2000/svg" width="18px" height="18px" viewBox="0 0 48 48" fill="rgba(255, 255, 255, 0.54)"><path d="M17.17 32.92l9.17-9.17-9.17-9.17L20 11.75l12 12-12 12z"/><path d="M0-.25h48v48H0z" fill="none"/></svg>') !important;
-}
-
 .i-amphtml-story-tooltip-arrow {
   border-right: 8px solid transparent !important;
   border-left: 8px solid transparent !important;

diff --git a/extensions/amp-story/1.0/page-advancement.js b/extensions/amp-story/1.0/page-advancement.js
@@ -16,8 +16,8 @@ import {dev, user} from '#utils/log';
 import {escapeCssSelectorIdent} from '#core/dom/css-selectors';
 import {getAmpdoc} from '../../../src/service-helpers';
 import {hasTapAction, timeStrToMillis} from './utils';
-import {interactiveElementsSelectors} from './amp-story-embedded-component';
 import {listenOnce} from '#utils/event-helper';
+import {interactiveElementsSelectors} from './amp-story-embedded-component';
 
 /** @private @const {number} */
 const HOLD_TOUCH_THRESHOLD_MS = 500;
@@ -59,10 +59,6 @@ const MINIMUM_TIME_BASED_AUTO_ADVANCE_MS = 500;
  */
 const MAX_LINK_SCREEN_PERCENT = 0.8;
 
-const INTERACTIVE_EMBEDDED_COMPONENTS_SELECTORS = Object.values(
-  interactiveElementsSelectors()
-).join(',');
-
 /** @const {number} */
 export const POLL_INTERVAL_MS = 300;
 
@@ -644,7 +640,7 @@ export class ManualAdvancement extends AdvancementConfig {
 
     return (
       inExpandedMode ||
-      (matches(target, INTERACTIVE_EMBEDDED_COMPONENTS_SELECTORS) &&
+      (matches(target, interactiveElementsSelectors()) &&
         this.canShowTooltip_(event, pageRect))
     );
   }