Skip to content

Migrate out of react-hotkeys to @mantine/hooks #2968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
May 3, 2024
Merged
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@
"react-drag-drop-files": "^2.3.10",
"react-draggable": "^4.4.5",
"react-dropzone": "^14.2.3",
"react-hotkeys": "^2.0.0",
"react-i18next": "^14.1.0",
"react-konva": "^18.2.10",
"react-latex-next": "^3.0.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ exports[`AssessmentWorkspace AssessmentWorkspace page with ContestVoting questio
>
<div
class="Editor bp5-card bp5-elevation-0"
tabindex="-1"
>
<div
class="row editor-react-ace"
Expand Down Expand Up @@ -1236,7 +1235,6 @@ exports[`AssessmentWorkspace AssessmentWorkspace page with ContestVoting questio
>
<div
class="repl-input-parent row bp5-card bp5-elevation-0"
tabindex="-1"
>
<div
class=" ace_editor ace_hidpi ace-source ace_dark repl-react-ace react-ace"
Expand Down Expand Up @@ -2021,7 +2019,6 @@ exports[`AssessmentWorkspace AssessmentWorkspace page with MCQ question renders
>
<div
class="repl-input-parent row bp5-card bp5-elevation-0"
tabindex="-1"
>
<div
class=" ace_editor ace_hidpi ace-source ace_dark repl-react-ace react-ace"
Expand Down Expand Up @@ -2354,7 +2351,6 @@ exports[`AssessmentWorkspace AssessmentWorkspace page with overdue assessment re
>
<div
class="Editor bp5-card bp5-elevation-0"
tabindex="-1"
>
<div
class="row editor-react-ace"
Expand Down Expand Up @@ -2841,7 +2837,6 @@ exports[`AssessmentWorkspace AssessmentWorkspace page with overdue assessment re
>
<div
class="repl-input-parent row bp5-card bp5-elevation-0"
tabindex="-1"
>
<div
class=" ace_editor ace_hidpi ace-source ace_dark repl-react-ace react-ace"
Expand Down Expand Up @@ -3204,7 +3199,6 @@ exports[`AssessmentWorkspace AssessmentWorkspace page with programming question
>
<div
class="Editor bp5-card bp5-elevation-0"
tabindex="-1"
>
<div
class="row editor-react-ace"
Expand Down Expand Up @@ -3691,7 +3685,6 @@ exports[`AssessmentWorkspace AssessmentWorkspace page with programming question
>
<div
class="repl-input-parent row bp5-card bp5-elevation-0"
tabindex="-1"
>
<div
class=" ace_editor ace_hidpi ace-source ace_dark repl-react-ace react-ace"
Expand Down Expand Up @@ -4054,7 +4047,6 @@ exports[`AssessmentWorkspace AssessmentWorkspace renders Grading tab correctly i
>
<div
class="Editor bp5-card bp5-elevation-0"
tabindex="-1"
>
<div
class="row editor-react-ace"
Expand Down Expand Up @@ -4759,7 +4751,6 @@ exports[`AssessmentWorkspace AssessmentWorkspace renders Grading tab correctly i
>
<div
class="repl-input-parent row bp5-card bp5-elevation-0"
tabindex="-1"
>
<div
class=" ace_editor ace_hidpi ace-source ace_dark repl-react-ace react-ace"
Expand Down
16 changes: 3 additions & 13 deletions src/commons/editor/Editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,13 @@ import 'ace-builds/src-noconflict/ext-searchbox';
import 'ace-builds/src-noconflict/ext-settings_menu';
import 'js-slang/dist/editors/ace/theme/source';

import { Classes } from '@blueprintjs/core';
import { Card } from '@blueprintjs/core';
import * as AceBuilds from 'ace-builds';
import { Ace, require as acequire, createEditSession } from 'ace-builds';
import classNames from 'classnames';
import { Chapter, Variant } from 'js-slang/dist/types';
import React from 'react';
import AceEditor, { IAceEditorProps, IEditorProps } from 'react-ace';
import { IAceEditor } from 'react-ace/lib/types';
import { HotKeys } from 'react-hotkeys';
import { EditorBinding } from '../WorkspaceSettingsContext';
import { getModeString, selectMode } from '../utils/AceHelper';
import { objectEntries } from '../utils/TypeHelper';
Expand Down Expand Up @@ -327,11 +325,6 @@ const moveCursor = (editor: AceEditor['editor'], position: Position) => {
editor.renderer.scrollCursorIntoView(position, 0.5);
};

/* Override handler, so does not trigger when focus is in editor */
const handlers = {
goGreen: () => {}
};

const EditorBase = React.memo((props: EditorProps & LocalStateProps) => {
const reactAceRef: React.MutableRefObject<AceEditor | null> = React.useRef(null);
const [filePath, setFilePath] = React.useState<string | undefined>(undefined);
Expand Down Expand Up @@ -652,14 +645,11 @@ const EditorBase = React.memo((props: EditorProps & LocalStateProps) => {
}, []);

return (
<HotKeys
className={classNames('Editor', Classes.CARD, Classes.ELEVATION_0)}
handlers={handlers}
>
<Card className="Editor">
<div className="row editor-react-ace" data-testid="Editor">
<AceEditor {...aceEditorProps} ref={reactAceRef} />
</div>
</HotKeys>
</Card>
);
});

Expand Down
39 changes: 39 additions & 0 deletions src/commons/hotkeys/HotKeys.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { getHotkeyHandler, HotkeyItem } from '@mantine/hooks';
import React, { PropsWithChildren } from 'react';

type HotKeysProps = {
bindings: HotkeyItem[];
};

/**
* This HOC was created to facilitate the migration out of react-hotkeys in favor of @mantine/hooks useHotkeys,
* as SideContentCseMachine.tsx and SideContentDataVisualizer still use class-based React.
*
* NOTE:
* - New hotkey implementations should NOT use this component. Use functional React and the useHotkeys hook
* from @mantine/hooks directly.
*
* TODO:
* - Eventually migrate out of class-based React in the aforementioned components and use useHotkeys directly.
*/
const HotKeys: React.FC<
PropsWithChildren<
HotKeysProps & {
style?: React.CSSProperties;
}
>
> = ({ bindings, children, style }) => {
const handler = getHotkeyHandler(bindings);

return (
<div
tabIndex={-1} // tab index necessary to fire keydown events on div element
onKeyDown={handler}
style={style}
>
{children}
</div>
);
};

export default HotKeys;
15 changes: 3 additions & 12 deletions src/commons/repl/Repl.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { Card, Classes, Pre } from '@blueprintjs/core';
import { Card, Pre } from '@blueprintjs/core';
import { Ace } from 'ace-builds';
import classNames from 'classnames';
import { parseError } from 'js-slang';
import { Chapter, Variant } from 'js-slang/dist/types';
import React from 'react';
import { HotKeys } from 'react-hotkeys';

import { InterpreterOutput } from '../application/ApplicationTypes';
import { ExternalLibraryName } from '../application/types/ExternalTypes';
Expand Down Expand Up @@ -52,12 +51,9 @@ const Repl: React.FC<ReplProps> = props => {
<div className="repl-output-parent">
{cards}
{!props.inputHidden && (
<HotKeys
className={classNames('repl-input-parent', 'row', Classes.CARD, Classes.ELEVATION_0)}
handlers={handlers}
>
<Card className={classNames('repl-input-parent', 'row')}>
<ReplInput {...props} />
</HotKeys>
</Card>
)}
</div>
</div>
Expand Down Expand Up @@ -133,9 +129,4 @@ export const Output: React.FC<OutputProps> = props => {
}
};

/* Override handler, so does not trigger when focus is in editor */
const handlers = {
goGreen: () => {}
};

export default Repl;
13 changes: 5 additions & 8 deletions src/commons/repl/__tests__/__snapshots__/Repl.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,10 @@ exports[`Repl renders correctly 1`] = `
}
usingSubst={false}
/>
<HotKeysEnabled
className="repl-input-parent row bp5-card bp5-elevation-0"
handlers={
Object {
"goGreen": [Function],
}
}
<Blueprint5.Card
className="repl-input-parent row"
elevation={0}
interactive={false}
>
<ReplInput
externalLibrary="NONE"
Expand Down Expand Up @@ -201,7 +198,7 @@ exports[`Repl renders correctly 1`] = `
sourceChapter={1}
sourceVariant="default"
/>
</HotKeysEnabled>
</Blueprint5.Card>
</div>
</div>
`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,14 @@

exports[`CSE Machine component renders correctly 1`] = `
<div
onBlur={[Function]}
onFocus={[Function]}
onKeyDown={[Function]}
onKeyPress={[Function]}
onKeyUp={[Function]}
style={
Object {
"maxHeight": "100%",
"overflow": "auto",
}
}
tabIndex="-1"
tabIndex={-1}
>
<div
className="sa-substituter bp5-dark"
Expand Down
39 changes: 16 additions & 23 deletions src/commons/sideContent/content/SideContentCseMachine.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@ import {
Tooltip
} from '@blueprintjs/core';
import { IconNames } from '@blueprintjs/icons';
import { HotkeyItem } from '@mantine/hooks';
import classNames from 'classnames';
import { Chapter } from 'js-slang/dist/types';
import { debounce } from 'lodash';
import React from 'react';
import { HotKeys } from 'react-hotkeys';
import { connect, MapDispatchToProps, MapStateToProps } from 'react-redux';
import { bindActionCreators } from 'redux';
import HotKeys from 'src/commons/hotkeys/HotKeys';
import { Output } from 'src/commons/repl/Repl';
import type { PlaygroundWorkspaceState } from 'src/commons/workspace/WorkspaceTypes';
import CseMachine from 'src/features/cseMachine/CseMachine';
Expand Down Expand Up @@ -73,13 +74,6 @@ type DispatchProps = {
handleAlertSideContent: () => void;
};

const cseMachineKeyMap = {
FIRST_STEP: 'a',
NEXT_STEP: 'f',
PREVIOUS_STEP: 'b',
LAST_STEP: 'e'
};

class SideContentCseMachineBase extends React.Component<CseMachineProps, State> {
constructor(props: CseMachineProps) {
super(props);
Expand Down Expand Up @@ -200,24 +194,23 @@ class SideContentCseMachineBase extends React.Component<CseMachineProps, State>
}

public render() {
const cseMachineHandlers = this.state.visualization
? {
FIRST_STEP: this.stepFirst,
NEXT_STEP: this.stepNext,
PREVIOUS_STEP: this.stepPrevious,
LAST_STEP: this.stepLast(this.props.stepsTotal)
}
: {
FIRST_STEP: () => {},
NEXT_STEP: () => {},
PREVIOUS_STEP: () => {},
LAST_STEP: () => {}
};
const hotkeyBindings: HotkeyItem[] = this.state.visualization
? [
['a', this.stepFirst],
['f', this.stepNext],
['b', this.stepPrevious],
['e', this.stepLast(this.props.stepsTotal)]
]
: [
['a', () => {}],
['f', () => {}],
['b', () => {}],
['e', () => {}]
];
Comment on lines +197 to +209
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While not related to this PR, it it just be or are the keyboard shortcuts not intuitive? I would expect the following:

(first, prev, next, last) to be mapped to (h, j, k, l) or (a, s, d, f) -- basically just a sequence of keys that are next to each other on the keyboard.


return (
<HotKeys
keyMap={cseMachineKeyMap}
handlers={cseMachineHandlers}
bindings={hotkeyBindings}
style={{
maxHeight: '100%',
overflow: this.state.visualization ? 'hidden' : 'auto'
Expand Down
Loading