Skip to content
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

Shortcuts investigation #489

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions plugin-hrm-form/src/HrmFormPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import setUpMonitoring from './utils/setUpMonitoring';
import * as TransferHelpers from './utils/transfer';
import { changeLanguage } from './states/configuration/actions';
import { issueSyncToken } from './services/ServerlessService';
import CannedResponses from './components/CannedResponses';
import KeyboardShortcutManager from './components/KeyboardShortcut/KeyboardShortcutManager';

const PLUGIN_NAME = 'HrmFormPlugin';
export const PLUGIN_VERSION = '0.10.0';
Expand All @@ -26,6 +26,8 @@ export const DEFAULT_TRANSFER_MODE = transferModes.cold;
*/
let sharedStateClient;

let shortcutManager;

export const getConfig = () => {
const manager = Flex.Manager.getInstance();

Expand Down Expand Up @@ -74,6 +76,7 @@ export const getConfig = () => {
pdfImagesSource,
multipleOfficeSupport,
permissionConfig,
shortcutManager,
};
};

Expand Down Expand Up @@ -175,6 +178,7 @@ const setUpComponents = setupObject => {
const { helpline, featureFlags } = setupObject;

// setUp (add) dynamic components
Components.setUpShortcuts();
Components.setUpQueuesStatusWriter(setupObject);
Components.setUpQueuesStatus();
Components.setUpAddButtons(setupObject);
Expand Down Expand Up @@ -255,9 +259,9 @@ export default class HrmFormPlugin extends FlexPlugin {

console.log(`Welcome to ${PLUGIN_NAME} Version ${PLUGIN_VERSION}`);
this.registerReducers(manager);
shortcutManager = new KeyboardShortcutManager(manager);

const config = getConfig();

/*
* localization setup (translates the UI if necessary)
* WARNING: the way this is done right now is "hacky". More info in initLocalization declaration
Expand Down
3 changes: 3 additions & 0 deletions plugin-hrm-form/src/___tests__/mockStyled.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ jest.mock('../styles/HrmStyles', () => ({
PopoverText: 'PopoverText',
CannedResponsesContainer: 'CannedResponsesContainer',
Bold: 'Bold',
StyledBackButton: 'StyledBackButton',
}));

jest.mock('../styles/search', () => ({
Expand All @@ -94,6 +95,8 @@ jest.mock('../styles/search', () => ({
ContactDetailsIcon: () => 'ContactDetailsIcon',
DetailsContainer: 'DetailsContainer',
SectionTitleContainer: 'SectionTitleContainer',
SectionTitleButton: 'SectionTitleButton',
SectionCollapse: 'SectionCollapse',
NameContainer: 'NameContainer',
BackText: 'BackText',
DetNameText: 'DetNameText',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { connect } from 'react-redux';
import { bindActionCreators } from 'redux';

import { namespace, shortcutBase, RootState } from '../../states';
import { updatePressedKeys, openGuideModal, closeGuideModal } from '../../states/shortcuts/actions';
import KeyboardShortcut from './KeyboardShortcut';

/**
* We're not using the pattern Container/Component in the rest of the app.
* So I suggest we don't do this here. We should unify KeyboarShortcut.Container and
* KeyboardShortcut to be a single component/file.
*/
const mapStateToProps = (state: RootState) => ({
pressedKeys: state[namespace][shortcutBase].pressedKeys,
isGuideModalOpen: state[namespace][shortcutBase].isGuideModalOpen,
});

const mapDispatchToProps = dispatch => ({
updatePressedKeys: bindActionCreators(updatePressedKeys, dispatch),
openGuideModal: bindActionCreators(openGuideModal, dispatch),
closeGuideModal: bindActionCreators(closeGuideModal, dispatch),
});

export default connect(mapStateToProps, mapDispatchToProps)(KeyboardShortcut);
127 changes: 127 additions & 0 deletions plugin-hrm-form/src/components/KeyboardShortcut/KeyboardShortcut.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import React, { Component } from 'react';
import { Dialog, DialogContent, DialogTitle, List, ListItem, ListItemText, Typography } from '@material-ui/core';

import { KeyBoardShortcutRule } from './KeyboardShortcutManager';
import { keyboardGuide } from './KeyboardShortcutGuide';
import { PressedKeys } from '../../states/shortcuts/types';
import { getConfig } from '../../HrmFormPlugin';

type OwnProps = {
shortcuts: KeyBoardShortcutRule[];
};

type ReduxProps = {
shortcuts: KeyBoardShortcutRule[];
pressedKeys: PressedKeys;
isGuideModalOpen: boolean;
updatePressedKeys: (pressedKeys: PressedKeys) => void;
closeGuideModal: () => void;
};

type Props = OwnProps & ReduxProps;

// TODO: Make this a pure function component
class KeyboardShortcut extends Component<Props> {
static displayName = 'KeyboardShortcut';

componentDidMount() {
window.addEventListener('keydown', this.keyboardEventListener, true);
window.addEventListener('keyup', this.keyboardEventListener, true);

const initialKeys = this.props.shortcuts.reduce<{ [key: string]: boolean }>((result, curr) => {
curr.keys.forEach(key => (result[key] = false));
return result;
}, {});

this.props.updatePressedKeys(initialKeys);
}

componentWillUnmount() {
window.removeEventListener('keydown', this.keyboardEventListener);
window.removeEventListener('keyup', this.keyboardEventListener);
}

keyboardEventListener = (e: KeyboardEvent) => {
/**
* Here we use event.pressedKey to determine the key the user has typed.
* If our shortcuts contains Shift or Alt, this pressed key value will be secondary
* or terciary value of that key. Example: 3, #, £. I'm not sure if those secondary
* and terciary values are consistent between different keyboards.
*
* One alternative to investigate is to use event.keyCode instead, that keeps the
* same value, regardless if we're pressing Shift or Alt with it.
*/
const { key: pressedKey, type: eventType, repeat } = e;
const { tagName } = e.target as Element;
console.log({ e, pressedKey });

const { shortcutManager } = getConfig();

/**
* Shortcuts should not be single key. When we change them to be like 'Control + Command + Letter',
* we can remove the bellow condition.
*/
// Check user is not inputting text
if (['INPUT', 'TEXTAREA', 'SELECT'].includes(tagName)) {
return;
}

// Check that this is a non-repeating key event
if (repeat) return;

if (eventType === 'keydown') {
for (const { keys, action } of shortcutManager.shortcuts) {
if (keys.every(key => key === pressedKey || this.props.pressedKeys[key])) {
action();
break;
}
}
}

/**
* This part has an issue.
* How it should work?
* After a keydown event is detected, the related key is marked as TRUE on redux.
* After the key is released, the keyup event for this key should go into this if condition, and thus,
* this key will become FALSE on redux.
*
* What's the issue?
* Sometimes the keyup event is not detected. For single key shortcuts, it looks like this issue never arises.
* But for multiple keys short, such as 'Control + Command + M', if the action triggers things like an alert or a dialog,
* the keyup event for the letter 'M' is not detected, and the 'M' key remains TRUE on redux, making the app behave wrong.
* For some reason, the Control and Command keyup events seem to be always detected, but not the keyup events for letters.
*/
if (this.props.pressedKeys[pressedKey] !== undefined) {
const updatedPressedKeys = { ...this.props.pressedKeys, [pressedKey]: eventType === 'keydown' };
this.props.updatePressedKeys(updatedPressedKeys);
}
};

render() {
return (
<Dialog
open={this.props.isGuideModalOpen}
onClose={this.props.closeGuideModal}
fullWidth
maxWidth="sm"
onBackdropClick={this.props.closeGuideModal}
>
<DialogTitle>Keyboard Shortcuts Help</DialogTitle>
<DialogContent>
<List>
{keyboardGuide.map((shortcut, i) => {
return (
<ListItem divider={i !== keyboardGuide.length - 1} key={shortcut.description}>
<ListItemText>{shortcut.description}</ListItemText>
<Typography align="right">{shortcut.keys}</Typography>
</ListItem>
);
})}
</List>
</DialogContent>
</Dialog>
);
}
}

export default KeyboardShortcut;
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* This part is completely hardcoded and centralized.
* Should we make it more dynamically, in a way that we wouldn't
* need this file at all?
*
* For example: When calling the addShorcut() method, we could pass its description as a parameter.
* That way it will be easier to keep the keyboard guide up to date. And the 'keys' part could be
* built automatically filled (or overwritten if necessary for accessibility reasons).
*/
export const keyboardGuide = [
{ description: 'Toggle keyboard guide', keys: '?' },
{ description: 'Toggle sidebar', keys: 'M' },
{ description: 'Toggle availability', keys: 'O' },
{ description: 'Open standalone search', keys: 'S' },
{ description: 'Open agent desktop', keys: 'A' },
{ description: 'Choose Tab', keys: '0/1/2/3/4 (tab number)' },
];
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import * as Flex from '@twilio/flex-ui';

import { openGuideModal, closeGuideModal } from '../../states/shortcuts/actions';
import { namespace, shortcutBase } from '../../states';

export interface KeyBoardShortcutRule {
keys: string[];
action: () => void;
}

/**
* Suggestion: we should make the methods of this class to be arrow functions,
* so that we don't need to bind the instance to use the correct 'this' value.
*
* Right now, we're using these methods like this: shortcutManager.toggleGuide.bind(shortcutManager).
* Notice the '.bind(shortcutManager)'. We can avoid that if we make these methods arrow functions.
*/
class KeyboardShortcutManager {
private manager: Flex.Manager;

public shortcuts: KeyBoardShortcutRule[];

constructor(manager: Flex.Manager) {
this.manager = manager;
this.shortcuts = [];
}

public addShortcut(keys: string[], action: () => void) {
this.shortcuts = [...this.shortcuts, { keys, action }];
}

public toggleGuide() {
if (this.manager.store.getState()[namespace][shortcutBase].isGuideModalOpen) {
this.manager.store.dispatch(closeGuideModal());
} else {
this.manager.store.dispatch(openGuideModal());
}
}

public toggleSidebar() {
Flex.Actions.invokeAction('ToggleSidebar');
}

public toggleAvailability() {
const { activity } = this.manager.store.getState().flex.worker;
if (activity.name === 'Offline' || activity.name === 'Unavailable') {
Flex.Actions.invokeAction('SetActivity', { activityAvailable: true, activityName: 'Available' });
}
if (activity.name === 'Available') {
Flex.Actions.invokeAction('SetActivity', { activityAvailable: false, activityName: 'Unavailable' });
}
}

public openStandaloneSearch() {
Flex.Actions.invokeAction('NavigateToView', { viewName: 'search' });
}

public openAgentDesktop() {
Flex.Actions.invokeAction('NavigateToView', { viewName: 'agent-desktop' });
}
}

export default KeyboardShortcutManager;
17 changes: 11 additions & 6 deletions plugin-hrm-form/src/components/Section.jsx
Original file line number Diff line number Diff line change
@@ -1,24 +1,29 @@
import React from 'react';
import PropTypes from 'prop-types';
import { ButtonBase, Collapse } from '@material-ui/core';
import { ArrowDropDownTwoTone, ArrowDropUpTwoTone } from '@material-ui/icons';

import { SectionTitleContainer, SectionTitleText, ContactDetailsIcon } from '../styles/search';
import {
SectionTitleContainer,
SectionTitleButton,
SectionTitleText,
SectionCollapse,
ContactDetailsIcon,
} from '../styles/search';

const ArrowDownIcon = ContactDetailsIcon(ArrowDropDownTwoTone);
const ArrowUpIcon = ContactDetailsIcon(ArrowDropUpTwoTone);

const Section = ({ color, sectionTitle, expanded, hideIcon, children, handleExpandClick, buttonDataTestid }) => (
<>
<SectionTitleContainer color={color}>
<ButtonBase style={{ width: '100%', padding: 0 }} onClick={handleExpandClick} data-testid={buttonDataTestid}>
<SectionTitleButton onClick={handleExpandClick} data-testid={buttonDataTestid}>
<SectionTitleText>{sectionTitle}</SectionTitleText>
{!hideIcon && (expanded ? <ArrowUpIcon /> : <ArrowDownIcon />)}
</ButtonBase>
</SectionTitleButton>
</SectionTitleContainer>
<Collapse in={expanded} timeout="auto">
<SectionCollapse expanded={expanded} timeout="auto">
{children}
</Collapse>
</SectionCollapse>
</>
);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
/* eslint-disable react/prop-types */
import React from 'react';
import { ButtonBase } from '@material-ui/core';

import { Row } from '../../../styles/HrmStyles';
import { Row, StyledBackButton } from '../../../styles/HrmStyles';
import { BackText, BackIcon } from '../../../styles/search';

type OwnProps = {
Expand All @@ -15,12 +14,12 @@ type Props = OwnProps;
const SearchResultsBackButton: React.FC<Props> = ({ text, handleBack }) => {
return (
<Row>
<ButtonBase onClick={handleBack}>
<StyledBackButton onClick={handleBack}>
<Row>
<BackIcon />
<BackText>{text}</BackText>
</Row>
</ButtonBase>
</StyledBackButton>
</Row>
);
};
Expand Down
5 changes: 4 additions & 1 deletion plugin-hrm-form/src/components/tabbedForms/TabbedForms.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import BottomBar from './BottomBar';
import { hasTaskControl } from '../../utils/transfer';
import { isNonDataCallType } from '../../states/ValidationRules';
import SearchResultsBackButton from '../search/SearchResults/SearchResultsBackButton';
import { isStandaloneITask } from '../case/Case';
import { getConfig } from '../../HrmFormPlugin';

// eslint-disable-next-line react/display-name
const mapTabsComponents = (errors: any) => (t: TabbedFormSubroutes) => {
Expand Down Expand Up @@ -126,6 +126,9 @@ const TabbedForms: React.FC<Props> = ({ dispatch, routing, task, contactForm, cu
dispatch(changeRoute({ route: 'tabbed-forms', subroute: tab }, taskId));
};

const { shortcutManager } = getConfig();
tabsToIndex.forEach((_, index) => shortcutManager.addShortcut([String(index)], () => handleTabsChange(null, index)));

const { subroute } = routing;
let tabIndex = tabsToIndex.findIndex(t => t === subroute);

Expand Down
Loading