From 6012235dd7b91641f73bad64cf4c847f3e8c0beb Mon Sep 17 00:00:00 2001 From: Agam Rafaeli Date: Sun, 9 Apr 2017 21:32:17 +0300 Subject: [PATCH 1/2] Change buttonbase tests to use faketimers Testing of keyboard focus on button base relied on a certain time gap. This commit changes the test for this case to use sinon's fake timers --- src/internal/ButtonBase.js | 2 ++ src/internal/ButtonBase.spec.js | 36 ++++++++++++++++++++++----------- src/utils/keyboardFocus.js | 4 ++-- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/internal/ButtonBase.js b/src/internal/ButtonBase.js index 9959dbd262856d..eee727286d4ed6 100644 --- a/src/internal/ButtonBase.js +++ b/src/internal/ButtonBase.js @@ -114,6 +114,8 @@ export default class ButtonBase extends Component { keyDown = false; // Used to help track keyboard activation keyDown button = null; keyboardFocusTimeout = undefined; + keyboardFocusCheckTime = 40; + keyboardFocusMaxCheckTimes = 5; focus = () => this.button.focus(); diff --git a/src/internal/ButtonBase.spec.js b/src/internal/ButtonBase.spec.js index 91379a0dc369ef..df3db076025c1a 100644 --- a/src/internal/ButtonBase.spec.js +++ b/src/internal/ButtonBase.spec.js @@ -3,7 +3,7 @@ import React from 'react'; import keycode from 'keycode'; import { assert } from 'chai'; -import { spy } from 'sinon'; +import { spy, useFakeTimers } from 'sinon'; import { createShallow, createMount } from 'src/test-utils'; import ButtonBase, { styleSheet } from './ButtonBase'; @@ -287,12 +287,20 @@ describe('', () => { }); describe('mounted tab press listener', () => { - it('should listen for tab presses and set keyboard focus', (done) => { - const wrapper = mount( + let wrapper; + let instance; + let button; + let clock; + + before(() => { + clock = useFakeTimers(); + wrapper = mount( Hello, ); + instance = wrapper.instance(); + + button = document.getElementById('test-button'); - const button = document.getElementById('test-button'); if (!button) { throw new Error('missing button'); } @@ -301,15 +309,19 @@ describe('', () => { const event = new window.Event('keyup'); event.which = keycode('tab'); window.dispatchEvent(event); + }); + + it('should not set keyboard focus before time has passed', () => { + assert.strictEqual(wrapper.state('keyboardFocused'), false, 'should not be keyboardFocused'); + }); + + it('should listen for tab presses and set keyboard focus', () => { + clock.tick(instance.keyboardFocusCheckTime * instance.keyboardFocusMaxCheckTimes); + assert.strictEqual(wrapper.state('keyboardFocused'), true, 'should be keyboardFocused'); + }); - setTimeout(() => { - assert.strictEqual( - wrapper.state('keyboardFocused'), - true, - 'should be keyboardFocused', - ); - done(); - }, 200); + after(() => { + clock.restore(); }); }); diff --git a/src/utils/keyboardFocus.js b/src/utils/keyboardFocus.js index d9b065685e5dc0..4b4dcf90f26f56 100644 --- a/src/utils/keyboardFocus.js +++ b/src/utils/keyboardFocus.js @@ -22,10 +22,10 @@ export function detectKeyboardFocus(instance, element, cb, attempt = 1) { (document.activeElement === element || contains(element, document.activeElement)) ) { cb(); - } else if (attempt < 5) { + } else if (attempt < instance.keyboardFocusMaxCheckTimes) { detectKeyboardFocus(instance, element, cb, attempt + 1); } - }, 40); + }, instance.keyboardFocusCheckTime); } export function listenForFocusKeys() { From 01e0df9d068f10f0f5d692860241430790ad8ff4 Mon Sep 17 00:00:00 2001 From: Agam Rafaeli Date: Mon, 10 Apr 2017 18:10:33 +0300 Subject: [PATCH 2/2] [ButtonBase] Move the `after()` phase next to the `before()` --- src/internal/ButtonBase.spec.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/internal/ButtonBase.spec.js b/src/internal/ButtonBase.spec.js index df3db076025c1a..341dbd0ee2f8f0 100644 --- a/src/internal/ButtonBase.spec.js +++ b/src/internal/ButtonBase.spec.js @@ -311,6 +311,10 @@ describe('', () => { window.dispatchEvent(event); }); + after(() => { + clock.restore(); + }); + it('should not set keyboard focus before time has passed', () => { assert.strictEqual(wrapper.state('keyboardFocused'), false, 'should not be keyboardFocused'); }); @@ -319,10 +323,6 @@ describe('', () => { clock.tick(instance.keyboardFocusCheckTime * instance.keyboardFocusMaxCheckTimes); assert.strictEqual(wrapper.state('keyboardFocused'), true, 'should be keyboardFocused'); }); - - after(() => { - clock.restore(); - }); }); describe('prop: disabled', () => {