Skip to content

Commit 38a916d

Browse files
committed
keyboard: simplify clipboard internals to enable future kb navigation
Until this commit, the Clipboard implementation relied on an always-focused hidden textarea element. This had a few benefits: - makes it easy to handle the "input" command, - prevents browser-quirks about copy/paste events. The downside were: - it makes it hard to handle usual browser keyboard navigation (with tab, arrow keys, etc.). For now, this default navigation is overriden anyway with app-specific shortcuts so we don't care much. But it makes future improvements about that difficult. - it makes screen reader support difficult. As basically any interact focuses back to one dummy element, this is an actual barrier to any future work on this. In modern day browser APIs, the copy/paste quirks are not there anymore, so the need to go around them is no more. (actually, not 100% sure yet, I'm testing this more now) This commit starts some ground work to stop relying on an hidden input, making it possible then to work on more complete keyboard navigation, and eventually actual screen reader support. This still doesn't work really great, there are a few @todo marked in the comments.
1 parent 1a527d7 commit 38a916d

File tree

3 files changed

+138
-50
lines changed

3 files changed

+138
-50
lines changed

app/client/components/Clipboard.js

Lines changed: 64 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,11 @@
22
* Clipboard component manages the copy/cut/paste events by capturing these events from the browser,
33
* managing their state, and exposing an API to other components to get/set the data.
44
*
5-
* Because of a lack of standardization of ClipboardEvents between browsers, the way Clipboard
6-
* captures the events is by creating a hidden textarea element that's always focused with some text
7-
* selected. Here is a good write-up of this:
8-
* https://www.lucidchart.com/techblog/2014/12/02/definitive-guide-copying-pasting-javascript/
9-
*
105
* When ClipboardEvent is detected, Clipboard captures the event and calls the corresponding
116
* copy/cut/paste/input command actions, which will get called on the appropriate component.
127
*
8+
* The Clipboard also handles triggering correctly the "input" command when any key is pressed.
9+
*
1310
* Usage:
1411
* Components need to register copy/cut/paste actions with command.js:
1512
* .copy() should return @pasteObj (defined below).
@@ -45,7 +42,6 @@ var {confirmModal} = require('app/client/ui2018/modals');
4542
var {styled} = require('grainjs');
4643

4744
var commands = require('./commands');
48-
var dom = require('../lib/dom');
4945
var Base = require('./Base');
5046
var tableUtil = require('../lib/tableUtil');
5147

@@ -54,27 +50,10 @@ const t = makeT('Clipboard');
5450
function Clipboard(app) {
5551
Base.call(this, null);
5652
this._app = app;
57-
this.copypasteField = this.autoDispose(dom('textarea.copypaste.mousetrap', ''));
58-
this.timeoutId = null;
59-
60-
this.onEvent(this.copypasteField, 'input', function(elem, event) {
61-
var value = elem.value;
62-
elem.value = '';
63-
commands.allCommands.input.run(value);
64-
return false;
65-
});
66-
this.onEvent(this.copypasteField, 'copy', this._onCopy);
67-
this.onEvent(this.copypasteField, 'cut', this._onCut);
68-
this.onEvent(this.copypasteField, 'paste', this._onPaste);
69-
70-
document.body.appendChild(this.copypasteField);
7153

7254
FocusLayer.create(this, {
73-
defaultFocusElem: this.copypasteField,
74-
allowFocus: allowFocus,
55+
defaultFocusElem: document.body,
7556
onDefaultFocus: () => {
76-
this.copypasteField.value = ' ';
77-
this.copypasteField.select();
7857
this._app.trigger('clipboard_focus');
7958
},
8059
onDefaultBlur: () => {
@@ -94,6 +73,32 @@ function Clipboard(app) {
9473
}
9574
});
9675

76+
// Listen for copy/cut/paste events and trigger the corresponding clipboard action.
77+
// Note that internally, before triggering the action, we check if the currently active element
78+
// doesn't already handle these events itself.
79+
// This allows to globally handle copy/cut/paste events, without impacting
80+
// inputs/textareas/selects where copy/cut/paste events should be left alone.
81+
this.onEvent(document, 'copy', (_, event) => this._onCopy(event));
82+
this.onEvent(document, 'cut', (_, event) => this._onCut(event));
83+
this.onEvent(document, 'paste', (_, event) => this._onPaste(event));
84+
85+
// when typing a random printable character while not focusing an interactive element,
86+
// trigger the input command with it
87+
// @TODO: there is currently an issue, sometimes when typing something, that makes us focus a cell textarea,
88+
// and then we can mouseclick on a different cell: dom focus is still on textarea, visual table focus is on new cell.
89+
this.onEvent(document.body, 'keydown', (_, event) => {
90+
if (shouldAvoidClipboardShortcuts(document.activeElement)) {
91+
return;
92+
}
93+
const ev = event.originalEvent;
94+
const collapsesWithCommands = commands.keypressCollapsesWithExistingCommand(ev);
95+
const isPrintableCharacter = commands.keypressIsPrintableCharacter(ev);
96+
if (!collapsesWithCommands && isPrintableCharacter) {
97+
commands.allCommands.input.run(ev.key);
98+
event.preventDefault();
99+
}
100+
});
101+
97102
// In the event of a cut a callback is provided by the viewsection that is the target of the cut.
98103
// When called it returns the additional removal action needed for a cut.
99104
this._cutCallback = null;
@@ -116,7 +121,10 @@ Clipboard.commands = {
116121
* Internal helper fired on `copy` events. If a callback was registered from a component, calls the
117122
* callback to get selection data and puts it on the clipboard.
118123
*/
119-
Clipboard.prototype._onCopy = function(elem, event) {
124+
Clipboard.prototype._onCopy = function(event) {
125+
if (shouldAvoidClipboardShortcuts(document.activeElement)) {
126+
return;
127+
}
120128
event.preventDefault();
121129

122130
let pasteObj = commands.allCommands.copy.run();
@@ -136,7 +144,11 @@ Clipboard.prototype._doContextMenuCopyWithHeaders = function() {
136144
this._copyToClipboard(pasteObj, 'copy', true);
137145
};
138146

139-
Clipboard.prototype._onCut = function(elem, event) {
147+
Clipboard.prototype._onCut = function(event) {
148+
if (shouldAvoidClipboardShortcuts(document.activeElement)) {
149+
return;
150+
}
151+
140152
event.preventDefault();
141153

142154
let pasteObj = commands.allCommands.cut.run();
@@ -211,7 +223,11 @@ Clipboard.prototype._setCutCallback = function(pasteObj, cutData) {
211223
* Internal helper fired on `paste` events. If a callback was registered from a component, calls the
212224
* callback with data from the clipboard.
213225
*/
214-
Clipboard.prototype._onPaste = function(elem, event) {
226+
Clipboard.prototype._onPaste = function(event) {
227+
if (shouldAvoidClipboardShortcuts(document.activeElement)) {
228+
return;
229+
}
230+
215231
event.preventDefault();
216232
const cb = event.originalEvent.clipboardData;
217233
const plainText = cb.getData('text/plain');
@@ -220,12 +236,6 @@ Clipboard.prototype._onPaste = function(elem, event) {
220236
this._doPaste(pasteData, plainText);
221237
};
222238

223-
var FOCUS_TARGET_TAGS = {
224-
'INPUT': true,
225-
'TEXTAREA': true,
226-
'SELECT': true,
227-
'IFRAME': true,
228-
};
229239

230240
Clipboard.prototype._doContextMenuPaste = async function() {
231241
let clipboardItem;
@@ -293,6 +303,17 @@ async function getTextFromClipboardItem(clipboardItem, type) {
293303
}
294304
}
295305

306+
const CLIPBOARD_TAGS = {
307+
'INPUT': true,
308+
'TEXTAREA': true,
309+
'SELECT': true,
310+
};
311+
312+
const FOCUS_TARGET_TAGS = {
313+
...CLIPBOARD_TAGS,
314+
'IFRAME': true,
315+
};
316+
296317
/**
297318
* Helper to determine if the currently active element deserves to keep its own focus, and capture
298319
* copy-paste events. Besides inputs and textareas, any element can be marked to be a valid
@@ -304,6 +325,16 @@ function allowFocus(elem) {
304325
elem.classList.contains('clipboard_focus'));
305326
}
306327

328+
/**
329+
* Helper to determine if the given element is a valid target for copy-cut-paste actions.
330+
*
331+
* It slightly differs from allowFocus: here we exclusively check for clipboard-related actions,
332+
* not focus-related ones.
333+
*/
334+
function shouldAvoidClipboardShortcuts(elem) {
335+
return elem && CLIPBOARD_TAGS.hasOwnProperty(elem.tagName)
336+
}
337+
307338
Clipboard.allowFocus = allowFocus;
308339

309340
function showUnavailableMenuCommandModal(action) {

app/client/components/commands.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,77 @@ export const allCommands: { [key in CommandName]: Command } = {} as any;
5353
*/
5454
const _allKeys: Record<string, CommandGroup[]> = {};
5555

56+
export const allShortcuts: Array<string> = [];
57+
58+
const saveShortcut = (key: string | string[]) => {
59+
const keyString = typeof key === 'string' ? key.toLowerCase() : key.join('+').toLowerCase();
60+
if (keyString === "+" || !keyString.includes('+')) {
61+
allShortcuts.push(keyString);
62+
} else {
63+
const splitKeys = keyString.split('+');
64+
allShortcuts.push(splitKeys.slice(0, -1).sort().join('+') + '+' + splitKeys[splitKeys.length - 1]);
65+
}
66+
};
67+
68+
const _keyAliases: Record<string, string> = {
69+
'return': 'enter',
70+
'esc': 'escape',
71+
'+': 'plus',
72+
' ': 'space',
73+
};
74+
75+
/**
76+
* Helper to know if a given keypress matches an existing command shortcut.
77+
*
78+
* Helpful when we want to handle specific keypresses without interfering with our commands.
79+
*/
80+
export const keypressCollapsesWithExistingCommand = (event: KeyboardEvent) => {
81+
let key = event.key.toLowerCase();
82+
if (_keyAliases[key]) {
83+
key = _keyAliases[key];
84+
}
85+
const modifiers = [];
86+
if (event.shiftKey) {
87+
modifiers.push('shift');
88+
}
89+
if (event.altKey) {
90+
modifiers.push('alt');
91+
}
92+
if (event.ctrlKey) {
93+
modifiers.push('ctrl');
94+
}
95+
if (event.metaKey) {
96+
modifiers.push('meta');
97+
}
98+
if (modifiers.length && ['shift', 'alt', 'ctrl', 'meta', 'mod', 'control', 'option', 'command'].includes(key)) {
99+
key = '';
100+
}
101+
const shortcut = modifiers.sort().join('+') + (modifiers.length && key.length ? '+' : '') + key;
102+
const checkModKey = event.ctrlKey && !isMac || event.metaKey && isMac;
103+
// some commands dont stop propagation, meaning we consider the keypress never collapses with them
104+
// @TODO: have a better test than listing precise commands here
105+
if (shortcut === "=") {
106+
return false;
107+
}
108+
if (!checkModKey) {
109+
return allShortcuts.includes(shortcut);
110+
}
111+
if (isMac) {
112+
return allShortcuts.includes(shortcut.replace('meta', 'mod'));
113+
}
114+
return allShortcuts.includes(shortcut.replace('ctrl', 'mod'));
115+
};
116+
117+
export const keypressIsPrintableCharacter = (event: KeyboardEvent) => {
118+
const isOneChar = [...event.key].length === 1;
119+
// we assume that any 'action' modifier key pressed will not result in a printable character
120+
// this allows us to avoid stuff like "ctrl+r" (action), while keeping stuff like "altgr+r" (printable char)
121+
if (event.getModifierState('Control') || event.getModifierState('Meta')) {
122+
return false;
123+
}
124+
return isOneChar;
125+
};
126+
56127
/**
57128
* Populate allCommands from those provided, or listed in commandList.js. Also populates the
58129
* globally exposed `cmd` object whose properties invoke commands: e.g. typing `cmd.cursorDown` in
@@ -68,6 +139,7 @@ export function init(optCommandGroups?: CommendGroupDef[]) {
68139
Object.keys(_allKeys).forEach(function(k) {
69140
delete _allKeys[k as CommandName];
70141
});
142+
allShortcuts.length = 0;
71143

72144
commandGroups.forEach(function(commandGroup) {
73145
commandGroup.commands.forEach(function(c) {
@@ -78,6 +150,7 @@ export function init(optCommandGroups?: CommendGroupDef[]) {
78150
bindKeys: c.bindKeys,
79151
deprecated: c.deprecated,
80152
});
153+
c.keys.forEach(k => saveShortcut(k));
81154
}
82155
});
83156
});

app/client/ui/App.ts

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import * as commandList from 'app/client/components/commandList';
55
import * as commands from 'app/client/components/commands';
66
import {unsavedChanges} from 'app/client/components/UnsavedChanges';
77
import {get as getBrowserGlobals} from 'app/client/lib/browserGlobals';
8-
import {isDesktop} from 'app/client/lib/browserInfo';
9-
import {FocusLayer} from 'app/client/lib/FocusLayer';
108
import * as koUtil from 'app/client/lib/koUtil';
119
import {reportError, TopAppModel, TopAppModelImpl} from 'app/client/models/AppModel';
1210
import {DocPageModel} from 'app/client/models/DocPageModel';
@@ -65,21 +63,7 @@ export class App extends DisposableWithEvents {
6563
this._settings = ko.observable({});
6664
this.features = ko.computed(() => this._settings().features || {});
6765

68-
if (isDesktop()) {
69-
this.autoDispose(Clipboard.create(this));
70-
} else {
71-
// On mobile, we do not want to keep focus on a special textarea (which would cause unwanted
72-
// scrolling and showing of mobile keyboard). But we still rely on 'clipboard_focus' and
73-
// 'clipboard_blur' events to know when the "app" has a focus (rather than a particular
74-
// input), by making document.body focusable and using a FocusLayer with it as the default.
75-
document.body.setAttribute('tabindex', '-1');
76-
FocusLayer.create(this, {
77-
defaultFocusElem: document.body,
78-
allowFocus: Clipboard.allowFocus,
79-
onDefaultFocus: () => this.trigger('clipboard_focus'),
80-
onDefaultBlur: () => this.trigger('clipboard_blur'),
81-
});
82-
}
66+
this.autoDispose(Clipboard.create(this));
8367

8468
this.topAppModel = this.autoDispose(TopAppModelImpl.create(null, G.window));
8569

0 commit comments

Comments
 (0)