Try adding autofocus in lightbox activate

ampproject · Apr 17, 2018 · 5112842 · 5112842
1 parent b33553b
commit 5112842
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 27 deletions.
diff --git a/extensions/amp-lightbox/0.1/amp-lightbox.js b/extensions/amp-lightbox/0.1/amp-lightbox.js
@@ -29,6 +29,7 @@ import {debounce} from '../../../src/utils/rate-limit';
 import {dev, user} from '../../../src/log';
 import {getMode} from '../../../src/mode';
 import {toArray} from '../../../src/types';
+import {tryFocus} from '../../../src/dom';
 
 /** @const {string} */
 const TAG = 'amp-lightbox';
@@ -189,36 +190,54 @@ class AmpLightbox extends AMP.BaseElement {
         .then(() => this.finalizeOpen_());
   }
 
+  /**
+   * Any child of the lightbox with the autofocus attribute should be focused
+   * after the lightbox opens.
+   * @private
+   */
+  handleAutofocus_() {
+    const autofocusElement = this.container_.querySelector('[autofocus]');
+    if (autofocusElement) {
+      tryFocus(autofocusElement);
+    }
+  }
+
+  /**
+   * @private
+   */
   finalizeOpen_() {
     if (this.isScrollable_) {
       st.setStyle(this.element, 'webkitOverflowScrolling', 'touch');
     }
-    this.mutateElement(() => {
-      st.setStyles(this.element, {
-        display: '',
-        opacity: 0,
-        // TODO(dvoytenko): use new animations support instead.
-        transition: 'opacity 0.1s ease-in',
-      });
-      Services.vsyncFor(this.win).mutate(() => {
-        st.setStyle(this.element, 'opacity', '');
-      });
-    }).then(() => {
-      const container = dev().assertElement(this.container_);
-      if (!this.isScrollable_) {
-        this.updateInViewport(container, true);
-      } else {
-        this.scrollHandler_();
-        this.updateChildrenInViewport_(this.pos_, this.pos_);
-      }
-      // TODO: instead of laying out children all at once, layout children based
-      // on visibility.
-      this.element.addEventListener('transitionend', this.boundReschedule_);
-      this.element.addEventListener('animationend', this.boundReschedule_);
-      this.scheduleLayout(container);
-      this.scheduleResume(container);
-      this.triggerEvent_(LightboxEvents.OPEN);
+
+    // This should be in a mutateElement block, but iOS focus doesn't work
+    // if triggered asynchronously.
+    st.setStyles(this.element, {
+      display: '',
+      opacity: 0,
+      // TODO(dvoytenko): use new animations support instead.
+      transition: 'opacity 0.1s ease-in',
     });
+    Services.vsyncFor(this.win).mutate(() => {
+      st.setStyle(this.element, 'opacity', '');
+    });
+
+    this.handleAutofocus_();
+
+    const container = dev().assertElement(this.container_);
+    if (!this.isScrollable_) {
+      this.updateInViewport(container, true);
+    } else {
+      this.scrollHandler_();
+      this.updateChildrenInViewport_(this.pos_, this.pos_);
+    }
+    // TODO: instead of laying out children all at once, layout children based
+    // on visibility.
+    this.element.addEventListener('transitionend', this.boundReschedule_);
+    this.element.addEventListener('animationend', this.boundReschedule_);
+    this.scheduleLayout(container);
+    this.scheduleResume(container);
+    this.triggerEvent_(LightboxEvents.OPEN);
 
     this.getHistory_().push(this.close.bind(this)).then(historyId => {
       this.historyId_ = historyId;

diff --git a/test/manual/amp-lightbox-focus.amp.html b/test/manual/amp-lightbox-focus.amp.html
@@ -43,12 +43,12 @@
   </style>
 </head>
 <body>
-  <amp-lightbox id="my-lightbox" layout="nodisplay" on="lightboxOpen:input.focus">
+  <amp-lightbox id="my-lightbox" layout="nodisplay">
     <div class="lightbox" role="button" tabindex="0">
       <button id="close-button" on="tap:my-lightbox.close" role="button" tabindex="0">
         Close lightbox
       </button>
-      <input id="input" type="text">
+      <input id="input" type="text" autofocus>
 
     </div>
   </amp-lightbox>