Skip to content

Commit

Permalink
Focus the currently focused node when it mounts (#593)
Browse files Browse the repository at this point in the history
* Add failing test for focus after node insertion

* Redo the way ToplevelBlockEditable works to be more friendly

* Focus the currently focused node as soon as it mounts

* Only focus a node on mount if the editor has focus

* Add tests for focusing after a top-level deletion

* Add tests for focusing after deleting a child node
  • Loading branch information
pcardune authored Nov 20, 2021
1 parent 88f004e commit 794689c
Show file tree
Hide file tree
Showing 7 changed files with 273 additions and 140 deletions.
4 changes: 2 additions & 2 deletions packages/cmb-toolkit/src/simulate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ export function insertText(text: string) {
// See https://github.com/testing-library/dom-testing-library/pull/235
// for some discussion about this.
fireEvent.input(activeEl, {
target: { innerHTML: text },
target: { innerHTML: activeEl.innerHTML + text },
});
}
}
Expand Down Expand Up @@ -171,7 +171,7 @@ function makeKeyEvent<T extends Record<string, unknown>>(
// https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key/Key_Values
function getKeyCode(key: string): number {
// The key code for an (uppercase) letter is that letter's ascii value.
if (key.match(/^[A-Z]$/)) {
if (key.match(/^[A-Za-z0-9]$/)) {
return key.charCodeAt(0);
}
// The key code for a digit is that digit's ascii value.
Expand Down
77 changes: 75 additions & 2 deletions packages/codemirror-blocks/spec/activation-test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import wescheme from "../src/languages/wescheme";
import "codemirror/addon/search/searchcursor.js";

import { screen } from "@testing-library/react";
import {
cmd_ctrl,
teardown,
Expand All @@ -10,6 +9,8 @@ import {
mountCMB,
isNodeEditable,
elementForNode,
keyPress,
click,
} from "../src/toolkit/test-utils";
import { API } from "../src/CodeMirrorBlocks";
import { ASTNode } from "../src/ast";
Expand Down Expand Up @@ -131,6 +132,78 @@ describe("when dealing with node activation,", () => {
});
});

describe("inserting a new node", () => {
let cmb: API;
beforeEach(() => {
cmb = mountCMB(wescheme).cmb;
});
it("should focus the node that was just inserted", () => {
cmb.focus();
// start inserting a new node
keyPress("a");
insertText("BrandNewLiteral");
// confirm the insertion by pressing Enter
keyDown("Enter");
// confirm that the new new was saved to the ast and focused
expect(cmb.getAst().toString()).toBe("aBrandNewLiteral");
expect(cmb.getValue()).toEqual("aBrandNewLiteral");
expect(
screen.getByRole("treeitem", { name: /aBrandNewLiteral/ })
).toHaveFocus();
});
});

describe("switching to block mode", () => {
let cmb: API;
beforeEach(() => {
cmb = mountCMB(wescheme).cmb;
cmb.setBlockMode(false);
cmb.setValue("foo bar");
});

it("should not change the focused element", () => {
const blockModeBtn = screen.getByRole("button", {
name: /Switch to blocks mode/,
});
blockModeBtn.focus();
expect(blockModeBtn).toHaveFocus();
click(blockModeBtn);
expect(blockModeBtn).toHaveFocus();
});
});

describe("deleting a node", () => {
let cmb: API;
beforeEach(() => {
cmb = mountCMB(wescheme).cmb;
});
it("should focus on the previous node when a top-level node is deleted", () => {
cmb.setValue("firstLiteral\nsecondLiteral\nthirdLiteral");
mouseDown(screen.getByRole("treeitem", { name: /thirdLiteral/ }));
keyDown(" ");
keyDown("Delete");
expect(cmb.getValue()).toBe("firstLiteral\nsecondLiteral\n");
expect(
screen.getByRole("treeitem", { name: /secondLiteral/ })
).toHaveFocus();
expect(cmb.getCursor()).toMatchInlineSnapshot(`
Object {
"ch": 0,
"line": 1,
}
`);
});

it("should focus on the previous child node when a child node is deleted", () => {
cmb.setValue("(someFunc firstArg secondArg)");
mouseDown(screen.getByRole("treeitem", { name: /secondArg/ }));
keyDown(" ");
keyDown("Delete");
expect(cmb.getValue()).toBe("(someFunc firstArg)");
expect(screen.getByRole("treeitem", { name: /firstArg/ })).toHaveFocus();
});
});

describe("cut/copy/paste", () => {
let cmb!: API;
let literal1!: ASTNode;
Expand Down
19 changes: 19 additions & 0 deletions packages/codemirror-blocks/src/components/Node.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,25 @@ const Node = ({ expandable = true, ...props }: Props) => {
}
}, [props.node.id]);

// If this is the currently focused node, then focus the node element
// as soon as we finish rendering.
const focusedNode = useSelector(selectors.getFocusedNode);
useEffect(() => {
if (
!editor?.hasFocus() &&
document.activeElement !== document.body &&
document.activeElement !== null &&
document.activeElement.id !== "SR_fix_for_slow_dom"
) {
// If the editor doesn't have focus, then we don't want to steal
// focus from something else.
return;
}
if (nodeElementRef.current && focusedNode?.id === props.node.id) {
nodeElementRef.current?.focus();
}
}, [editor, focusedNode?.id, props.node.id]);

if (editable) {
if (!editor) {
throw new Error("can't edit nodes before codemirror has mounted");
Expand Down
Loading

0 comments on commit 794689c

Please sign in to comment.