🚀 [Story attachment] Move page-attachment to separate extension (#37253)

* Started moving page attachment * Create bundle * Fixed tests * Fixed import * Not use service getters to avoid bundling * Fix services * Fix shopping unit test * Removed dependency on localizationservice * Fixed localization * Fixed tests * Sync service fetchers * Made services sync * Fixed tests for sync services * Remove unused import in test * AnalyticsService to sync * Import devAssert * Remove param to buildInline * Undo describes.js * Fix tests * Remove unused import * Debundle store with forms * Install extension without check
ampproject · Dec 23, 2021 · 3d06e61 · 3d06e61
1 parent 9d0306f
commit 3d06e61
Show file tree

Hide file tree

Showing 17 changed files with 181 additions and 89 deletions.
diff --git a/build-system/compile/bundles.config.extensions.json b/build-system/compile/bundles.config.extensions.json
@@ -871,6 +871,13 @@
       "hasCss": true
     }
   },
+  {
+    "name": "amp-story-page-attachment",
+    "version": "0.1",
+    "options": {
+      "hasCss": true
+    }
+  },
   {
     "name": "amp-story-panning-media",
     "version": "0.1",

diff --git a/build-system/test-configs/dep-check-config.js b/build-system/test-configs/dep-check-config.js
@@ -302,6 +302,16 @@ exports.rules = [
       // amp-smartlinks depends on amp-skimlinks/link-rewriter
       'extensions/amp-smartlinks/0.1/amp-smartlinks.js->extensions/amp-skimlinks/0.1/link-rewriter/link-rewriter-manager.js',
       'extensions/amp-smartlinks/0.1/linkmate.js->extensions/amp-skimlinks/0.1/link-rewriter/two-steps-response.js',
+
+      'extensions/amp-story-page-attachment/0.1/amp-story-draggable-drawer.js->extensions/amp-story/1.0/amp-story-store-service.js',
+      'extensions/amp-story-page-attachment/0.1/amp-story-draggable-drawer.js->extensions/amp-story/1.0/utils.js',
+      'extensions/amp-story-page-attachment/0.1/amp-story-form.js->extensions/amp-story/1.0/amp-story-store-service.js',
+      'extensions/amp-story-page-attachment/0.1/amp-story-form.js->extensions/amp-story/1.0/loading-spinner.js',
+      'extensions/amp-story-page-attachment/0.1/amp-story-page-attachment.js->extensions/amp-story/1.0/amp-story-open-page-attachment.js',
+      'extensions/amp-story-page-attachment/0.1/amp-story-page-attachment.js->extensions/amp-story/1.0/amp-story-store-service.js',
+      'extensions/amp-story-page-attachment/0.1/amp-story-page-attachment.js->extensions/amp-story/1.0/history.js',
+      'extensions/amp-story-page-attachment/0.1/amp-story-page-attachment.js->extensions/amp-story/1.0/story-analytics.js',
+      'extensions/amp-story-page-attachment/0.1/amp-story-page-attachment.js->extensions/amp-story/1.0/utils.js',
     ],
   },
   {

diff --git a/css/Z_INDEX.md b/css/Z_INDEX.md
@@ -49,7 +49,7 @@
 | `.i-amphtml-story-focused-state-layer`                                                           | 100001     | [extensions/amp-story/1.0/amp-story-tooltip.css](/extensions/amp-story/1.0/amp-story-tooltip.css)                                                                       |
 | `.i-amphtml-story-360-activate-button`                                                           | 100000     | [extensions/amp-story-360/0.1/amp-story-360.css](/extensions/amp-story-360/0.1/amp-story-360.css)                                                                       |
 | `.i-amphtml-story-360-discovery`                                                                 | 100000     | [extensions/amp-story-360/0.1/amp-story-360.css](/extensions/amp-story-360/0.1/amp-story-360.css)                                                                       |
-| `.i-amphtml-story-page-attachment-expand`                                                        | 100000     | [extensions/amp-story/1.0/amp-story-page-attachment.css](/extensions/amp-story/1.0/amp-story-page-attachment.css)                                                       |
+| `.i-amphtml-story-page-attachment-expand`                                                        | 100000     | [extensions/amp-story-page-attachment/0.1/amp-story-page-attachment.css](/extensions/amp-story-page-attachment/0.1/amp-story-page-attachment.css)                       |
 | `.i-amphtml-story-system-layer`                                                                  | 100000     | [extensions/amp-story/1.0/amp-story-system-layer.css](/extensions/amp-story/1.0/amp-story-system-layer.css)                                                             |
 | `.i-amphtml-story-page-error`                                                                    | 10000      | [extensions/amp-story/1.0/amp-story.css](/extensions/amp-story/1.0/amp-story.css)                                                                                       |
 | `.i-amphtml-story-page-play-button`                                                              | 10000      | [extensions/amp-story/1.0/amp-story.css](/extensions/amp-story/1.0/amp-story.css)                                                                                       |
@@ -77,7 +77,7 @@
 | `.i-amphtml-byside-content-loading-container .i-amphtml-byside-content-loading-animation:before` | 9          | [extensions/amp-byside-content/0.1/amp-byside-content.css](/extensions/amp-byside-content/0.1/amp-byside-content.css)                                                   |
 | `100%`                                                                                           | 9          | [extensions/amp-byside-content/0.1/amp-byside-content.css](/extensions/amp-byside-content/0.1/amp-byside-content.css)                                                   |
 | `.i-amphtml-story-ad-attribution`                                                                | 4          | [extensions/amp-story-auto-ads/0.1/amp-story-auto-ads-attribution.css](/extensions/amp-story-auto-ads/0.1/amp-story-auto-ads-attribution.css)                           |
-| `.amp-story-draggable-drawer-root`                                                               | 4          | [extensions/amp-story/1.0/amp-story-draggable-drawer.css](/extensions/amp-story/1.0/amp-story-draggable-drawer.css)                                                     |
+| `.amp-story-draggable-drawer-root`                                                               | 4          | [extensions/amp-story-page-attachment/0.1/amp-story-draggable-drawer.css](/extensions/amp-story-page-attachment/0.1/amp-story-draggable-drawer.css)                     |
 | `.i-amphtml-image-slider-bar`                                                                    | 3          | [extensions/amp-image-slider/0.1/amp-image-slider.css](/extensions/amp-image-slider/0.1/amp-image-slider.css)                                                           |
 | `styles`                                                                                         | 3          | [extensions/amp-story-interactive/0.1/amp-story-interactive-abstract.js](/extensions/amp-story-interactive/0.1/amp-story-interactive-abstract.js)                       |
 | `.i-amphtml-story-page-open-attachment[active]`                                                  | 3          | [extensions/amp-story/1.0/amp-story-open-page-attachment.css](/extensions/amp-story/1.0/amp-story-open-page-attachment.css)                                             |
@@ -136,8 +136,8 @@
 | `.i-amphtml-story-interactive-slider-input`                                                      | 1          | [extensions/amp-story-interactive/0.1/amp-story-interactive-slider.css](/extensions/amp-story-interactive/0.1/amp-story-interactive-slider.css)                         |
 | `.i-amphtml-story-interactive-slider-input::-webkit-slider-thumb`                                | 1          | [extensions/amp-story-interactive/0.1/amp-story-interactive-slider.css](/extensions/amp-story-interactive/0.1/amp-story-interactive-slider.css)                         |
 | `.i-amphtml-story-interactive-confetti-wrapper`                                                  | 1          | [extensions/amp-story-interactive/0.1/amp-story-interactive.css](/extensions/amp-story-interactive/0.1/amp-story-interactive.css)                                       |
+| `.i-amphtml-story-draggable-drawer-header`                                                       | 1          | [extensions/amp-story-page-attachment/0.1/amp-story-draggable-drawer-header.css](/extensions/amp-story-page-attachment/0.1/amp-story-draggable-drawer-header.css)       |
 | `.i-amphtml-story-consent-actions`                                                               | 1          | [extensions/amp-story/1.0/amp-story-consent.css](/extensions/amp-story/1.0/amp-story-consent.css)                                                                       |
-| `.i-amphtml-story-draggable-drawer-header`                                                       | 1          | [extensions/amp-story/1.0/amp-story-draggable-drawer-header.css](/extensions/amp-story/1.0/amp-story-draggable-drawer-header.css)                                       |
 | `.i-amphtml-story-hint-container .i-amphtml-story-hint-tap-button-icon`                          | 1          | [extensions/amp-story/1.0/amp-story-hint.css](/extensions/amp-story/1.0/amp-story-hint.css)                                                                             |
 | `amp-story-page[active]`                                                                         | 1          | [extensions/amp-story/1.0/amp-story.css](/extensions/amp-story/1.0/amp-story.css)                                                                                       |
 | `.i-amphtml-stream-gallery-next`                                                                 | 1          | [extensions/amp-stream-gallery/0.1/arrows.css](/extensions/amp-stream-gallery/0.1/arrows.css)                                                                           |

diff --git a/...1.0/amp-story-draggable-drawer-header.css → ...0.1/amp-story-draggable-drawer-header.css b/...1.0/amp-story-draggable-drawer-header.css → ...0.1/amp-story-draggable-drawer-header.css
@@ -1,6 +1,6 @@
 
 
-@import './amp-story-shadow-reset.css';
+@import '../../amp-story/1.0/amp-story-shadow-reset.css';
 
 :host {
   border-radius: inherit !important; /* Propogate border-radius */

diff --git a/...-story/1.0/amp-story-draggable-drawer.css → ...chment/0.1/amp-story-draggable-drawer.css b/...-story/1.0/amp-story-draggable-drawer.css → ...chment/0.1/amp-story-draggable-drawer.css
diff --git a/...p-story/1.0/amp-story-draggable-drawer.js → ...achment/0.1/amp-story-draggable-drawer.js b/...p-story/1.0/amp-story-draggable-drawer.js → ...achment/0.1/amp-story-draggable-drawer.js
@@ -1,21 +1,23 @@
+import {devAssert} from '#core/assert';
+import {isAmpElement} from '#core/dom/amp-element-helpers';
 import * as Preact from '#core/dom/jsx';
+import {Layout_Enum} from '#core/dom/layout';
+import {closest} from '#core/dom/query';
+import {resetStyles, setImportantStyles, toggle} from '#core/dom/style';
+
+import {Services} from '#service';
+import {LocalizedStringId_Enum} from '#service/localization/strings';
+
+import {listen} from '#utils/event-helper';
+import {dev} from '#utils/log';
+
+import {CSS} from '../../../build/amp-story-draggable-drawer-header-0.1.css';
 import {
   Action,
   StateProperty,
   UIType,
-  getStoreService,
-} from './amp-story-store-service';
-import {CSS} from '../../../build/amp-story-draggable-drawer-header-1.0.css';
-import {Layout_Enum} from '#core/dom/layout';
-import {LocalizedStringId_Enum} from '#service/localization/strings';
-import {Services} from '#service';
-import {closest} from '#core/dom/query';
-import {createShadowRootWithStyle} from './utils';
-import {dev} from '#utils/log';
-import {localize} from './amp-story-localization-service';
-import {isAmpElement} from '#core/dom/amp-element-helpers';
-import {listen} from '#utils/event-helper';
-import {resetStyles, setImportantStyles, toggle} from '#core/dom/style';
+} from '../../amp-story/1.0/amp-story-store-service';
+import {createShadowRootWithStyle} from '../../amp-story/1.0/utils';
 
 /** @const {number} */
 const TOGGLE_THRESHOLD_PX = 50;
@@ -87,8 +89,13 @@ export class DraggableDrawer extends AMP.BaseElement {
     /** @protected {!DrawerState} */
     this.state = DrawerState.CLOSED;
 
-    /** @protected @const {!./amp-story-store-service.AmpStoryStoreService} */
-    this.storeService = getStoreService(this.win);
+    /** @protected @const {!../../amp-story/1.0/amp-story-store-service.AmpStoryStoreService} */
+    this.storeService = devAssert(Services.storyStoreService(this.win));
+
+    /** @protected @const {!../../../src/services/localization.LocalizationService} */
+    this.localizationService = devAssert(
+      Services.localizationForDoc(this.element)
+    );
 
     /** @private {!Object} */
     this.touchEventState_ = {
@@ -137,8 +144,7 @@ export class DraggableDrawer extends AMP.BaseElement {
       <button
         role="button"
         class="i-amphtml-story-draggable-drawer-spacer i-amphtml-story-system-reset"
-        aria-label={localize(
-          this.element,
+        aria-label={this.localizationService.getLocalizedString(
           LocalizedStringId_Enum.AMP_STORY_CLOSE_BUTTON_LABEL
         )}
       ></button>

diff --git a/extensions/amp-story/1.0/amp-story-form.css → ...ry-page-attachment/0.1/amp-story-form.css b/extensions/amp-story/1.0/amp-story-form.css → ...ry-page-attachment/0.1/amp-story-form.css
diff --git a/extensions/amp-story/1.0/amp-story-form.js → ...ory-page-attachment/0.1/amp-story-form.js b/extensions/amp-story/1.0/amp-story-form.js → ...ory-page-attachment/0.1/amp-story-form.js
@@ -1,16 +1,21 @@
+import {escapeCssSelectorIdent} from '#core/dom/css-selectors';
 import * as Preact from '#core/dom/jsx';
-import {Action, getStoreService} from './amp-story-store-service';
-import {renderLoadingSpinner, toggleLoadingSpinner} from './loading-spinner';
+
+import {Services} from '#service';
 import {LocalizedStringId_Enum} from '#service/localization/strings';
-import {escapeCssSelectorIdent} from '#core/dom/css-selectors';
-import {localize} from './amp-story-localization-service';
+
+import {Action} from '../../amp-story/1.0/amp-story-store-service';
+import {
+  renderLoadingSpinner,
+  toggleLoadingSpinner,
+} from '../../amp-story/1.0/loading-spinner';
 
 /**
  * Adds AMP form actions to the action allow list.
  * @param {!Window} win
  */
 export function allowlistFormActions(win) {
-  const storeService = getStoreService(win);
+  const storeService = Services.storyStoreService(win);
   storeService.dispatch(Action.ADD_TO_ACTIONS_ALLOWLIST, [
     {tagOrTarget: 'FORM', method: 'clear'},
     {tagOrTarget: 'FORM', method: 'submit'},
@@ -28,26 +33,31 @@ let FormElementChildrenDef;
 const createStatusChildrenByAttribute = {
   'submitting': () => toggleLoadingSpinner(renderLoadingSpinner(), true),
 
-  'submit-success': (formEl) =>
+  'submit-success': (localizationService) =>
     createFormResultChildren(
-      localize(formEl, LocalizedStringId_Enum.AMP_STORY_FORM_SUBMIT_SUCCESS)
+      localizationService.getLocalizedString(
+        LocalizedStringId_Enum.AMP_STORY_FORM_SUBMIT_SUCCESS
+      )
     ),
 
-  'submit-error': (formEl) =>
+  'submit-error': (localizationService) =>
     createFormResultChildren(
-      localize(formEl, LocalizedStringId_Enum.AMP_STORY_FORM_SUBMIT_ERROR)
+      localizationService.getLocalizedString(
+        LocalizedStringId_Enum.AMP_STORY_FORM_SUBMIT_ERROR
+      )
     ),
 };
 
 /**
  * Add a default form attribute element for each absent response attribute.
  * @param {!Element} formEl The form to which the attribute elements will be
  *     selected or added.
+ * @param {!../../../src/service/localization.LocalizationService} localizationService
  * @return {Array<!Element>} The list of response attribute elements that
  *     belong to the given form.
  * @private
  */
-export function setupResponseAttributeElements(formEl) {
+export function setupResponseAttributeElements(formEl, localizationService) {
   return Object.keys(createStatusChildrenByAttribute).map((attr) => {
     const selected = formEl.querySelector(`[${escapeCssSelectorIdent(attr)}]`);
     if (selected) {
@@ -56,7 +66,7 @@ export function setupResponseAttributeElements(formEl) {
     const created = (
       <div>
         <div class="i-amphtml-story-page-attachment-form-submission-status">
-          {createStatusChildrenByAttribute[attr](formEl)}
+          {createStatusChildrenByAttribute[attr](localizationService)}
         </div>
       </div>
     );

diff --git a/...p-story/1.0/amp-story-page-attachment.css → ...achment/0.1/amp-story-page-attachment.css b/...p-story/1.0/amp-story-page-attachment.css → ...achment/0.1/amp-story-page-attachment.css
@@ -1,4 +1,5 @@
-
+@import "./amp-story-draggable-drawer.css";
+@import './amp-story-form.css';
 
 /** Remote attachment styles. */