Skip to content

Commit

Permalink
keyboard: simplify clipboard internals to enable future kb navigation
Browse files Browse the repository at this point in the history
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 interaction
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.
  • Loading branch information
manuhabitela committed Oct 14, 2024
1 parent 1a527d7 commit b432a9d
Show file tree
Hide file tree
Showing 4 changed files with 206 additions and 52 deletions.
99 changes: 66 additions & 33 deletions app/client/components/Clipboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,11 @@
* Clipboard component manages the copy/cut/paste events by capturing these events from the browser,
* managing their state, and exposing an API to other components to get/set the data.
*
* Because of a lack of standardization of ClipboardEvents between browsers, the way Clipboard
* captures the events is by creating a hidden textarea element that's always focused with some text
* selected. Here is a good write-up of this:
* https://www.lucidchart.com/techblog/2014/12/02/definitive-guide-copying-pasting-javascript/
*
* When ClipboardEvent is detected, Clipboard captures the event and calls the corresponding
* copy/cut/paste/input command actions, which will get called on the appropriate component.
*
* The Clipboard also handles triggering correctly the "input" command when any key is pressed.
*
* Usage:
* Components need to register copy/cut/paste actions with command.js:
* .copy() should return @pasteObj (defined below).
Expand Down Expand Up @@ -45,7 +42,6 @@ var {confirmModal} = require('app/client/ui2018/modals');
var {styled} = require('grainjs');

var commands = require('./commands');
var dom = require('../lib/dom');
var Base = require('./Base');
var tableUtil = require('../lib/tableUtil');

Expand All @@ -54,27 +50,10 @@ const t = makeT('Clipboard');
function Clipboard(app) {
Base.call(this, null);
this._app = app;
this.copypasteField = this.autoDispose(dom('textarea.copypaste.mousetrap', ''));
this.timeoutId = null;

this.onEvent(this.copypasteField, 'input', function(elem, event) {
var value = elem.value;
elem.value = '';
commands.allCommands.input.run(value);
return false;
});
this.onEvent(this.copypasteField, 'copy', this._onCopy);
this.onEvent(this.copypasteField, 'cut', this._onCut);
this.onEvent(this.copypasteField, 'paste', this._onPaste);

document.body.appendChild(this.copypasteField);

FocusLayer.create(this, {
defaultFocusElem: this.copypasteField,
allowFocus: allowFocus,
defaultFocusElem: document.body,
onDefaultFocus: () => {
this.copypasteField.value = ' ';
this.copypasteField.select();
this._app.trigger('clipboard_focus');
},
onDefaultBlur: () => {
Expand All @@ -94,6 +73,34 @@ function Clipboard(app) {
}
});

// Listen for copy/cut/paste events and trigger the corresponding clipboard action.
// Note that internally, before triggering the action, we check if the currently active element
// doesn't already handle these events itself.
// This allows to globally handle copy/cut/paste events, without impacting
// inputs/textareas/selects where copy/cut/paste events should be left alone.
this.onEvent(document, 'copy', (_, event) => this._onCopy(event));
this.onEvent(document, 'cut', (_, event) => this._onCut(event));
this.onEvent(document, 'paste', (_, event) => this._onPaste(event));

// when typing a random printable character while not focusing an interactive element,
// trigger the input command with it
// @TODO: there is currently an issue, sometimes when typing something, that makes us focus a cell textarea,
// and then we can mouseclick on a different cell: dom focus is still on textarea, visual table focus is on new cell.
this.onEvent(document.body, 'keydown', (_, event) => {
if (shouldAvoidClipboardShortcuts(document.activeElement)) {
return;
}
const ev = event.originalEvent;
const collapsesWithCommands = commands.keypressCollapsesWithExistingCommand(ev);
const isPrintableCharacter = commands.keypressIsPrintableCharacter(ev);
if (!collapsesWithCommands && isPrintableCharacter) {
commands.allCommands.input.run(ev.key);
event.preventDefault();
} else {
console.log(ev.key, ev.key.length, ev.code);
}
});

// In the event of a cut a callback is provided by the viewsection that is the target of the cut.
// When called it returns the additional removal action needed for a cut.
this._cutCallback = null;
Expand All @@ -116,7 +123,10 @@ Clipboard.commands = {
* Internal helper fired on `copy` events. If a callback was registered from a component, calls the
* callback to get selection data and puts it on the clipboard.
*/
Clipboard.prototype._onCopy = function(elem, event) {
Clipboard.prototype._onCopy = function(event) {
if (shouldAvoidClipboardShortcuts(document.activeElement)) {
return;
}
event.preventDefault();

let pasteObj = commands.allCommands.copy.run();
Expand All @@ -136,7 +146,11 @@ Clipboard.prototype._doContextMenuCopyWithHeaders = function() {
this._copyToClipboard(pasteObj, 'copy', true);
};

Clipboard.prototype._onCut = function(elem, event) {
Clipboard.prototype._onCut = function(event) {
if (shouldAvoidClipboardShortcuts(document.activeElement)) {
return;
}

event.preventDefault();

let pasteObj = commands.allCommands.cut.run();
Expand Down Expand Up @@ -211,7 +225,11 @@ Clipboard.prototype._setCutCallback = function(pasteObj, cutData) {
* Internal helper fired on `paste` events. If a callback was registered from a component, calls the
* callback with data from the clipboard.
*/
Clipboard.prototype._onPaste = function(elem, event) {
Clipboard.prototype._onPaste = function(event) {
if (shouldAvoidClipboardShortcuts(document.activeElement)) {
return;
}

event.preventDefault();
const cb = event.originalEvent.clipboardData;
const plainText = cb.getData('text/plain');
Expand All @@ -220,12 +238,6 @@ Clipboard.prototype._onPaste = function(elem, event) {
this._doPaste(pasteData, plainText);
};

var FOCUS_TARGET_TAGS = {
'INPUT': true,
'TEXTAREA': true,
'SELECT': true,
'IFRAME': true,
};

Clipboard.prototype._doContextMenuPaste = async function() {
let clipboardItem;
Expand Down Expand Up @@ -293,6 +305,17 @@ async function getTextFromClipboardItem(clipboardItem, type) {
}
}

const CLIPBOARD_TAGS = {
'INPUT': true,
'TEXTAREA': true,
'SELECT': true,
};

const FOCUS_TARGET_TAGS = {
...CLIPBOARD_TAGS,
'IFRAME': true,
};

/**
* Helper to determine if the currently active element deserves to keep its own focus, and capture
* copy-paste events. Besides inputs and textareas, any element can be marked to be a valid
Expand All @@ -304,6 +327,16 @@ function allowFocus(elem) {
elem.classList.contains('clipboard_focus'));
}

/**
* Helper to determine if the given element is a valid target for copy-cut-paste actions.
*
* It slightly differs from allowFocus: here we exclusively check for clipboard-related actions,
* not focus-related ones.
*/
function shouldAvoidClipboardShortcuts(elem) {
return elem && CLIPBOARD_TAGS.hasOwnProperty(elem.tagName)
}

Clipboard.allowFocus = allowFocus;

function showUnavailableMenuCommandModal(action) {
Expand Down
6 changes: 4 additions & 2 deletions app/client/components/commandList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ export interface CommandDef {
name: CommandName;
keys: string[];
desc: string | null;
stopsPropagation?: boolean;
bindKeys?: boolean;
deprecated?: boolean;
}
Expand Down Expand Up @@ -314,15 +315,15 @@ export const groups: CommendGroupDef[] = [{
name: 'cursorLeft',
keys: ['Left'],
desc: 'Move left to the previous field'
}, {
}, /**/{
name: 'nextField',
keys: ['Tab'],
desc: 'Move to the next field, saving changes if editing a value'
}, {
name: 'prevField',
keys: ['Shift+Tab'],
desc: 'Move to the previous field, saving changes if editing a value'
}, {
}, /**/{
name: 'pageDown',
keys: ['PageDown'],
desc: 'Move down one page of records, or to next record in a card list'
Expand Down Expand Up @@ -517,6 +518,7 @@ export const groups: CommendGroupDef[] = [{
name: 'makeFormula',
keys: ["="],
desc: 'When typed at the start of a cell, make this a formula column',
stopsPropagation: false,
}, {
name: 'unmakeFormula',
keys: ['Backspace'],
Expand Down
135 changes: 135 additions & 0 deletions app/client/components/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,137 @@ export const allCommands: { [key in CommandName]: Command } = {} as any;
*/
const _allKeys: Record<string, CommandGroup[]> = {};

/**
* This is an internal variable, listing all shortcuts defined in the app,
* and exposing whether or not they stop keyboard event propagation.
*
* Useful to check if a user keypress matches a command shortcut.
*/
const _allShortcuts: { [key: string]: { keys: string, stopsPropagation: boolean } } = {};

const saveShortcut = (key: string | string[], stopsPropagation?: boolean) => {
let shortcut = "";
const keyString = typeof key === 'string' ? key.toLowerCase() : key.join('+').toLowerCase();
if (keyString === "+" || !keyString.includes('+')) {
shortcut = keyString;
} else {
const splitKeys = keyString.split('+');
shortcut = splitKeys.slice(0, -1).sort().join('+') + '+' + splitKeys[splitKeys.length - 1];
}
// If multiple commands have the same shortcut, but triggered in different contexts,
// if one of them stopsPropagation, we assume all of them do.
// This works for now but I'm afraid it might to lead to issues in the future.
_allShortcuts[shortcut] = {
keys: shortcut,
stopsPropagation: !!_allShortcuts[shortcut] && _allShortcuts[shortcut].stopsPropagation
? true
: stopsPropagation ?? true,
};
};

const _keyAliases: Record<string, string> = {
'return': 'enter',
'esc': 'escape',
'+': 'plus',
' ': 'space',
};

/**
* Helper to know if a given keypress matches an existing command shortcut.
*
* Helpful when we want to handle specific keypresses without interfering with our commands.
*/
export const keypressCollapsesWithExistingCommand = (event: KeyboardEvent) => {
let key = event.key.toLowerCase();
if (_keyAliases[key]) {
key = _keyAliases[key];
}
const modifiers = [];
if (event.shiftKey) {
modifiers.push('shift');
}
if (event.altKey) {
modifiers.push('alt');
}
if (event.ctrlKey) {
modifiers.push('ctrl');
}
if (event.metaKey) {
modifiers.push('meta');
}
if (
modifiers.length
&& ['shift', 'alt', 'ctrl', 'meta', 'mod', 'control', 'option', 'command'].includes(key)
) {
key = '';
}
const shortcut = modifiers.sort().join('+')
+ (modifiers.length && key.length ? '+' : '')
+ key;
const checkModKey = event.ctrlKey && !isMac || event.metaKey && isMac;

let existingShortcut = null;
if (!checkModKey) {
existingShortcut = _allShortcuts[shortcut];
} else {
existingShortcut = isMac
? _allShortcuts[shortcut.replace('meta', 'mod')]
: _allShortcuts[shortcut.replace('ctrl', 'mod')];
}
return !!existingShortcut && existingShortcut.stopsPropagation;
};

/**
* Helper to know if a keypress from a keydown/keypress/etc event is an actually printable character.
*
* This is useful in the Clipboard where we listen for keypresses outside of an input field,
* trying to know if the keypress should be handled by the application or not.
*/
export const keypressIsPrintableCharacter = (event: KeyboardEvent) => {
console.log(event.key, {
shift: event.getModifierState('Shift'),
lower: event.key.toLocaleLowerCase(),
upper: event.key.toLocaleUpperCase(),
lowerEq: event.key.toLocaleLowerCase() === event.key,
upperEq: event.key.toLocaleUpperCase() === event.key,
});

// We assume that any 'action' modifier key pressed will not result in a printable character.
// This allows us to avoid stuff like "ctrl+r" (action), while keeping stuff like "altgr+r" (printable char).
if (event.getModifierState('Control') || event.getModifierState('Meta')) {
return false;
}
// Stop early if the key press does nothing, in order to prevent entering in a cell with no character.
if (event.key === "") {
return false;
}
// Count the number of characters in the key, using a spread operator trick to correctly count unicode characters.
const keyLength = [...event.key].length;
// From testing in various languages, we can assume all keys represented by a single character are printable.
// Stop early in that case.
if (keyLength === 1) {
return true;
}
// When here, `event.key` could be a non-printable character like `ArrowUp`, `Enter`, `F3`…
// or a printable character with length > 1, like `لا` in Arabic.
// We want to avoid the first case.

// Only special keys like `ArrowUp` etc are listed with uppercases when typed in lowercase.
// That means if `event.key` is the same in lowercase and uppercase, it's a printable character.
// @TODO: test more special keys, for now this fails with Shift+F3 for example.
if (!event.getModifierState('Shift') && event.key.toLocaleLowerCase() === event.key
|| (
event.getModifierState('Shift')
&& event.key.toLocaleLowerCase() === event.key
&& event.key.toLocaleUpperCase() === event.key
)
) {
return true;
}
// If we are here, it means the key is like `ArrowUp` and others, those are not printable.
return false;
};

/**
* Populate allCommands from those provided, or listed in commandList.js. Also populates the
* globally exposed `cmd` object whose properties invoke commands: e.g. typing `cmd.cursorDown` in
Expand All @@ -68,6 +199,9 @@ export function init(optCommandGroups?: CommendGroupDef[]) {
Object.keys(_allKeys).forEach(function(k) {
delete _allKeys[k as CommandName];
});
Object.keys(_allShortcuts).forEach(function(k) {
delete _allShortcuts[k];
});

commandGroups.forEach(function(commandGroup) {
commandGroup.commands.forEach(function(c) {
Expand All @@ -78,6 +212,7 @@ export function init(optCommandGroups?: CommendGroupDef[]) {
bindKeys: c.bindKeys,
deprecated: c.deprecated,
});
c.keys.forEach(k => saveShortcut(k, c.stopsPropagation));
}
});
});
Expand Down
18 changes: 1 addition & 17 deletions app/client/ui/App.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import * as commandList from 'app/client/components/commandList';
import * as commands from 'app/client/components/commands';
import {unsavedChanges} from 'app/client/components/UnsavedChanges';
import {get as getBrowserGlobals} from 'app/client/lib/browserGlobals';
import {isDesktop} from 'app/client/lib/browserInfo';
import {FocusLayer} from 'app/client/lib/FocusLayer';
import * as koUtil from 'app/client/lib/koUtil';
import {reportError, TopAppModel, TopAppModelImpl} from 'app/client/models/AppModel';
import {DocPageModel} from 'app/client/models/DocPageModel';
Expand Down Expand Up @@ -65,21 +63,7 @@ export class App extends DisposableWithEvents {
this._settings = ko.observable({});
this.features = ko.computed(() => this._settings().features || {});

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

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

Expand Down

0 comments on commit b432a9d

Please sign in to comment.