Skip to content
Permalink
Browse files
fix: Scroll active element into view while focusing
It looks like using element.focus() doesn't always scroll the element
into view if a parent element has scrolling enabled. This caused a
problem with large dropdown menus and using the ArrowUp key to trigger
the menu that would not have the current element visible until the
ArrowUp key was pressed again.
  • Loading branch information
mlaursen committed Jul 12, 2020
1 parent 3f2f4e8 commit a9a090268f8aecb8b7478dc3fb6c06eec346c62a
Showing 2 changed files with 63 additions and 0 deletions.
@@ -1,10 +1,13 @@
import { mocked } from "ts-jest/utils";
import focusElementWithin from "../focusElementWithin";
import getFocusableElements_ from "../getFocusableElements";
import scrollIntoView_ from "../../scrollIntoView";

jest.mock("../getFocusableElements");
jest.mock("../../scrollIntoView");

const getFocusableElements = mocked(getFocusableElements_);
const scrollIntoView = mocked(scrollIntoView_);
const ELEMENT = document.createElement("div");
const element1 = document.createElement("button");
element1.className = "element element--1";
@@ -26,6 +29,7 @@ describe("focusElementWithin", () => {
focus2.mockClear();
focus3.mockClear();
getFocusableElements.mockClear();
scrollIntoView.mockClear();
});

it("should throw an error if it can not find a focusable element", () => {
@@ -132,4 +136,59 @@ describe("focusElementWithin", () => {
expect(focus2).not.toBeCalled();
expect(focus3).toBeCalled();
});

it("should call scrollIntoView if preventScroll is false and the container element is not the document", () => {
elements.forEach((element) => {
document.body.appendChild(element);
});
focusElementWithin(document, "first", false, false, elements);
focusElementWithin(document, "last", false, false, elements);
focusElementWithin(document, ".element", false, false, elements);
focusElementWithin(document, "first", true, false, elements);
focusElementWithin(document, "last", true, false, elements);
focusElementWithin(document, ".element", true, false, elements);
focusElementWithin(document, "first", false, true, elements);
focusElementWithin(document, "last", false, true, elements);
focusElementWithin(document, ".element", false, true, elements);
focusElementWithin(document, "first", true, true, elements);
focusElementWithin(document, "last", true, true, elements);
focusElementWithin(document, ".element", true, true, elements);
elements.forEach((element) => {
document.body.removeChild(element);
});

expect(scrollIntoView).not.toBeCalled();

const container = document.createElement("div");
elements.forEach((element) => {
container.appendChild(element);
});

focusElementWithin(container, "first", false, false, elements);
expect(scrollIntoView).toBeCalledWith(container, elements[0]);

focusElementWithin(container, "last", false, false, elements);
expect(scrollIntoView).toBeCalledWith(container, elements[2]);

focusElementWithin(
container,
".element:nth-child(2)",
false,
false,
elements
);
expect(scrollIntoView).toBeCalledWith(container, elements[1]);

scrollIntoView.mockClear();
focusElementWithin(container, "first", false, true, elements);
focusElementWithin(container, "last", false, true, elements);
focusElementWithin(
container,
".element:nth-child(2)",
false,
true,
elements
);
expect(scrollIntoView).not.toBeCalled();
});
});
@@ -1,4 +1,5 @@
import getFocusableElements from "./getFocusableElements";
import scrollIntoView from "../scrollIntoView";

export type Focus = "first" | "last" | string;

@@ -45,4 +46,7 @@ export default function focusElementWithin(
}

el.focus({ preventScroll });
if (!preventScroll && container !== document) {
scrollIntoView(container as HTMLElement, el);
}
}

0 comments on commit a9a0902

Please sign in to comment.