🚮 Clean up dead amp-sidebar code within stories (#36178)

* Remove amp-sidebar code from extensions/amp-story * Remove amp-sidebar visual tests from examples/visual-tests/amp-story * Remove reference to amp-sidebar from amp-story-interactive README * Remove remaining amp-sidebar logic from extensions/amp-story * Remove amp-sidebar logic from amp-story-system-layer code * Remove amp-sidebar code from amp-story-store-service.js * Remove amp-sidebar storybook JS files * A few missed deletions in test-amp-story and build-system/ caught by linter * Revert extensions/amp-sidebar/*/storybook removal from forbidden terms * Add back the amp-sidebar storybook files * Add newline at end of each storybook file * Run amp get-zindex --fix
ampproject · Sep 30, 2021 · 5bcd648 · 5bcd648
1 parent 098310c
commit 5bcd648
Show file tree

Hide file tree

Showing 12 changed files with 2 additions and 506 deletions.
diff --git a/css/Z_INDEX.md b/css/Z_INDEX.md
@@ -40,7 +40,6 @@
 | `.i-amphtml-story-toast`                                                                         | 100004     | [extensions/amp-story/1.0/amp-story.css](/extensions/amp-story/1.0/amp-story.css)                                                                                       |
 | `amp-story-access`                                                                               | 100003     | [extensions/amp-story/1.0/amp-story-access.css](/extensions/amp-story/1.0/amp-story-access.css)                                                                         |
 | `.i-amphtml-story-share-menu`                                                                    | 100003     | [extensions/amp-story/1.0/amp-story-share-menu.css](/extensions/amp-story/1.0/amp-story-share-menu.css)                                                                 |
-| `.i-amphtml-story-opacity-mask`                                                                  | 100003     | [extensions/amp-story/1.0/amp-story.css](/extensions/amp-story/1.0/amp-story.css)                                                                                       |
 | `.i-amphtml-story-has-new-page-notification-container`                                           | 100002     | [extensions/amp-story/1.0/amp-story-system-layer.css](/extensions/amp-story/1.0/amp-story-system-layer.css)                                                             |
 | `amp-story[standalone] .i-amphtml-story-developer-log`                                           | 100002     | [extensions/amp-story/1.0/amp-story.css](/extensions/amp-story/1.0/amp-story.css)                                                                                       |
 | `.i-amphtml-story-button-container`                                                              | 100002     | [extensions/amp-story/1.0/pagination-buttons.css](/extensions/amp-story/1.0/pagination-buttons.css)                                                                     |

diff --git a/examples/visual-tests/amp-story/amp-story-sidebar.html b/examples/visual-tests/amp-story/amp-story-sidebar.html
diff --git a/examples/visual-tests/amp-story/amp-story-sidebar.js b/examples/visual-tests/amp-story/amp-story-sidebar.js
diff --git a/extensions/amp-story-interactive/README.md b/extensions/amp-story-interactive/README.md
@@ -382,7 +382,7 @@ This is a starting point for creating the inputs on a creation tool (and just a
 
 -   _Styles_: A good starting point for customizing the style is to provide a color picker for accent color (and can additionally provide gradients for the prompt-background), and have dropdowns to select the theme=dark|light) and chip-style=flat|shadow. Dropdowns are better for future-proofing the attributes, as we may add more styles later.
 -   _Fields_: Prompt text can be a field that, if left empty, the tool doesn't specify it in the component. Options should be a list of custom fields, where users start with 2 options and can add new options (up to 4 if not a binary-poll). Each option requires a text, but more attributes such as the confetti/correct can be assigned to each option.
--   _Results_: It's useful to guide the users through creating multi-page results. For quizzes, users can add the results component to any page, but it's a good idea to warn users that they need quizzes on previous pages to use it correctly. Users can edit the attributes in a WYSIWYG editor for the multiple states if tools provide a dropdown to change the visible option, allowing users to input one state at a time (thresholds can be specified on a sidebar for example). Poll results can also be edited in a WYSIWYG manner, but it's a good idea to first require users to create the categories in the results component, and provide a dropdown on each poll option to link it to a category of the results page. This prevents errors in matching the names of the categories across components (eg: titlecase vs lowercase, extra spaces, etc) that are hard to debug from a user standpoint.
+-   _Results_: It's useful to guide the users through creating multi-page results. For quizzes, users can add the results component to any page, but it's a good idea to warn users that they need quizzes on previous pages to use it correctly. Users can edit the attributes in a WYSIWYG editor for the multiple states if tools provide a dropdown to change the visible option, allowing users to input one state at a time. Poll results can also be edited in a WYSIWYG manner, but it's a good idea to first require users to create the categories in the results component, and provide a dropdown on each poll option to link it to a category of the results page. This prevents errors in matching the names of the categories across components (eg: titlecase vs lowercase, extra spaces, etc) that are hard to debug from a user standpoint.
 
 **What if I want to animate the interactive components?**
 

diff --git a/extensions/amp-story/1.0/amp-story-desktop-user-overridable.css b/extensions/amp-story/1.0/amp-story-desktop-user-overridable.css
@@ -4,7 +4,3 @@
 .i-amphtml-story-background {
   background-color: transparent;
 }
-
-amp-sidebar {
-  background-color: #ffffff;
-}
diff --git a/extensions/amp-story/1.0/amp-story-store-service.js b/extensions/amp-story/1.0/amp-story-store-service.js
@@ -87,7 +87,6 @@ export let InteractiveReactData;
  *    desktopState: boolean,
  *    educationState: boolean,
  *    gyroscopeEnabledState: string,
- *    hasSidebarState: boolean,
  *    infoDialogState: boolean,
  *    interactiveEmbeddedComponentState: !InteractiveComponentDef,
  *    interactiveReactState: !Map<string, !InteractiveReactData>,
@@ -100,7 +99,6 @@ export let InteractiveReactData;
  *    previewState: boolean,
  *    rtlState: boolean,
  *    shareMenuState: boolean,
- *    sidebarState: boolean,
  *    storyHasAudioState: boolean,
  *    storyHasPlaybackUiState: boolean,
  *    storyHasBackgroundAudioState: boolean,
@@ -139,7 +137,6 @@ export const StateProperty = {
   AFFILIATE_LINK_STATE: 'affiliateLinkState',
   EDUCATION_STATE: 'educationState',
   GYROSCOPE_PERMISSION_STATE: 'gyroscopePermissionState',
-  HAS_SIDEBAR_STATE: 'hasSidebarState',
   INFO_DIALOG_STATE: 'infoDialogState',
   INTERACTIVE_COMPONENT_STATE: 'interactiveEmbeddedComponentState',
   // State of interactive components (polls, quizzes) on the story.
@@ -154,7 +151,6 @@ export const StateProperty = {
   PREVIEW_STATE: 'previewState',
   RTL_STATE: 'rtlState',
   SHARE_MENU_STATE: 'shareMenuState',
-  SIDEBAR_STATE: 'sidebarState',
   SUPPORTED_BROWSER_STATE: 'supportedBrowserState',
   // Any page has audio, or amp-story has a `background-audio` attribute.
   STORY_HAS_AUDIO_STATE: 'storyHasAudioState',
@@ -190,7 +186,6 @@ export const Action = {
   TOGGLE_AD: 'toggleAd',
   TOGGLE_AFFILIATE_LINK: 'toggleAffiliateLink',
   TOGGLE_EDUCATION: 'toggleEducation',
-  TOGGLE_HAS_SIDEBAR: 'toggleHasSidebar',
   TOGGLE_INFO_DIALOG: 'toggleInfoDialog',
   TOGGLE_INTERACTIVE_COMPONENT: 'toggleInteractiveComponent',
   TOGGLE_KEYBOARD_ACTIVE_STATE: 'toggleKeyboardActiveState',
@@ -201,7 +196,6 @@ export const Action = {
   TOGGLE_PAUSED: 'togglePaused',
   TOGGLE_RTL: 'toggleRtl',
   TOGGLE_SHARE_MENU: 'toggleShareMenu',
-  TOGGLE_SIDEBAR: 'toggleSidebar',
   TOGGLE_SUPPORTED_BROWSER: 'toggleSupportedBrowser',
   TOGGLE_STORY_HAS_AUDIO: 'toggleStoryHasAudio',
   TOGGLE_STORY_HAS_BACKGROUND_AUDIO: 'toggleStoryHasBackgroundAudio',
@@ -381,21 +375,6 @@ const actions = (state, action, data) => {
         ...state,
         [StateProperty.KEYBOARD_ACTIVE_STATE]: !!data,
       });
-    case Action.TOGGLE_SIDEBAR:
-      // Don't change the PAUSED_STATE if SIDEBAR_STATE is not changed.
-      if (state[StateProperty.SIDEBAR_STATE] === data) {
-        return state;
-      }
-      return /** @type {!State} */ ({
-        ...state,
-        [StateProperty.PAUSED_STATE]: !!data,
-        [StateProperty.SIDEBAR_STATE]: !!data,
-      });
-    case Action.TOGGLE_HAS_SIDEBAR:
-      return /** @type {!State} */ ({
-        ...state,
-        [StateProperty.HAS_SIDEBAR_STATE]: !!data,
-      });
     case Action.TOGGLE_SUPPORTED_BROWSER:
       return /** @type {!State} */ ({
         ...state,
@@ -573,7 +552,6 @@ export class AmpStoryStoreService {
       [StateProperty.AFFILIATE_LINK_STATE]: null,
       [StateProperty.EDUCATION_STATE]: false,
       [StateProperty.GYROSCOPE_PERMISSION_STATE]: '',
-      [StateProperty.HAS_SIDEBAR_STATE]: false,
       [StateProperty.INFO_DIALOG_STATE]: false,
       [StateProperty.INTERACTIVE_COMPONENT_STATE]: {
         state: EmbeddedComponentState.HIDDEN,
@@ -588,7 +566,6 @@ export class AmpStoryStoreService {
       [StateProperty.PAUSED_STATE]: false,
       [StateProperty.RTL_STATE]: false,
       [StateProperty.SHARE_MENU_STATE]: false,
-      [StateProperty.SIDEBAR_STATE]: false,
       [StateProperty.SUPPORTED_BROWSER_STATE]: true,
       [StateProperty.STORY_HAS_AUDIO_STATE]: false,
       [StateProperty.STORY_HAS_BACKGROUND_AUDIO_STATE]: false,

diff --git a/extensions/amp-story/1.0/amp-story-system-layer.css b/extensions/amp-story/1.0/amp-story-system-layer.css
@@ -293,15 +293,10 @@
 .i-amphtml-story-mute-text,
 .i-amphtml-story-unmute-sound-text,
 .i-amphtml-story-unmute-no-sound-text,
-.i-amphtml-story-sidebar-control,
 .i-amphtml-story-skip-to-next  {
   display: none !important;
 }
 
-[i-amphtml-story-has-sidebar] .i-amphtml-story-sidebar-control {
-  display: block !important;
-}
-
 .i-amphtml-story-has-audio:not([muted]) .i-amphtml-story-mute-audio-control,
 .i-amphtml-story-has-audio[muted] .i-amphtml-story-unmute-audio-control {
   display: block !important;
@@ -361,10 +356,6 @@
   background-image: url('data:image/svg+xml;charset=utf-8,<svg xmlns="http://www.w3.org/2000/svg" width="32px" height="32px" viewBox="0 0 32 32" fill="#FFFFFF"><path d="M23.5211 16.335L9 25.67V7L23.5211 16.335Z"/></svg>') !important;
 }
 
-.i-amphtml-story-sidebar-control {
-  background-image: url('data:image/svg+xml;charset=utf-8,<svg xmlns="http://www.w3.org/2000/svg" width="24px" height="24px" viewBox="0 0 24 24" fill="#FFFFFF"><path d="M3 18h18v-2H3v2zm0-5h18v-2H3v2zm0-7v2h18V6H3z"/><path d="M0 0h24v24H0z" fill="none"/></svg>') !important;
-}
-
 .i-amphtml-story-share-control {
   background-image: url('data:image/svg+xml;charset=utf-8,<svg xmlns="http://www.w3.org/2000/svg" width="24px" height="24px" viewBox="0 0 24 24" fill="#FFFFFF"><path d="M15.6392 16.4092C16.125 15.9509 16.7758 15.6667 17.5 15.6667C19.0217 15.6667 20.25 16.895 20.25 18.4167C20.25 19.9384 19.0217 21.1667 17.5 21.1667C15.9783 21.1667 14.75 19.9384 14.75 18.4167C14.75 18.1967 14.7867 17.9859 14.8325 17.7842L8.37 14.0075C7.875 14.4659 7.22417 14.75 6.5 14.75C4.97833 14.75 3.75 13.5217 3.75 12C3.75 10.4784 4.97833 9.25004 6.5 9.25004C7.22417 9.25004 7.875 9.53421 8.37 9.99254L14.8325 6.22504C14.7867 6.02337 14.75 5.80337 14.75 5.58337C14.75 4.06171 15.9783 2.83337 17.5 2.83337C19.0217 2.83337 20.25 4.06171 20.25 5.58337C20.25 7.10504 19.0217 8.33337 17.5 8.33337C16.7758 8.33337 16.1158 8.04921 15.63 7.59087L9.1675 11.3584C9.21333 11.5692 9.25 11.78 9.25 12C9.25 12.22 9.21333 12.4309 9.1675 12.6417L15.6392 16.4092ZM18.4167 5.58337C18.4167 5.07921 18.0042 4.66671 17.5 4.66671C16.9958 4.66671 16.5833 5.07921 16.5833 5.58337C16.5833 6.08754 16.9958 6.50004 17.5 6.50004C18.0042 6.50004 18.4167 6.08754 18.4167 5.58337ZM6.5 12.9167C5.99583 12.9167 5.58333 12.5042 5.58333 12C5.58333 11.4959 5.99583 11.0834 6.5 11.0834C7.00417 11.0834 7.41667 11.4959 7.41667 12C7.41667 12.5042 7.00417 12.9167 6.5 12.9167ZM16.5833 18.4167C16.5833 18.9209 16.9958 19.3334 17.5 19.3334C18.0042 19.3334 18.4167 18.9209 18.4167 18.4167C18.4167 17.9125 18.0042 17.5 17.5 17.5C16.9958 17.5 16.5833 17.9125 16.5833 18.4167Z" fill-rule="evenodd"/></svg>') !important;
 }

diff --git a/extensions/amp-story/1.0/amp-story-system-layer.js b/extensions/amp-story/1.0/amp-story-system-layer.js
@@ -71,18 +71,12 @@ const MESSAGE_DISPLAY_CLASS = 'i-amphtml-story-messagedisplay';
 /** @private @const {string} */
 const CURRENT_PAGE_HAS_AUDIO_ATTRIBUTE = 'i-amphtml-current-page-has-audio';
 
-/** @private @const {string} */
-const HAS_SIDEBAR_ATTRIBUTE = 'i-amphtml-story-has-sidebar';
-
 /** @private @const {string} */
 const SHARE_CLASS = 'i-amphtml-story-share-control';
 
 /** @private @const {string} */
 const INFO_CLASS = 'i-amphtml-story-info-control';
 
-/** @private @const {string} */
-const SIDEBAR_CLASS = 'i-amphtml-story-sidebar-control';
-
 /** @private @const {string} */
 const HAS_NEW_PAGE_ATTRIBUTE = 'i-amphtml-story-has-new-page';
 
@@ -266,13 +260,6 @@ const TEMPLATE = {
           }),
           localizedLabelId: LocalizedStringId.AMP_STORY_SHARE_BUTTON_LABEL,
         },
-        {
-          tag: 'button',
-          attrs: dict({
-            'class': SIDEBAR_CLASS + ' i-amphtml-story-button',
-          }),
-          localizedLabelId: LocalizedStringId.AMP_STORY_SIDEBAR_BUTTON_LABEL,
-        },
         {
           tag: 'button',
           attrs: dict({
@@ -329,7 +316,6 @@ const VIEWER_CONTROL_DEFAULTS = {
  *   - story progress bar
  *   - share button
  *   - domain info button
- *   - sidebar
  *   - story updated label (for live stories)
  *   - close (for players)
  *   - skip (for players)
@@ -524,8 +510,6 @@ export class SystemLayer {
         this.onShareClick_(event);
       } else if (matches(target, `.${INFO_CLASS}, .${INFO_CLASS} *`)) {
         this.onInfoClick_();
-      } else if (matches(target, `.${SIDEBAR_CLASS}, .${SIDEBAR_CLASS} *`)) {
-        this.onSidebarClick_();
       } else if (
         matches(
           target,
@@ -641,14 +625,6 @@ export class SystemLayer {
       true /** callToInitialize */
     );
 
-    this.storeService_.subscribe(
-      StateProperty.HAS_SIDEBAR_STATE,
-      (hasSidebar) => {
-        this.onHasSidebarStateUpdate_(hasSidebar);
-      },
-      true /** callToInitialize */
-    );
-
     this.storeService_.subscribe(
       StateProperty.SYSTEM_UI_IS_VISIBLE_STATE,
       (isVisible) => {
@@ -695,20 +671,6 @@ export class SystemLayer {
       : this.getShadowRoot().removeAttribute(AD_SHOWING_ATTRIBUTE);
   }
 
-  /**
-   * Checks if the story has a sidebar in order to display the icon representing
-   * the opening of the sidebar.
-   * @param {boolean} hasSidebar
-   * @private
-   */
-  onHasSidebarStateUpdate_(hasSidebar) {
-    if (hasSidebar) {
-      this.getShadowRoot().setAttribute(HAS_SIDEBAR_ATTRIBUTE, '');
-    } else {
-      this.getShadowRoot().removeAttribute(HAS_SIDEBAR_ATTRIBUTE);
-    }
-  }
-
   /**
    * Reacts to updates to whether audio UI may be shown, and updates the UI
    * accordingly.
@@ -1012,14 +974,6 @@ export class SystemLayer {
     this.storeService_.dispatch(Action.TOGGLE_INFO_DIALOG, !isOpen);
   }
 
-  /**
-   * Handles click events on the sidebar button and toggles the sidebar.
-   * @private
-   */
-  onSidebarClick_() {
-    this.storeService_.dispatch(Action.TOGGLE_SIDEBAR, true);
-  }
-
   /**
    * Shows the "story updated" label when a new page was added to the story.
    * @private