From 2accff896c73da0c7a9df5e98eeef64a3a221330 Mon Sep 17 00:00:00 2001 From: Ben Monro Date: Thu, 2 Jan 2020 16:51:06 -0800 Subject: [PATCH 1/2] fix: account for sorting bug in node 10 --- __tests__/react/tab.js | 26 +++++++++++++++++++++++++- src/index.js | 31 +++++++++++++++++++++++-------- 2 files changed, 48 insertions(+), 9 deletions(-) diff --git a/__tests__/react/tab.js b/__tests__/react/tab.js index 55b3e5e8..2af92f57 100644 --- a/__tests__/react/tab.js +++ b/__tests__/react/tab.js @@ -157,7 +157,8 @@ describe("userEvent.tab", () => { expect(link).toHaveFocus(); }); - it("should stay within a focus trab", () => { + + it("should stay within a focus trap", () => { const { getAllByTestId, getByTestId } = render( <>
@@ -219,4 +220,27 @@ describe("userEvent.tab", () => { // cycle goes back to first element expect(checkbox2).toHaveFocus(); }); + + // prior to node 11, Array.sort was unstable for arrays w/ length > 10. + // https://twitter.com/mathias/status/1036626116654637057 + // for this reason, the tab() function needs to account for this in it's sorting. + // for example under node 10 in this test: + // > 'abcdefghijklmnopqrstuvwxyz'.split('').sort(() => 0).join('') + // will give you 'nacdefghijklmbopqrstuvwxyz' + it("should support unstable sorting environments like node 10", () => { + const letters = 'abcdefghijklmnopqrstuvwxyz'; + + const { getByTestId } = render(<> + {letters.split('').map(letter => )} + ) + + expect.assertions(26); + + + letters.split('').forEach(letter => { + userEvent.tab(); + expect(getByTestId(letter)).toHaveFocus(); + }) + + }) }); diff --git a/src/index.js b/src/index.js index 043ef857..d23b6a65 100644 --- a/src/index.js +++ b/src/index.js @@ -1,7 +1,7 @@ import { fireEvent } from "@testing-library/dom"; function wait(time) { - return new Promise(function(resolve) { + return new Promise(function (resolve) { setTimeout(() => resolve(), time); }); } @@ -12,6 +12,23 @@ function findTagInParents(element, tagName) { return findTagInParents(element.parentNode, tagName); } +let sortListByTabIndex; +let nodeVersion = +process.version.slice(1).split('.')[0]; +if (nodeVersion < 11) { + sortListByTabIndex = list => list.map((el, idx) => ({ el, idx })) + .sort((a, b) => { + const tabIndexA = +a.el.getAttribute("tabindex"); + const tabIndexB = +b.el.getAttribute("tabindex"); + return tabIndexA < tabIndexB ? -1 : tabIndexA > tabIndexB ? 1 : a.idx - b.idx; + }).map(({ el }) => el); +} else { + sortListByTabIndex = list => list.sort((a, b) => { + const tabIndexA = +a.getAttribute("tabindex"); + const tabIndexB = +b.getAttribute("tabindex"); + return tabIndexA < tabIndexB ? -1 : tabIndexA > tabIndexB ? 1 : 0; + }); +} + function clickLabel(label) { fireEvent.mouseOver(label); fireEvent.mouseMove(label); @@ -232,15 +249,13 @@ const userEvent = { const focusableElements = focusTrap.querySelectorAll( "input, button, select, textarea, a[href], [tabindex]" ); - const list = Array.prototype.filter - .call(focusableElements, function(item) { + let list = Array.prototype.filter + .call(focusableElements, function (item) { return item.getAttribute("tabindex") !== "-1"; - }) - .sort((a, b) => { - const tabIndexA = a.getAttribute("tabindex"); - const tabIndexB = b.getAttribute("tabindex"); - return tabIndexA < tabIndexB ? -1 : tabIndexA > tabIndexB ? 1 : 0; }); + + list = sortListByTabIndex(list); + const index = list.indexOf(document.activeElement); let nextIndex = shift ? index - 1 : index + 1; From 7acdf8470879372c22c63f7a7f793e32dce040e8 Mon Sep 17 00:00:00 2001 From: Ben Monro Date: Fri, 3 Jan 2020 08:03:51 -0800 Subject: [PATCH 2/2] just one sort regardless of node version --- __tests__/react/tab.js | 21 +++++++++++---------- src/index.js | 36 ++++++++++++------------------------ 2 files changed, 23 insertions(+), 34 deletions(-) diff --git a/__tests__/react/tab.js b/__tests__/react/tab.js index 2af92f57..fcd25279 100644 --- a/__tests__/react/tab.js +++ b/__tests__/react/tab.js @@ -157,7 +157,6 @@ describe("userEvent.tab", () => { expect(link).toHaveFocus(); }); - it("should stay within a focus trap", () => { const { getAllByTestId, getByTestId } = render( <> @@ -228,19 +227,21 @@ describe("userEvent.tab", () => { // > 'abcdefghijklmnopqrstuvwxyz'.split('').sort(() => 0).join('') // will give you 'nacdefghijklmbopqrstuvwxyz' it("should support unstable sorting environments like node 10", () => { - const letters = 'abcdefghijklmnopqrstuvwxyz'; + const letters = "abcdefghijklmnopqrstuvwxyz"; - const { getByTestId } = render(<> - {letters.split('').map(letter => )} - ) + const { getByTestId } = render( + <> + {letters.split("").map(letter => ( + + ))} + + ); expect.assertions(26); - - letters.split('').forEach(letter => { + letters.split("").forEach(letter => { userEvent.tab(); expect(getByTestId(letter)).toHaveFocus(); - }) - - }) + }); + }); }); diff --git a/src/index.js b/src/index.js index d23b6a65..ba53d28c 100644 --- a/src/index.js +++ b/src/index.js @@ -12,23 +12,6 @@ function findTagInParents(element, tagName) { return findTagInParents(element.parentNode, tagName); } -let sortListByTabIndex; -let nodeVersion = +process.version.slice(1).split('.')[0]; -if (nodeVersion < 11) { - sortListByTabIndex = list => list.map((el, idx) => ({ el, idx })) - .sort((a, b) => { - const tabIndexA = +a.el.getAttribute("tabindex"); - const tabIndexB = +b.el.getAttribute("tabindex"); - return tabIndexA < tabIndexB ? -1 : tabIndexA > tabIndexB ? 1 : a.idx - b.idx; - }).map(({ el }) => el); -} else { - sortListByTabIndex = list => list.sort((a, b) => { - const tabIndexA = +a.getAttribute("tabindex"); - const tabIndexB = +b.getAttribute("tabindex"); - return tabIndexA < tabIndexB ? -1 : tabIndexA > tabIndexB ? 1 : 0; - }); -} - function clickLabel(label) { fireEvent.mouseOver(label); fireEvent.mouseMove(label); @@ -249,19 +232,24 @@ const userEvent = { const focusableElements = focusTrap.querySelectorAll( "input, button, select, textarea, a[href], [tabindex]" ); - let list = Array.prototype.filter - .call(focusableElements, function (item) { - return item.getAttribute("tabindex") !== "-1"; - }); + let list = Array.prototype.filter.call(focusableElements, function (item) { + return item.getAttribute("tabindex") !== "-1"; + }).map((el, idx) => ({ el, idx })) + .sort((a, b) => { + const tabIndexA = a.el.getAttribute("tabindex"); + const tabIndexB = b.el.getAttribute("tabindex"); + + const diff = tabIndexA - tabIndexB; - list = sortListByTabIndex(list); + return diff !== 0 ? diff : a.idx - b.idx; + }) - const index = list.indexOf(document.activeElement); + const index = list.findIndex(({ el }) => el === document.activeElement); let nextIndex = shift ? index - 1 : index + 1; let defaultIndex = shift ? list.length - 1 : 0; - const next = list[nextIndex] || list[defaultIndex]; + const { el: next } = (list[nextIndex] || list[defaultIndex]); if (next.getAttribute("tabindex") === null) { next.setAttribute("tabindex", "0"); // jsdom requires tabIndex=0 for an item to become 'document.activeElement' (the browser does not)