Merge pull request #102 from mobify/release-v2.0.4

Update develop with legacy a11y release
mobify · Oct 6, 2016 · 683dce5 · 683dce5
2 parents 59f81ab + 5177700
commit 683dce5
Show file tree

Hide file tree

Showing 11 changed files with 91 additions and 42 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,10 +1,15 @@
+3.0.1
+    - Bring forward a11y fixes from 2.0.4 ([#99](https://github.com/mobify/pinny/pull/99))
 3.0.0
     - Migrate from Zepto to jQuery
     - Migrate from Mobify's fork of Velocity.js to current Velocity.js release
     - Migrate Bower dependencies to NPM
     - Migrate JS linting to current standards (eslint)
     - Migrate SCSS compilation to current standards (libsass)
     - Add SCSS linting
+2.0.4
+    - Fix a11y issue where lockup's aria hidden attr was causing all pinnies inside to be inaccessible for screenreaders
+    - Changes new default container to <body> instead of lockup container
 2.0.3
     - Fire lock and unlock events even if not locking/unlocking due to presence
       of activePinnies

diff --git a/README.md b/README.md
@@ -76,7 +76,7 @@ Pinny requires very minimal markup. All Pinny needs is a div with your content a
 
 > To avoid any unwanted FOUT, decorate the content you will be passing to Pinny with the `hidden` attribute. The `hidden` attribute will be removed when Pinny is initialized.
 
-For accessibility and functional purposes, Pinny will wrap all of your body content in a wrapping container. This could conflict with other plugins that alter your page's markup. If you're seeing issues, try initializing Pinny after your other plugins. If you want to specify your own wrapping container, add a class of `lockup__container` to the element. This element should be a root level element to be effective. You can also [pass Pinny a `container` option](#pinnyoptions).
+For accessibility and functional purposes, Pinny will wrap all of your body content in a wrapping container. This could conflict with other plugins that alter your page's markup. If you're seeing issues, try initializing Pinny after your other plugins. If you want to specify your own wrapping container, you can [pass Pinny a `container` option](https://github.com/mobify/pinny#container).
 
 ```html
 <!-- Include the CSS -->
@@ -185,7 +185,7 @@ $('#myPinny').pinny({
 
 #### container
 
-default: `$container` (lockup's container)
+default: `<body>`
 
 Specify the container Pinny will be created within
 
@@ -227,7 +227,7 @@ If you are using `structure: false`, you will need to structure your HTML to inc
 ```html
 <div id="myPinny" class="pinny__wrapper" role="document" hidden>
     <div class="pinny__header">
-    	<a class="pinny__close">close</a>
+    	<button type="button" class="pinny__close">close</button>
     </div>
     <div class="pinny__content pinny--is-scrollable"></div>
     <div class="pinny__footer"></div>

diff --git a/dist/effect/modal-center.min.js b/dist/effect/modal-center.min.js
diff --git a/dist/effect/sheet-bottom.min.js b/dist/effect/sheet-bottom.min.js
diff --git a/dist/effect/sheet-left.min.js b/dist/effect/sheet-left.min.js
diff --git a/dist/effect/sheet-right.min.js b/dist/effect/sheet-right.min.js
diff --git a/dist/effect/sheet-top.min.js b/dist/effect/sheet-top.min.js
diff --git a/dist/pinny.js b/dist/pinny.js
@@ -74,7 +74,7 @@
         Pinny.__super__.call(this, element, options, Pinny.DEFAULTS);
     };
 
-    Pinny.VERSION = '2.0.3';
+    Pinny.VERSION = '3.0.0';
 
     Pinny.DEFAULTS = {
         effect: null,
@@ -95,7 +95,8 @@
         close: $.noop,
         closed: $.noop,
         scrollDuration: 50,
-        spacerHeight: 300
+        spacerHeight: 300,
+        pageContentSelector: '#x-root'
     };
 
     Plugin.create('pinny', Pinny, {
@@ -105,9 +106,9 @@
         animation: {
             openComplete: function() {
                 setTimeout(function() {
-
-                    this._disableExternalInputs();
                     this._focus();
+                    this._disableExternalInputs();
+
                     // Only run lockup if another pinny isn't open and has
                     // locked up the viewport already
                     if (this._activePinnies()) {
@@ -123,6 +124,10 @@
                         .addClass(classes.OPENED)
                         .attr('aria-hidden', 'false');
 
+                    // Lockup a11y fix
+                    // Instead of making whole lockup container aria hidden,
+                    // just make page content aria hidden, since otherwise pinny
+                    // cannot be read by screenreaders
                     this.$container.attr('aria-hidden', 'true');
 
                     this._trigger('opened');
@@ -135,9 +140,6 @@
                         .removeClass(classes.OPENED)
                         .attr('aria-hidden', 'true');
 
-                    this._enableExternalInputs();
-                    this._resetFocus();
-
                     // only unlock if there isn't another pinny
                     // that requires the viewport to be locked
                     if (this._activePinnies()) {
@@ -148,8 +150,15 @@
                         $window.off(events.orientationchange);
                     }
 
+                    // Lockup a11y fix
+                    // Instead of making whole lockup container aria hidden,
+                    // just make page content aria hidden, since otherwise pinny
+                    // cannot be read by screenreaders
                     this.$container.attr('aria-hidden', 'false');
 
+                    this._resetFocus();
+                    this._enableExternalInputs();
+
                     this._trigger('closed');
 
                 }.bind(this), 0);
@@ -199,6 +208,9 @@
 
             this._trigger('open');
 
+            // Record active element so that it may be re-focused when pinny is closed
+            this.originalActiveElement = document.activeElement;
+
             bouncefix.add(classes.SCROLLABLE);
 
             this.options.shade && this.$shade.shade('open');
@@ -240,9 +252,9 @@
          Builds Pinny using the following structure:
 
          <section class="pinny">
-         <div class="pinny__wrapper">
+         <div class="pinny__wrapper" tabindex="0">
          <header class="pinny__header">{header content}</header>
-         <div class="pinny__content">
+         <div class="pinny__content" tabindex="0">
          {content}
          </div>
          <footer class="pinny__footer">{footer content}</footer>
@@ -275,20 +287,29 @@
                     }
                 });
 
-            this.$container = this.$pinny.data('lockup').$container;
+            this.$container;
+            if ($(this.options.pageContentSelector).length) {
+                this.$container = $(this.options.pageContentSelector);
+            } else {
+                this.$container = this.$pinny.data('lockup').$container;
+            }
 
-            this.$pinny.appendTo(this.options.appendTo ? $(this.options.appendTo) : this.$container);
+            // Append to document.body by default, so that pinnies will be
+            // outside the lockup container, which can be hidden to screenreaders
+            this.$pinny.appendTo(this.options.appendTo ? $(this.options.appendTo) : this.$body);
 
             if (this.options.structure) {
                 var $wrapper = $('<div />')
                     .addClass(classes.WRAPPER)
+                    .attr('tabindex', '0')
                     .appendTo(this.$pinny);
 
                 this._buildComponent('header').appendTo($wrapper);
 
                 $('<div />')
                     .addClass(classes.CONTENT)
                     .addClass(classes.SCROLLABLE)
+                    .attr('tabindex', '0')
                     .append(this.$element)
                     .appendTo($wrapper);
 
@@ -372,8 +393,11 @@
             var $header = this.$pinny.find('h1, .' + classes.TITLE).first();
             var $wrapper = this.$pinny.find('.' + classes.WRAPPER);
 
-            this.$container
-                .attr('aria-hidden', 'false');
+            // Lockup a11y fix
+            // Instead of making whole lockup container aria hidden,
+            // just make page content aria hidden, since otherwise pinny
+            // cannot be read by screenreaders
+            this.$container.attr('aria-hidden', 'false');
 
             this.$pinny
                 .attr('role', 'dialog')
@@ -393,9 +417,7 @@
         },
 
         _focus: function() {
-            this.originalActiveElement = document.activeElement;
-
-            this.$pinny.children().first().focus();
+            this.$pinny.find('.' + classes.WRAPPER).focus();
         },
 
         _resetFocus: function() {