Repository: braintree/braintree-web-drop-in
Pull request #722 (2021-04-19 21:29:48)

Accessibility updates
### Summary

Improves keyboard navigation

### Checklist

- [X] Added a changelog entry

### Authors
> List GitHub usernames for everyone who contributed to this pull request.
- @crookedneighbor 
- @jplukarski 



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 41b706d..e8b3855 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,7 @@ unreleased
 - Update browser-detection to v1.11.1
 - Fix issue where card view would not render in a shadow DOM within a shadow DOM
 - Fix issue where Venmo would setup in Firefox for iOS with `allowNewBrowserTab: false` set
+- Improve keyboard navigation
 
 1.27.0
 ------
diff --git a/src/html/main.html b/src/html/main.html
index 36832dc..11d5e88 100644
--- a/src/html/main.html
+++ b/src/html/main.html
@@ -1,6 +1,6 @@
 <div class="braintree-dropin">
   <div data-braintree-id="methods-label" class="braintree-heading">&nbsp;</div>
-  <div data-braintree-id="methods-edit" class="braintree-hidden braintree-heading">{{edit}}</div>
+  <div data-braintree-id="methods-edit" class="braintree-hidden braintree-heading" role="button" tabindex="0">{{edit}}</div>
   <div data-braintree-id="choose-a-way-to-pay" class="braintree-heading">{{chooseAWayToPay}}</div>
   <div class="braintree-placeholder">&nbsp;</div>
 
@@ -16,8 +16,8 @@
     <div data-braintree-id="delete-confirmation" class="braintree-delete-confirmation braintree-sheet">
       <div data-braintree-id="delete-confirmation__message"></div>
       <div class="braintree-delete-confirmation__button-container">
-        <div role="button" data-braintree-id="delete-confirmation__no" class="braintree-delete-confirmation__button">{{deleteCancelButton}}</div>
-        <div role="button" data-braintree-id="delete-confirmation__yes" class="braintree-delete-confirmation__button">{{deleteConfirmationButton}}</div>
+        <div tabindex="0" role="button" data-braintree-id="delete-confirmation__no" class="braintree-delete-confirmation__button">{{deleteCancelButton}}</div>
+        <div tabindex="0" role="button" data-braintree-id="delete-confirmation__yes" class="braintree-delete-confirmation__button">{{deleteConfirmationButton}}</div>
       </div>
     </div>
 
diff --git a/src/views/delete-confirmation-view.js b/src/views/delete-confirmation-view.js
index 1f5da2d..d83351f 100644
--- a/src/views/delete-confirmation-view.js
+++ b/src/views/delete-confirmation-view.js
@@ -58,4 +58,10 @@ DeleteConfirmationView.prototype.applyPaymentMethod = function (paymentMethod) {
   this._messageBox.innerText = messageText;
 };
 
+DeleteConfirmationView.prototype.onSelection = function () {
+  window.requestAnimationFrame(function () {
+    this._yesButton.focus();
+  }.bind(this));
+};
+
 module.exports = DeleteConfirmationView;
diff --git a/src/views/payment-method-view.js b/src/views/payment-method-view.js
index 6588cd3..65aa7ce 100644
--- a/src/views/payment-method-view.js
+++ b/src/views/payment-method-view.js
@@ -27,8 +27,15 @@ PaymentMethodView.prototype._initialize = function () {
   this.element = document.createElement('div');
   this.element.className = 'braintree-method';
   this.element.setAttribute('tabindex', '0');
+  this.element.setAttribute('role', 'button');
 
-  addSelectionEventHandler(this.element, this._choosePaymentMethod.bind(this));
+  addSelectionEventHandler(this.element, function () {
+    if (this.model.isInEditMode()) {
+      this._selectDelete();
+    } else {
+      this._choosePaymentMethod();
+    }
+  }.bind(this));
 
   switch (this.paymentMethod.type) {
     case paymentMethodTypes.applePay:
@@ -68,7 +75,6 @@ PaymentMethodView.prototype._initialize = function () {
 
   this.element.innerHTML = html;
   this.checkMark = this.element.querySelector('.braintree-method__check-container');
-  addSelectionEventHandler(this.element.querySelector('.braintree-method__delete-container'), this._selectDelete.bind(this));
 };
 
 PaymentMethodView.prototype.setActive = function (isActive) {
@@ -90,10 +96,13 @@ PaymentMethodView.prototype.disableEditMode = function () {
   classList.remove(this.element, 'braintree-method--disabled');
 };
 
-PaymentMethodView.prototype._choosePaymentMethod = function () {
-  if (this.model.isInEditMode()) {
-    return;
+PaymentMethodView.prototype.teardown = function () {
+  if (this.element.parentNode) {
+    this.element.parentNode.removeChild(this.element);
   }
+};
+
+PaymentMethodView.prototype._choosePaymentMethod = function () {
   if (this.paymentMethod.vaulted) {
     analytics.sendEvent(this.client, 'vaulted-' + constants.analyticsKinds[this.paymentMethod.type] + '.select');
   }
diff --git a/src/views/payment-methods-view.js b/src/views/payment-methods-view.js
index f37accf..a6adc4b 100644
--- a/src/views/payment-methods-view.js
+++ b/src/views/payment-methods-view.js
@@ -106,7 +106,7 @@ PaymentMethodsView.prototype._addPaymentMethod = function (paymentMethod) {
   });
 
   if (this.model.isGuestCheckout && this.container.firstChild) {
-    this.container.removeChild(this.container.firstChild);
+    this.views[0].teardown();
     this.views.pop();
   }
 
@@ -124,7 +124,7 @@ PaymentMethodsView.prototype._removePaymentMethod = function (paymentMethod) {
 
   for (i = 0; i < this.views.length; i++) {
     if (this.views[i].paymentMethod === paymentMethod) {
-      this.container.removeChild(this.views[i].element);
+      this.views[i].teardown();
       this._headingLabel.innerHTML = '&nbsp;';
       this.views.splice(i, 1);
       break;
@@ -164,8 +164,8 @@ PaymentMethodsView.prototype.refreshPaymentMethods = function () {
   var paymentMethods = this.model.getPaymentMethods();
 
   this.views.forEach(function (view) {
-    this.container.removeChild(view.element);
-  }.bind(this));
+    view.teardown();
+  });
 
   this.views = [];
 


Test patch:
diff --git a/.github/workflows/ci-integration-tests.yml b/.github/workflows/ci-integration-tests.yml
index a1d9917..b59207c 100644
--- a/.github/workflows/ci-integration-tests.yml
+++ b/.github/workflows/ci-integration-tests.yml
@@ -46,10 +46,10 @@ jobs:
         with:
           local-testing: stop
 
-  paypal-on-sensible-browsers:
+  paypal-on-firefox:
     if: always()
     needs: non-paypal-on-sensible-browsers
-    name: "PayPal Tests on Chrome, Firefox and Safari"
+    name: "PayPal Tests on Firefox"
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
@@ -75,11 +75,88 @@ jobs:
           rm -rf node_modules/@types/mocha
           sudo echo "127.0.0.1 bs-local.com" | sudo tee -a /etc/hosts
           npm run development & echo "starting integration app"
-      - name: "PayPal tests on Chrome, Safari and Firefox"
+      - name: "PayPal tests on Firefox"
         env:
-          ONLY_BROWSERS: chrome,safari,firefox
+          ONLY_BROWSERS: firefox
         run: npm run test:integration:paypal-only
       - name: "BrowserStackLocal Stop"
         uses: "browserstack/github-actions/setup-local@master"
         with:
           local-testing: stop
+
+  paypal-on-chrome:
+    if: always()
+    needs: paypal-on-firefox
+    name: "PayPal Tests on Chrome"
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: "BrowserStack Env Setup"
+        uses: "browserstack/github-actions/setup-env@master"
+        with:
+          username:  ${{ secrets.BROWSERSTACK_USERNAME }}
+          access-key: ${{ secrets.BROWSERSTACK_ACCESS_KEY }}
+          build-name: BUILD_INFO
+          project-name: REPO_NAME
+      - name: "BrowserStackLocal Setup"
+        uses: "browserstack/github-actions/setup-local@master"
+        with:
+          local-testing: start
+          local-identifier: random
+      - name: "Node Setup"
+        uses: "actions/setup-node@v1"
+        with:
+          node-version: "12.x"
+      - name: "Demo App Setup"
+        run: |
+          npm install
+          rm -rf node_modules/@types/mocha
+          sudo echo "127.0.0.1 bs-local.com" | sudo tee -a /etc/hosts
+          npm run development & echo "starting integration app"
+      - name: "PayPal tests on Chrome"
+        env:
+          ONLY_BROWSERS: chrome
+        run: npm run test:integration:paypal-only
+      - name: "BrowserStackLocal Stop"
+        uses: "browserstack/github-actions/setup-local@master"
+        with:
+          local-testing: stop
+
+  # safari is struggling to open the PayPal popup for some reason
+  # paypal-on-safari:
+  #   if: always()
+  #   needs: paypal-on-chrome
+  #   name: "PayPal Tests on Safari"
+  #   runs-on: ubuntu-latest
+  #   steps:
+  #     - uses: actions/checkout@v2
+  #     - name: "BrowserStack Env Setup"
+  #       uses: "browserstack/github-actions/setup-env@master"
+  #       with:
+  #         username:  ${{ secrets.BROWSERSTACK_USERNAME }}
+  #         access-key: ${{ secrets.BROWSERSTACK_ACCESS_KEY }}
+  #         build-name: BUILD_INFO
+  #         project-name: REPO_NAME
+  #     - name: "BrowserStackLocal Setup"
+  #       uses: "browserstack/github-actions/setup-local@master"
+  #       with:
+  #         local-testing: start
+  #         local-identifier: random
+  #     - name: "Node Setup"
+  #       uses: "actions/setup-node@v1"
+  #       with:
+  #         node-version: "12.x"
+  #     - name: "Demo App Setup"
+  #       run: |
+  #         npm install
+  #         rm -rf node_modules/@types/mocha
+  #         sudo echo "127.0.0.1 bs-local.com" | sudo tee -a /etc/hosts
+  #         npm run development & echo "starting integration app"
+  #     - name: "PayPal tests on Safari"
+  #       env:
+  #         ONLY_BROWSERS: safari
+  #       run: npm run test:integration:paypal-only
+  #     - name: "BrowserStackLocal Stop"
+  #       uses: "browserstack/github-actions/setup-local@master"
+  #       with:
+  #         local-testing: stop
diff --git a/test/integration/helper.js b/test/integration/helper.js
index 1aa4c4b..47a961f 100644
--- a/test/integration/helper.js
+++ b/test/integration/helper.js
@@ -177,16 +177,34 @@ module.exports = function createHelpers() {
     browser.waitForElementToDissapear('.paypal-checkout-sandbox-iframe');
   });
 
-  browser.addCommand('confirmButtonIsEnabled', function () {
-    return ($('#payment-submit-btn').isDisplayed() && $('#payment-submit-btn').isEnabled()) ||
-      ($('#fiSubmitButton').isDisplayed() && $('#fiSubmitButton').isEnabled()) ||
-      ($('#consentButton').isDisplayed() && $('#consentButton').isEnabled()) ||
-      ($('#confirmButtonTop').isDisplayed() && $('#confirmButtonTop').isEnabled());
+  browser.addCommand('getPayPalConfirmButtonSelector', function () {
+    browser.waitUntil(() => {
+      return $('#fiSubmitButton').isDisplayed() ||
+        $('#consentButton').isDisplayed() ||
+        $('#payment-submit-btn').isDisplayed() ||
+        $('#confirmButtonTop').isDisplayed();
+    }, {
+      timeout: PAYPAL_TIMEOUT
+    });
+
+    if ($('#fiSubmitButton').isDisplayed()) {
+      return '#fiSubmitButton';
+    } else if ($('#consentButton').isDisplayed()) {
+      return '#consentButton';
+    } else if ($('#payment-submit-btn').isDisplayed()) {
+      return '#payment-submit-btn';
+    } else if ($('#confirmButtonTop').isDisplayed()) {
+      return '#confirmButtonTop';
+    }
+
+    throw new Error('Could not find confirm payment button');
   });
 
   browser.addCommand('waitForConfirmButtonEnabled', function () {
     browser.waitUntil(() => {
-      return browser.confirmButtonIsEnabled();
+      const selector = browser.getPayPalConfirmButtonSelector();
+
+      return $(selector).isEnabled();
     }, {
       timeout: PAYPAL_TIMEOUT
     });
@@ -203,15 +221,10 @@ module.exports = function createHelpers() {
 
     browser.waitForConfirmButtonEnabled();
 
-    if ($('#fiSubmitButton').isDisplayed()) {
-      $('#fiSubmitButton').click();
-    } else if ($('#consentButton').isDisplayed()) {
-      $('#consentButton').click();
-    } else if ($('#payment-submit-btn').isDisplayed()) {
-      $('#payment-submit-btn').click();
-    } else if ($('#confirmButtonTop').isDisplayed()) {
-      $('#confirmButtonTop').click();
-    }
+    const selector = browser.getPayPalConfirmButtonSelector();
+
+    $(selector).scrollIntoView();
+    $(selector).click();
   });
 
   browser.addCommand('waitForElementToDissapear', function (selector) {
diff --git a/test/unit/views/delete-confirmation-view.js b/test/unit/views/delete-confirmation-view.js
index 86453b2..3f91a35 100644
--- a/test/unit/views/delete-confirmation-view.js
+++ b/test/unit/views/delete-confirmation-view.js
@@ -123,4 +123,17 @@ describe('DeleteConfirmationView', () => {
       }
     );
   });
+
+  describe('onSelection', () => {
+    test('focuses the yes button', async () => {
+      jest.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => {
+        cb();
+      });
+      const focusSpy = jest.spyOn(testContext.view.getElementById('delete-confirmation__yes'), 'focus');
+
+      testContext.view.onSelection();
+
+      expect(focusSpy).toBeCalledTimes(1);
+    });
+  });
 });
diff --git a/test/unit/views/payment-method-view.js b/test/unit/views/payment-method-view.js
index 5a5b47a..1572f9d 100644
--- a/test/unit/views/payment-method-view.js
+++ b/test/unit/views/payment-method-view.js
@@ -2,8 +2,11 @@ const analytics = require('../../../src/lib/analytics');
 const BaseView = require('../../../src/views/base-view');
 const fake = require('../../helpers/fake');
 const PaymentMethodView = require('../../../src/views/payment-method-view');
+const addSelectionEventHandler = require('../../../src/lib/add-selection-event-handler');
 const strings = require('../../../src/translations/en_US');
 
+jest.mock('../../../src/lib/add-selection-event-handler');
+
 describe('PaymentMethodView', () => {
   let config;
 
@@ -141,6 +144,36 @@ describe('PaymentMethodView', () => {
         expect(iconContainer.classList.contains('braintree-method__logo@CLASSNAME')).toBe(false);
       }
     );
+
+    test('calls model.confirmPaymentMethodDeletion when selection event occurs when in edit mode', () => {
+      config.model = {
+        isInEditMode: jest.fn().mockReturnValue(true),
+        confirmPaymentMethodDeletion: jest.fn()
+      };
+      // eslint-disable-next-line no-unused-vars
+      const view = new PaymentMethodView(config);
+      const handler = addSelectionEventHandler.mock.calls[0][1];
+
+      handler();
+
+      expect(config.model.confirmPaymentMethodDeletion).toBeCalledTimes(1);
+      expect(config.model.confirmPaymentMethodDeletion).toBeCalledWith(config.paymentMethod);
+    });
+
+    test('calls model.changeActivePaymentMethod when selection occurs when not in edit mode', () => {
+      config.model = {
+        isInEditMode: jest.fn().mockReturnValue(false),
+        changeActivePaymentMethod: jest.fn()
+      };
+      // eslint-disable-next-line no-unused-vars
+      const view = new PaymentMethodView(config);
+      const handler = addSelectionEventHandler.mock.calls[0][1];
+
+      handler();
+
+      expect(config.model.changeActivePaymentMethod).toBeCalledTimes(1);
+      expect(config.model.changeActivePaymentMethod).toBeCalledWith(config.paymentMethod);
+    });
   });
 
   describe('setActive', () => {
@@ -284,55 +317,25 @@ describe('PaymentMethodView', () => {
     );
   });
 
-  describe('edit mode', () => {
-    test(
-      'does not call model.changeActivePaymentMethod in click handler when in edit mode',
-      () => {
-        const model = fake.model();
-        const view = new PaymentMethodView({
-          model: model,
-          strings: strings,
-          paymentMethod: {
-            type: 'Foo',
-            nonce: 'nonce'
-          }
-        });
-
-        jest.spyOn(model, 'changeActivePaymentMethod').mockImplementation();
-        jest.spyOn(model, 'isInEditMode').mockReturnValue(true);
-
-        view._choosePaymentMethod();
-
-        expect(view.model.changeActivePaymentMethod).not.toBeCalled();
-
-        model.isInEditMode.mockReturnValue(false);
-        view._choosePaymentMethod();
-
-        expect(view.model.changeActivePaymentMethod).toBeCalledTimes(1);
-      }
-    );
+  describe('teardown', () => {
+    test('removes element from the container', () => {
+      const paymentMethod = {
+        type: 'Foo',
+        nonce: 'nonce'
+      };
+      const view = new PaymentMethodView({
+        model: {},
+        strings: strings,
+        paymentMethod: paymentMethod
+      });
 
-    test(
-      'calls model.confirmPaymentMethodDeletion when delete icon is clicked',
-      () => {
-        const fakeModel = {
-          confirmPaymentMethodDeletion: jest.fn()
-        };
-        const paymentMethod = {
-          type: 'Foo',
-          nonce: 'nonce'
-        };
-        const view = new PaymentMethodView({
-          model: fakeModel,
-          strings: strings,
-          paymentMethod: paymentMethod
-        });
+      document.body.appendChild(view.element);
+      jest.spyOn(document.body, 'removeChild');
 
-        view._selectDelete();
+      view.teardown();
 
-        expect(fakeModel.confirmPaymentMethodDeletion).toBeCalledTimes(1);
-        expect(fakeModel.confirmPaymentMethodDeletion).toBeCalledWith(paymentMethod);
-      }
-    );
+      expect(document.body.removeChild).toBeCalledTimes(1);
+      expect(document.body.removeChild).toBeCalledWith(view.element);
+    });
   });
 });
diff --git a/test/unit/views/payment-methods-view.js b/test/unit/views/payment-methods-view.js
index 6c0afde..0851902 100644
--- a/test/unit/views/payment-methods-view.js
+++ b/test/unit/views/payment-methods-view.js
@@ -3,6 +3,7 @@ const BaseView = require('../../../src/views/base-view');
 const CardView = require('../../../src/views/payment-sheet-views/card-view');
 const PayPalView = require('../../../src/views/payment-sheet-views/paypal-view');
 const PaymentMethodsView = require('../../../src/views/payment-methods-view');
+const PaymentMethodView = require('../../../src/views/payment-method-view');
 const DropinError = require('../../../src/lib/dropin-error');
 const classList = require('@braintree/class-list');
 const fake = require('../../helpers/fake');
@@ -228,8 +229,10 @@ describe('PaymentMethodsView', () => {
     });
 
     test(
-      'does not remove other payment methods in non-guest checkout',
+      'does not tear down other payment methods in non-guest checkout',
       () => {
+        jest.spyOn(PaymentMethodView.prototype, 'teardown');
+
         let model, paymentMethodsViews;
         const methodsContainer = testContext.element.querySelector('[data-braintree-id="methods-container"]');
         const modelOptions = fake.modelOptions();
@@ -259,11 +262,14 @@ describe('PaymentMethodsView', () => {
 
           expect(paymentMethodsViews.views.length).toBe(2);
           expect(methodsContainer.childElementCount).toBe(2);
+          expect(PaymentMethodView.prototype.teardown).not.toBeCalled();
         });
       }
     );
 
-    test('removes other payment methods in guest checkout', () => {
+    test('tears down other payment methods in guest checkout', () => {
+      jest.spyOn(PaymentMethodView.prototype, 'teardown');
+
       let model, paymentMethodsViews;
       const methodsContainer = testContext.element.querySelector('[data-braintree-id="methods-container"]');
       const modelOptions = fake.modelOptions();
@@ -287,6 +293,7 @@ describe('PaymentMethodsView', () => {
 
         expect(paymentMethodsViews.views.length).toBe(1);
         expect(methodsContainer.childElementCount).toBe(1);
+        expect(PaymentMethodView.prototype.teardown).toBeCalledTimes(1);
       });
     });
 
@@ -394,7 +401,8 @@ describe('PaymentMethodsView', () => {
         });
         testContext.paymentMethodsViews.views.push({
           paymentMethod: testContext.fakePaymentMethod,
-          element: testContext.element
+          element: testContext.element,
+          teardown: jest.fn()
         });
         testContext.paymentMethodsViews.container = {
           removeChild: jest.fn()
@@ -413,11 +421,12 @@ describe('PaymentMethodsView', () => {
       expect(testContext.paymentMethodsViews.views[0]).toBeFalsy();
     });
 
-    test('removes specified payment method div from DOM', () => {
+    test('tears down specified payment method', () => {
+      const teardown = testContext.paymentMethodsViews.views[0].teardown;
+
       testContext.paymentMethodsViews._removePaymentMethod(testContext.fakePaymentMethod);
 
-      expect(testContext.paymentMethodsViews.container.removeChild).toBeCalledTimes(1);
-      expect(testContext.paymentMethodsViews.container.removeChild).toBeCalledWith(testContext.element);
+      expect(teardown).toBeCalledTimes(1);
     });
 
     test('ignores payment methods that are not the exact object', () => {
@@ -641,12 +650,12 @@ describe('PaymentMethodsView', () => {
       });
     });
 
-    test('removes all payment method views from container', () => {
-      jest.spyOn(testContext.paymentMethodsViews.container, 'removeChild').mockImplementation();
+    test('tears down all payment method views', () => {
+      jest.spyOn(PaymentMethodView.prototype, 'teardown').mockImplementation();
 
       testContext.paymentMethodsViews.refreshPaymentMethods();
 
-      expect(testContext.paymentMethodsViews.container.removeChild).toBeCalledTimes(3);
+      expect(PaymentMethodView.prototype.teardown).toBeCalledTimes(3);
     });
 
     test(