Update sidebar v0.2 + nested menu and remove history API

build-system/compile/bundles.config.js

@@ -723,7 +723,7 @@ exports.extensionBundles = [
     type: TYPES.MISC,
   },
   {
-    name: 'amp-drilldown',
+    name: 'amp-nested-menu',
     version: '0.1',
     latestVersion: '0.1',
     options: {hasCss: true},
@@ -801,7 +801,7 @@ exports.extensionBundles = [
   },
   {
     name: 'amp-sidebar',
-    version: ['0.1', '1.0'],
+    version: ['0.1', '0.2'],
     latestVersion: '0.1',
     options: {hasCss: true},
     type: TYPES.MISC,

extensions/amp-drilldown/0.1/amp-drilldown.css → extensions/amp-nested-menu/0.1/amp-nested-menu.css

@@ -14,35 +14,35 @@
  * limitations under the License.
  */
 
-amp-drilldown,
-amp-drilldown [amp-drilldown-submenu] {
+amp-nested-menu,
+amp-nested-menu [amp-nested-submenu] {
   position: absolute !important;
   top: 0 !important;
   width: 100% !important;
   height: 100vh !important;
   transform: translateX(0) !important;
 }
 
-amp-drilldown.i-amphtml-layout-size-defined {
+amp-nested-menu.i-amphtml-layout-size-defined {
   overflow: visible !important;
 }
 
-amp-drilldown [amp-drilldown-submenu] {
+amp-nested-menu [amp-nested-submenu] {
   left: 100% !important;
   opacity: 0 !important;
   visibility: hidden !important;
   transition: transform 233ms, opacity 233ms, visibility 0s 233ms;
 }
 
-amp-drilldown,
-amp-drilldown [amp-drilldown-submenu][open] {
+amp-nested-menu,
+amp-nested-menu [amp-nested-submenu][open] {
   opacity: 1 !important;
   visibility: visible !important;
   transition: transform 233ms;
 }
 
-amp-drilldown[child-open],
-amp-drilldown [amp-drilldown-submenu][child-open] {
+amp-nested-menu[child-open],
+amp-nested-menu [amp-nested-submenu][child-open] {
   transform: translateX(-100%) !important;
   visibility: hidden !important;
   transition: transform 233ms, visibility 0s 233ms;

extensions/amp-drilldown/0.1/amp-drilldown.js → extensions/amp-nested-menu/0.1/amp-nested-menu.js

@@ -14,18 +14,25 @@
  * limitations under the License.
  */
 
-import {CSS} from '../../../build/amp-drilldown-0.1.css';
+import {CSS} from '../../../build/amp-nested-menu-0.1.css';
 import {Keys} from '../../../src/utils/key-codes';
 import {Layout} from '../../../src/layout';
 import {Services} from '../../../src/services';
-import {closestAncestorElementBySelector} from '../../../src/dom';
+import {
+  closestAncestorElementBySelector,
+  scopedQuerySelector,
+  tryFocus,
+} from '../../../src/dom';
 import {dev, userAssert} from '../../../src/log';
 import {isExperimentOn} from '../../../src/experiments';
+import {toArray} from '../../../src/types';
 
-const TAG = 'amp-drilldown';
+const TAG = 'amp-nested-menu';
 
-// TODO(#24668): add unit tests for click handlers and history.
-export class AmpDrilldown extends AMP.BaseElement {
+/** @private @const {number} */
+const ANIMATION_TIMEOUT = 350;
+
+export class AmpNestedMenu extends AMP.BaseElement {
   /** @param {!AmpElement} element */
   constructor(element) {
     super(element);
@@ -36,12 +43,21 @@ export class AmpDrilldown extends AMP.BaseElement {
     /** @private @const {!Element} */
     this.documentElement_ = this.document_.documentElement;
 
-    /** @private {Array} */
-    this.historyIds_ = [];
+    /** @private {?../../../src/service/action-impl.ActionService} */
+    this.action_ = null;
 
     /** @private {?Element} */
     this.currentSubmenu_ = null;
 
+    /** @private {?Array<!Element>} */
+    this.submenuElements_ = null;
+
+    /** @private {?Array<!Element>} */
+    this.submenuOpenElements_ = null;
+
+    /** @private {?Array<!Element>} */
+    this.submenuCloseElements_ = null;
+
     /** @private {function(!Event)} */
     this.submenuOpenHandler_ = this.handleSubmenuOpenClick_.bind(this);
 
@@ -54,10 +70,39 @@ export class AmpDrilldown extends AMP.BaseElement {
 
   /** @override */
   buildCallback() {
+    // TODO(#25022): remove this assert when cleaning up experiment post launch.
     userAssert(
       isExperimentOn(this.win, 'amp-sidebar-v2'),
-      'Turning on the amp-sidebar-v2 experiment is necessary to use the amp-drilldown component.'
+      `Turning on the amp-sidebar-v2 experiment is necessary to use the ${TAG} component.`
+    );
+
+    this.action_ = Services.actionServiceForDoc(this.element);
+
+    this.submenuElements_ = toArray(
+      this.element.querySelectorAll('[amp-nested-submenu]')
+    );
+    this.submenuElements_.forEach(submenu => {
+      // Make submenus programmatically focusable for a11y.
+      submenu.tabIndex = -1;
+    });
+    this.submenuOpenElements_ = toArray(
+      this.element.querySelectorAll('[amp-nested-submenu-open]')
+    );
+    this.submenuCloseElements_ = toArray(
+      this.element.querySelectorAll('[amp-nested-submenu-close]')
     );
+    this.submenuOpenElements_
+      .concat(this.submenuCloseElements_)
+      .forEach(element => {
+        if (!element.hasAttribute('tabindex')) {
+          element.setAttribute('tabindex', 0);
+        }
+        element.setAttribute('role', 'button');
+        userAssert(
+          this.action_.hasAction(element, 'tap') == false,
+          'submenu open/close buttons should not have tap actions registered.'
+        );
+      });
   }
 
   /** @override */
@@ -81,16 +126,10 @@ export class AmpDrilldown extends AMP.BaseElement {
    * Add event listeners on submenu open/close elements.
    */
   registerEventListeners_() {
-    const openElements = this.element.querySelectorAll(
-      '[amp-drilldown-submenu-open]'
-    );
-    openElements.forEach(element => {
+    this.submenuOpenElements_.forEach(element => {
       element.addEventListener('click', this.submenuOpenHandler_);
     });
-    const closeElements = this.element.querySelectorAll(
-      '[amp-drilldown-submenu-close]'
-    );
-    closeElements.forEach(element => {
+    this.submenuCloseElements_.forEach(element => {
       element.addEventListener('click', this.submenuCloseHandler_);
     });
 
@@ -101,16 +140,10 @@ export class AmpDrilldown extends AMP.BaseElement {
    * Remove listeners on all submenu open/close elements.
    */
   unregisterEventListeners_() {
-    const openElements = this.element.querySelectorAll(
-      '[amp-drilldown-submenu-open]'
-    );
-    openElements.forEach(element => {
+    this.submenuOpenElements_.forEach(element => {
       element.removeEventListener('click', this.submenuOpenHandler_);
     });
-    const closeElements = this.element.querySelectorAll(
-      '[amp-drilldown-submenu-close]'
-    );
-    closeElements.forEach(element => {
+    this.submenuCloseElements_.forEach(element => {
       element.removeEventListener('click', this.submenuCloseHandler_);
     });
 
@@ -122,10 +155,8 @@ export class AmpDrilldown extends AMP.BaseElement {
    * @param {Event} e
    */
   handleSubmenuOpenClick_(e) {
-    const submenuOpen = dev().assertElement(e.target);
-    const submenu = submenuOpen.parentElement.querySelector(
-      '[amp-drilldown-submenu]'
-    );
+    const submenuParent = dev().assertElement(e.currentTarget.parentElement);
+    const submenu = scopedQuerySelector(submenuParent, '>[amp-nested-submenu]');
     if (submenu) {
       this.open_(submenu);
     }
@@ -139,16 +170,18 @@ export class AmpDrilldown extends AMP.BaseElement {
     if (submenu.hasAttribute('open')) {
       return;
     }
-    const submenuParent = this.getParentMenu_(submenu);
-    if (submenuParent) {
-      submenuParent.setAttribute('child-open', '');
+    const parentMenu = this.getParentMenu_(submenu);
+    if (parentMenu) {
+      parentMenu.setAttribute('child-open', '');
       submenu.setAttribute('open', '');
-      this.getHistory_()
-        .push(() => this.close_(submenu))
-        .then(historyId => {
-          this.historyIds_.push(historyId);
-        });
       this.currentSubmenu_ = submenu;
+      // move focus to close element after submenu fully opens.
+      Services.timerFor(this.win).delay(() => {
+        const submenuClose = dev().assertElement(
+          submenu.querySelector('[amp-nested-submenu-close]')
+        );
+        tryFocus(submenuClose);
+      }, ANIMATION_TIMEOUT);
     }
   }
 
@@ -157,10 +190,10 @@ export class AmpDrilldown extends AMP.BaseElement {
    * @param {Event} e
    */
   handleSubmenuCloseClick_(e) {
-    const submenuClose = dev().assertElement(e.target);
+    const submenuClose = dev().assertElement(e.currentTarget);
     const submenu = closestAncestorElementBySelector(
       submenuClose,
-      '[amp-drilldown-submenu]'
+      '[amp-nested-submenu]'
     );
     if (submenu) {
       this.close_(submenu);
@@ -175,15 +208,19 @@ export class AmpDrilldown extends AMP.BaseElement {
     if (!submenu.hasAttribute('open')) {
       return;
     }
-    const submenuParent = this.getParentMenu_(submenu);
-    if (submenuParent) {
-      submenuParent.removeAttribute('child-open');
+    const parentMenu = this.getParentMenu_(submenu);
+    if (parentMenu) {
+      parentMenu.removeAttribute('child-open');
       submenu.removeAttribute('open');
-      if (this.historyIds_.length > 0) {
-        const lastHistoryId = this.historyIds_.pop();
-        this.getHistory_().pop(lastHistoryId);
-      }
-      this.currentSubmenu_ = submenuParent;
+      this.currentSubmenu_ = parentMenu;
+      // move focus back to open element after submenu fully closes.
+      Services.timerFor(this.win).delay(() => {
+        const submenuParent = dev().assertElement(submenu.parentElement);
+        const submenuOpen = dev().assertElement(
+          scopedQuerySelector(submenuParent, '>[amp-nested-submenu-open]')
+        );
+        tryFocus(submenuOpen);
+      }, ANIMATION_TIMEOUT);
     }
   }
 
@@ -217,20 +254,11 @@ export class AmpDrilldown extends AMP.BaseElement {
   getParentMenu_(submenu) {
     return closestAncestorElementBySelector(
       dev().assertElement(submenu.parentElement),
-      'amp-drilldown,[amp-drilldown-submenu]'
+      'amp-nested-menu,[amp-nested-submenu]'
     );
   }
-
-  /**
-   * Returns the history for the ampdoc.
-   * @return {!../../../src/service/history-impl.History}
-   * @private
-   */
-  getHistory_() {
-    return Services.historyForDoc(this.getAmpDoc());
-  }
 }
 
 AMP.extension(TAG, '0.1', AMP => {
-  AMP.registerElement(TAG, AmpDrilldown, CSS);
+  AMP.registerElement(TAG, AmpNestedMenu, CSS);
 });