Skip to content
Snippets Groups Projects
Unverified Commit 701b0d3b authored by Paul Rosenzweig's avatar Paul Rosenzweig Committed by GitHub
Browse files

use key rather than keyCode for comma (#10902)

parent e65fbcfd
No related branches found
No related tags found
No related merge requests found
...@@ -13,11 +13,11 @@ import Popover from "metabase/components/Popover"; ...@@ -13,11 +13,11 @@ import Popover from "metabase/components/Popover";
import { import {
KEYCODE_ESCAPE, KEYCODE_ESCAPE,
KEYCODE_ENTER, KEYCODE_ENTER,
KEYCODE_COMMA,
KEYCODE_TAB, KEYCODE_TAB,
KEYCODE_UP, KEYCODE_UP,
KEYCODE_DOWN, KEYCODE_DOWN,
KEYCODE_BACKSPACE, KEYCODE_BACKSPACE,
KEY_COMMA,
} from "metabase/lib/keyboard"; } from "metabase/lib/keyboard";
import { isObscured } from "metabase/lib/dom"; import { isObscured } from "metabase/lib/dom";
...@@ -266,7 +266,7 @@ export default class TokenField extends Component { ...@@ -266,7 +266,7 @@ export default class TokenField extends Component {
this.props.onInputKeyDown(event); this.props.onInputKeyDown(event);
} }
const keyCode = event.keyCode; const { key, keyCode } = event;
const { filteredOptions, selectedOptionValue } = this.state; const { filteredOptions, selectedOptionValue } = this.state;
...@@ -274,7 +274,11 @@ export default class TokenField extends Component { ...@@ -274,7 +274,11 @@ export default class TokenField extends Component {
if ( if (
keyCode === KEYCODE_ESCAPE || keyCode === KEYCODE_ESCAPE ||
keyCode === KEYCODE_TAB || keyCode === KEYCODE_TAB ||
keyCode === KEYCODE_COMMA || // We check event.key for comma presses because some keyboard layouts
// (e.g. Russian) have a letter on that key and require a modifier to type
// ",". Similarly, if you want to type "<" on the US keyboard layout, you
// need to look at `key` to distinguish it from ",".
key === KEY_COMMA ||
keyCode === KEYCODE_ENTER keyCode === KEYCODE_ENTER
) { ) {
if (this.addSelectedOption(event)) { if (this.addSelectedOption(event)) {
......
...@@ -8,5 +8,5 @@ export const KEYCODE_UP = 38; ...@@ -8,5 +8,5 @@ export const KEYCODE_UP = 38;
export const KEYCODE_RIGHT = 39; export const KEYCODE_RIGHT = 39;
export const KEYCODE_DOWN = 40; export const KEYCODE_DOWN = 40;
export const KEYCODE_COMMA = 188; export const KEY_COMMA = ",";
export const KEYCODE_FORWARD_SLASH = 191; export const KEYCODE_FORWARD_SLASH = 191;
...@@ -11,7 +11,7 @@ import { ...@@ -11,7 +11,7 @@ import {
KEYCODE_DOWN, KEYCODE_DOWN,
KEYCODE_TAB, KEYCODE_TAB,
KEYCODE_ENTER, KEYCODE_ENTER,
KEYCODE_COMMA, KEY_COMMA,
} from "metabase/lib/keyboard"; } from "metabase/lib/keyboard";
const DEFAULT_OPTIONS = ["Doohickey", "Gadget", "Gizmo", "Widget"]; const DEFAULT_OPTIONS = ["Doohickey", "Gadget", "Gizmo", "Widget"];
...@@ -168,6 +168,19 @@ describe("TokenField", () => { ...@@ -168,6 +168,19 @@ describe("TokenField", () => {
expect(value()).toEqual(["bar"]); expect(value()).toEqual(["bar"]);
}); });
it("should type a character that's on the comma key", () => {
component = mount(
<TokenFieldWithStateAndDefaults value={[]} options={["fooбar"]} />,
);
focus();
type("foo");
// 188 is comma on most layouts
input().simulate("keydown", { keyCode: 188, key: "б" });
// if that keydown was interpreted as a comma, the value would be "fooбar"
expect(input().props().value).toEqual("foo");
});
describe("when updateOnInputChange is provided", () => { describe("when updateOnInputChange is provided", () => {
beforeEach(() => { beforeEach(() => {
component = mount( component = mount(
...@@ -340,8 +353,12 @@ describe("TokenField", () => { ...@@ -340,8 +353,12 @@ describe("TokenField", () => {
}); });
describe("key selection", () => { describe("key selection", () => {
[KEYCODE_TAB, KEYCODE_ENTER, KEYCODE_COMMA].map(key => [
it(`should allow the user to use arrow keys and then ${key} to select a recipient`, () => { ["keyCode", KEYCODE_TAB],
["keyCode", KEYCODE_ENTER],
["key", KEY_COMMA],
].map(([keyType, keyValue]) =>
it(`should allow the user to use arrow keys and then ${keyType}: ${keyValue} to select a recipient`, () => {
const spy = jest.fn(); const spy = jest.fn();
component = mount( component = mount(
...@@ -367,7 +384,7 @@ describe("TokenField", () => { ...@@ -367,7 +384,7 @@ describe("TokenField", () => {
expect(component.state().selectedOptionValue).toBe(DEFAULT_OPTIONS[2]); expect(component.state().selectedOptionValue).toBe(DEFAULT_OPTIONS[2]);
input().simulate("keydown", { input().simulate("keydown", {
keyCode: key, [keyType]: keyValue,
preventDefalut: jest.fn(), preventDefalut: jest.fn(),
}); });
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment