From e8193d6d48b191dc114bce8146c629e508a1a235 Mon Sep 17 00:00:00 2001 From: Guillaume Peoc'h Date: Wed, 4 May 2022 12:17:43 +0200 Subject: [PATCH] SONAR-16337 Replace keymaster in Components --- .../main/js/components/common/SelectList.tsx | 61 ++-------- .../common/__tests__/SelectList-test.tsx | 94 ++++++--------- .../__tests__/withKeyboardNavigation-test.tsx | 19 +--- .../components/hoc/withKeyboardNavigation.tsx | 40 +++---- .../src/main/js/components/issue/Issue.tsx | 80 ++++++------- .../__snapshots__/issue-test.tsx.snap | 60 ++++++++++ .../components/issue/__tests__/issue-test.tsx | 107 ++++++++++++++++++ .../sonar-web/src/main/js/helpers/keycodes.ts | 10 +- 8 files changed, 275 insertions(+), 196 deletions(-) create mode 100644 server/sonar-web/src/main/js/components/issue/__tests__/__snapshots__/issue-test.tsx.snap create mode 100644 server/sonar-web/src/main/js/components/issue/__tests__/issue-test.tsx diff --git a/server/sonar-web/src/main/js/components/common/SelectList.tsx b/server/sonar-web/src/main/js/components/common/SelectList.tsx index 4435fc11dc4..9b25a4b6f08 100644 --- a/server/sonar-web/src/main/js/components/common/SelectList.tsx +++ b/server/sonar-web/src/main/js/components/common/SelectList.tsx @@ -18,8 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ import classNames from 'classnames'; -import key from 'keymaster'; -import { uniqueId } from 'lodash'; import * as React from 'react'; import { KeyboardCodes } from '../../helpers/keycodes'; import SelectListItem from './SelectListItem'; @@ -36,10 +34,6 @@ interface State { } export default class SelectList extends React.PureComponent { - currentKeyScope?: string; - previousFilter?: (event: any) => void; - previousKeyScope?: string; - constructor(props: Props) { super(props); this.state = { @@ -48,7 +42,7 @@ export default class SelectList extends React.PureComponent { } componentDidMount() { - this.attachShortcuts(); + document.addEventListener('keydown', this.handleKeyDown, { capture: true }); } componentDidUpdate(prevProps: Props) { @@ -61,56 +55,25 @@ export default class SelectList extends React.PureComponent { } componentWillUnmount() { - this.detachShortcuts(); + document.removeEventListener('keydown', this.handleKeyDown, { capture: true }); } - attachShortcuts = () => { - this.previousKeyScope = key.getScope(); - this.previousFilter = key.filter; - this.currentKeyScope = uniqueId('key-scope'); - key.setScope(this.currentKeyScope); - - // sometimes there is a *focused* search field next to the SelectList component - // we need to allow shortcuts in this case, but only for the used keys - - Object.assign(key, { - filter: (event: KeyboardEvent & { target: HTMLElement }) => { - const { tagName } = event.target || event.srcElement; - const isInput = tagName === 'INPUT' || tagName === 'SELECT' || tagName === 'TEXTAREA'; - return ( - [KeyboardCodes.Enter, KeyboardCodes.UpArrow, KeyboardCodes.DownArrow].includes( - event.code as KeyboardCodes - ) || !isInput - ); - } - }); - - key('down', this.currentKeyScope, () => { + handleKeyDown = (event: KeyboardEvent) => { + if (event.code === KeyboardCodes.DownArrow) { + event.preventDefault(); + event.stopImmediatePropagation(); this.setState(this.selectNextElement); - return false; - }); - - key('up', this.currentKeyScope, () => { + } else if (event.code === KeyboardCodes.UpArrow) { + event.preventDefault(); + event.stopImmediatePropagation(); this.setState(this.selectPreviousElement); - return false; - }); - - key('return', this.currentKeyScope, () => { + } else if (event.code === KeyboardCodes.Enter) { + event.preventDefault(); + event.stopImmediatePropagation(); if (this.state.active != null) { this.handleSelect(this.state.active); } - return false; - }); - }; - - detachShortcuts = () => { - if (this.previousKeyScope) { - key.setScope(this.previousKeyScope); - } - if (this.currentKeyScope) { - key.deleteScope(this.currentKeyScope); } - Object.assign(key, { filter: this.previousFilter }); }; handleSelect = (item: string) => { diff --git a/server/sonar-web/src/main/js/components/common/__tests__/SelectList-test.tsx b/server/sonar-web/src/main/js/components/common/__tests__/SelectList-test.tsx index 83e9937aae2..cc6caf4c1ed 100644 --- a/server/sonar-web/src/main/js/components/common/__tests__/SelectList-test.tsx +++ b/server/sonar-web/src/main/js/components/common/__tests__/SelectList-test.tsx @@ -17,77 +17,42 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -import { mount, shallow } from 'enzyme'; +import { shallow } from 'enzyme'; import * as React from 'react'; import { KeyboardCodes } from '../../../helpers/keycodes'; -import { click, KEYCODE_MAP, keydown } from '../../../helpers/testUtils'; +import { keydown } from '../../../helpers/testUtils'; import SelectList from '../SelectList'; import SelectListItem from '../SelectListItem'; -jest.mock('keymaster', () => { - const key: any = (bindKey: string, _: string, callback: Function) => { - document.addEventListener('keydown', (event: KeyboardEvent) => { - const keymasterCode = event.code && KEYCODE_MAP[event.code as KeyboardCodes]; - if (keymasterCode && bindKey.split(',').includes(keymasterCode)) { - return callback(); - } - return true; - }); - }; - let scope = 'key-scope'; - - key.getScope = () => scope; - key.setScope = (newScope: string) => { - scope = newScope; - }; - key.deleteScope = jest.fn(); - - return key; -}); - it('should render correctly without children', () => { - const onSelect = jest.fn(); - expect( - shallow( - - ) - ).toMatchSnapshot(); + const wrapper = shallowRender(); + expect(wrapper).toMatchSnapshot(); + wrapper.instance().componentWillUnmount!(); }); it('should render correctly with children', () => { - const onSelect = jest.fn(); const items = ['item', 'seconditem', 'third']; - expect( - shallow( - - {items.map(item => ( - - - item - - ))} - - ) - ).toMatchSnapshot(); + const children = items.map(item => ( + + + item + + )); + const wrapper = shallowRender({ items }, children); + expect(wrapper).toMatchSnapshot(); + wrapper.instance().componentWillUnmount!(); }); it('should correclty handle user actions', () => { const onSelect = jest.fn(); const items = ['item', 'seconditem', 'third']; - const list = mount( - - {items.map(item => ( - - - item - - ))} - - ); + const children = items.map(item => ( + + + item + + )); + const list = shallowRender({ items, onSelect }, children); expect(list.state().active).toBe('seconditem'); keydown({ code: KeyboardCodes.DownArrow }); expect(list.state().active).toBe('third'); @@ -97,6 +62,19 @@ it('should correclty handle user actions', () => { expect(list.state().active).toBe('third'); keydown({ code: KeyboardCodes.UpArrow }); expect(list.state().active).toBe('seconditem'); - click(list.find('a').at(2)); - expect(onSelect).toBeCalledWith('third'); + keydown({ code: KeyboardCodes.Enter }); + expect(onSelect).toBeCalledWith('seconditem'); + list.instance().componentWillUnmount!(); }); + +function shallowRender(props: Partial = {}, children?: React.ReactNode) { + return shallow( + + {children} + + ); +} diff --git a/server/sonar-web/src/main/js/components/hoc/__tests__/withKeyboardNavigation-test.tsx b/server/sonar-web/src/main/js/components/hoc/__tests__/withKeyboardNavigation-test.tsx index 2ff6d35dc41..b4197c05a62 100644 --- a/server/sonar-web/src/main/js/components/hoc/__tests__/withKeyboardNavigation-test.tsx +++ b/server/sonar-web/src/main/js/components/hoc/__tests__/withKeyboardNavigation-test.tsx @@ -21,7 +21,7 @@ import { mount, shallow } from 'enzyme'; import * as React from 'react'; import { KeyboardCodes } from '../../../helpers/keycodes'; import { mockComponent } from '../../../helpers/mocks/component'; -import { KEYCODE_MAP, keydown } from '../../../helpers/testUtils'; +import { keydown } from '../../../helpers/testUtils'; import { ComponentMeasure } from '../../../types/types'; import withKeyboardNavigation, { WithKeyboardNavigationProps } from '../withKeyboardNavigation'; @@ -42,23 +42,6 @@ const COMPONENTS = [ mockComponent({ key: 'file-3' }) ]; -jest.mock('keymaster', () => { - const key: any = (bindKey: string, _: string, callback: Function) => { - document.addEventListener('keydown', (event: KeyboardEvent) => { - const keymasterCode = event.code && KEYCODE_MAP[event.code as KeyboardCodes]; - if (keymasterCode && bindKey.split(',').includes(keymasterCode)) { - return callback(); - } - return true; - }); - }; - - key.setScope = jest.fn(); - key.deleteScope = jest.fn(); - - return key; -}); - it('should wrap component correctly', () => { const wrapper = shallow(applyProps()); expect(wrapper.find('X').exists()).toBe(true); diff --git a/server/sonar-web/src/main/js/components/hoc/withKeyboardNavigation.tsx b/server/sonar-web/src/main/js/components/hoc/withKeyboardNavigation.tsx index 800aa76023c..b92a470199e 100644 --- a/server/sonar-web/src/main/js/components/hoc/withKeyboardNavigation.tsx +++ b/server/sonar-web/src/main/js/components/hoc/withKeyboardNavigation.tsx @@ -17,10 +17,10 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -import key from 'keymaster'; import * as React from 'react'; import PageActions from '../../components/ui/PageActions'; import { getComponentMeasureUniqueKey } from '../../helpers/component'; +import { KeyboardCodes } from '../../helpers/keycodes'; import { ComponentMeasure } from '../../types/types'; import { getWrappedDisplayName } from './utils'; @@ -35,8 +35,6 @@ export interface WithKeyboardNavigationProps { selected?: ComponentMeasure; } -const KEY_SCOPE = 'key_nav'; - export default function withKeyboardNavigation

( WrappedComponent: React.ComponentClass

> ) { @@ -44,32 +42,29 @@ export default function withKeyboardNavigation

( static displayName = getWrappedDisplayName(WrappedComponent, 'withKeyboardNavigation'); componentDidMount() { - this.attachShortcuts(); + document.addEventListener('keydown', this.handleKeyDown); } componentWillUnmount() { - this.detachShortcuts(); + document.removeEventListener('keydown', this.handleKeyDown); } - attachShortcuts = () => { - key.setScope(KEY_SCOPE); - key('up', KEY_SCOPE, () => { + handleKeyDown = (event: KeyboardEvent) => { + if (event.code === KeyboardCodes.UpArrow) { + event.preventDefault(); return this.skipIfFile(this.handleHighlightPrevious); - }); - key('down', KEY_SCOPE, () => { + } else if (event.code === KeyboardCodes.DownArrow) { + event.preventDefault(); return this.skipIfFile(this.handleHighlightNext); - }); - key('right,enter', KEY_SCOPE, () => { + } else if (event.code === KeyboardCodes.RightArrow || event.code === KeyboardCodes.Enter) { + event.preventDefault(); return this.skipIfFile(this.handleSelectCurrent); - }); - key('left', KEY_SCOPE, () => { + } else if (event.code === KeyboardCodes.LeftArrow) { + event.preventDefault(); this.handleSelectParent(); - return false; // always hijack left - }); - }; - - detachShortcuts = () => { - key.deleteScope(KEY_SCOPE); + return false; // always hijack left / Why did you put this @wouter? + } + return false; }; getCurrentIndex = () => { @@ -85,10 +80,9 @@ export default function withKeyboardNavigation

( skipIfFile = (handler: () => void) => { if (this.props.isFile) { return true; - } else { - handler(); - return false; } + handler(); + return false; }; handleHighlightNext = () => { diff --git a/server/sonar-web/src/main/js/components/issue/Issue.tsx b/server/sonar-web/src/main/js/components/issue/Issue.tsx index 0d4c0d01ed7..c06a7d5d694 100644 --- a/server/sonar-web/src/main/js/components/issue/Issue.tsx +++ b/server/sonar-web/src/main/js/components/issue/Issue.tsx @@ -17,9 +17,9 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -import key from 'keymaster'; import * as React from 'react'; import { setIssueAssignee } from '../../api/issues'; +import { KeyboardKeys } from '../../helpers/keycodes'; import { BranchLike } from '../../types/branch-like'; import { Issue as TypeIssue } from '../../types/types'; import { updateIssue } from './actions'; @@ -48,69 +48,55 @@ export default class Issue extends React.PureComponent { componentDidMount() { if (this.props.selected) { - this.bindShortcuts(); + document.addEventListener('keydown', this.handleKeyDown, { capture: true }); } } componentDidUpdate(prevProps: Props) { if (!prevProps.selected && this.props.selected) { - this.bindShortcuts(); + document.addEventListener('keydown', this.handleKeyDown, { capture: true }); } else if (prevProps.selected && !this.props.selected) { - this.unbindShortcuts(); + document.removeEventListener('keydown', this.handleKeyDown, { capture: true }); } } componentWillUnmount() { if (this.props.selected) { - this.unbindShortcuts(); + document.removeEventListener('keydown', this.handleKeyDown, { capture: true }); } } - bindShortcuts() { - key('f', 'issues', () => { - this.togglePopup('transition'); + handleKeyDown = (event: KeyboardEvent) => { + const { tagName } = event.target as HTMLElement; + const isInput = tagName === 'INPUT' || tagName === 'SELECT' || tagName === 'TEXTAREA'; + if (isInput) { return false; - }); - key('a', 'issues', () => { - this.togglePopup('assign'); - return false; - }); - key('m', 'issues', () => { - if (this.props.issue.actions.includes('assign')) { - this.handleAssignement('_me'); - } - return false; - }); - key('i', 'issues', () => { - this.togglePopup('set-severity'); - return false; - }); - key('c', 'issues', () => { - this.togglePopup('comment'); - return false; - }); - key('t', 'issues', () => { - this.togglePopup('edit-tags'); - return false; - }); - key('space', 'issues', () => { + } else if (event.key === KeyboardKeys.KeyF) { + event.preventDefault(); + return this.togglePopup('transition'); + } else if (event.key === KeyboardKeys.KeyA) { + event.preventDefault(); + return this.togglePopup('assign'); + } else if (event.key === KeyboardKeys.KeyM && this.props.issue.actions.includes('assign')) { + event.preventDefault(); + return this.handleAssignement('_me'); + } else if (event.key === KeyboardKeys.KeyI) { + event.preventDefault(); + return this.togglePopup('set-severity'); + } else if (event.key === KeyboardKeys.KeyC) { + event.preventDefault(); + return this.togglePopup('comment'); + } else if (event.key === KeyboardKeys.KeyT) { + event.preventDefault(); + return this.togglePopup('edit-tags'); + } else if (event.key === KeyboardKeys.Space) { + event.preventDefault(); if (this.props.onCheck) { - this.props.onCheck(this.props.issue.key); - return false; + return this.props.onCheck(this.props.issue.key); } - return undefined; - }); - } - - unbindShortcuts() { - key.unbind('f', 'issues'); - key.unbind('a', 'issues'); - key.unbind('m', 'issues'); - key.unbind('i', 'issues'); - key.unbind('c', 'issues'); - key.unbind('space', 'issues'); - key.unbind('t', 'issues'); - } + } + return true; + }; togglePopup = (popupName: string, open?: boolean) => { this.props.onPopupToggle(this.props.issue.key, popupName, open); diff --git a/server/sonar-web/src/main/js/components/issue/__tests__/__snapshots__/issue-test.tsx.snap b/server/sonar-web/src/main/js/components/issue/__tests__/__snapshots__/issue-test.tsx.snap new file mode 100644 index 00000000000..9b2ad3a1112 --- /dev/null +++ b/server/sonar-web/src/main/js/components/issue/__tests__/__snapshots__/issue-test.tsx.snap @@ -0,0 +1,60 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`should render issues correctly 1`] = ` + +`; diff --git a/server/sonar-web/src/main/js/components/issue/__tests__/issue-test.tsx b/server/sonar-web/src/main/js/components/issue/__tests__/issue-test.tsx new file mode 100644 index 00000000000..85bfbe69d00 --- /dev/null +++ b/server/sonar-web/src/main/js/components/issue/__tests__/issue-test.tsx @@ -0,0 +1,107 @@ +/* + * SonarQube + * Copyright (C) 2009-2022 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +import { shallow } from 'enzyme'; +import * as React from 'react'; +import { KeyboardKeys } from '../../../helpers/keycodes'; +import { mockIssue } from '../../../helpers/testMocks'; +import { keydown } from '../../../helpers/testUtils'; +import Issue from '../Issue'; + +it('should render issues correctly', () => { + expect(shallowRender()).toMatchSnapshot(); +}); + +it('should call the proper function with the proper props when pressing shortcuts (FAMICT)', () => { + const onPopupToggle = jest.fn(); + const onCheck = jest.fn(); + const issue = mockIssue(false, { + comments: [ + { + key: '1', + htmlText: 'My comment', + markdown: 'My comment', + updatable: false, + createdAt: '2017-07-05T09:33:29+0200', + author: 'admin', + authorLogin: 'admin', + authorName: 'Admin', + authorAvatar: 'admin-avatar', + authorActive: true + } + ], + actions: ['assign'] + }); + + shallowRender({ onPopupToggle, issue, onCheck }); + keydown({ key: KeyboardKeys.KeyF }); + expect(onPopupToggle).toBeCalledWith(issue.key, 'transition', undefined); + + keydown({ key: KeyboardKeys.KeyA }); + expect(onPopupToggle).toBeCalledWith(issue.key, 'assign', undefined); + keydown({ key: KeyboardKeys.Escape }); + + keydown({ key: KeyboardKeys.KeyM }); + expect(onPopupToggle).toBeCalledWith(issue.key, 'assign', false); + + keydown({ key: KeyboardKeys.KeyI }); + expect(onPopupToggle).toBeCalledWith(issue.key, 'set-severity', undefined); + + keydown({ key: KeyboardKeys.KeyC }); + expect(onPopupToggle).toBeCalledWith(issue.key, 'comment', undefined); + keydown({ key: KeyboardKeys.Escape }); + + keydown({ key: KeyboardKeys.KeyT }); + expect(onPopupToggle).toBeCalledWith(issue.key, 'edit-tags', undefined); + + keydown({ key: KeyboardKeys.Space }); + expect(onCheck).toBeCalledWith(issue.key); +}); + +function shallowRender(props: Partial = {}) { + return shallow( + + ); +} diff --git a/server/sonar-web/src/main/js/helpers/keycodes.ts b/server/sonar-web/src/main/js/helpers/keycodes.ts index dd6cbf28114..eb3229aebd2 100644 --- a/server/sonar-web/src/main/js/helpers/keycodes.ts +++ b/server/sonar-web/src/main/js/helpers/keycodes.ts @@ -37,5 +37,13 @@ export enum KeyboardCodes { } export enum KeyboardKeys { - Alt = 'Alt' + Alt = 'Alt', + KeyF = 'f', + KeyA = 'a', + KeyM = 'm', + KeyI = 'i', + KeyC = 'c', + KeyT = 't', + Space = ' ', + Escape = 'Escape' } -- 2.39.5