From 0c2586aa85e4cbdce3946ef589eb28f238085ec8 Mon Sep 17 00:00:00 2001 From: Emmanuel Pelletier Date: Wed, 9 Oct 2024 11:43:01 +0200 Subject: [PATCH] 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 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. --- app/client/components/Clipboard.js | 161 +++++++++++++++++++++------ app/client/components/commandList.ts | 14 ++- app/client/components/commands.ts | 91 +++++++++++++++ app/client/ui/App.ts | 18 +-- 4 files changed, 232 insertions(+), 52 deletions(-) diff --git a/app/client/components/Clipboard.js b/app/client/components/Clipboard.js index decd3f163a..8ae8b32901 100644 --- a/app/client/components/Clipboard.js +++ b/app/client/components/Clipboard.js @@ -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). @@ -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'); @@ -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: () => { @@ -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 = keypressCollapsesWithExistingCommand(ev); + const isPrintableCharacter = 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; @@ -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(); @@ -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(); @@ -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'); @@ -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; @@ -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 @@ -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) { @@ -346,6 +379,68 @@ function showUnavailableMenuCommandModal(action) { ); } + +/** + * 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. + * + * @param {KeyboardEvent} event + * @returns {boolean} + */ +function keypressIsPrintableCharacter(event) { + // 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. + // Make tests around that depending on Shift key press. + 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; +} + +/** + * Helper to know if a given keypress matches an existing command shortcut. + * + * @param {KeyboardEvent} event + * @returns {boolean} + */ +function keypressCollapsesWithExistingCommand(event) { + const shortcut = commands.getShortcutFromKeypress(event); + return !!shortcut && shortcut.stopsPropagation; +} + module.exports = Clipboard; const cssModalContent = styled('div', ` diff --git a/app/client/components/commandList.ts b/app/client/components/commandList.ts index 00a9e02745..c2c7b8a190 100644 --- a/app/client/components/commandList.ts +++ b/app/client/components/commandList.ts @@ -127,6 +127,15 @@ export interface CommandDef { desc: string | null; bindKeys?: boolean; deprecated?: boolean; + /** + * Whether or not this command stops keyboard event propagation. + * + * Toggling this doesn't actually make the command stop propagation or not. + * It's a representation of what the function called when the command runs does. + * + * When not set, defaults to true, as 99% of commands do stop propagation. + */ + stopsPropagation?: boolean; } export interface MenuCommand { @@ -314,7 +323,7 @@ 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' @@ -322,7 +331,7 @@ export const groups: CommendGroupDef[] = [{ 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' @@ -517,6 +526,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'], diff --git a/app/client/components/commands.ts b/app/client/components/commands.ts index 4fa7daaf57..fe86be6c04 100644 --- a/app/client/components/commands.ts +++ b/app/client/components/commands.ts @@ -53,6 +53,93 @@ export const allCommands: { [key in CommandName]: Command } = {} as any; */ const _allKeys: Record = {}; +/** + * Internal variable listing all shortcuts defined in the app, + * and telling 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 } } = {}; + +/** + * Gets a shortcut string (like "mod+o", or a shorcut keys list like ["mod", "o"]) + * and saves it in _allShortcuts, with a boolean indicating whether the command stops propagation. + */ +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), + // we assume all commands stop event propagation if at least 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 = { + 'return': 'enter', + 'esc': 'escape', + '+': 'plus', + ' ': 'space', +}; + +/** + * Given a keyboard event, get a string representing the keyboard shortcut of a registered command. + * + * @returns A string like "mod+o", or null if no command is found + */ +export const getShortcutFromKeypress = (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; + + if (isMac && event.metaKey && _allShortcuts[shortcut.replace('meta', 'mod')]) { + return shortcut.replace('meta', 'mod'); + } + + if (!isMac && event.ctrlKey && _allShortcuts[shortcut.replace('ctrl', 'mod')]) { + return shortcut.replace('ctrl', 'mod'); + } + + if (_allShortcuts[shortcut]) { + return shortcut; + } + + return null; +}; + /** * 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 @@ -68,6 +155,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) { @@ -78,6 +168,7 @@ export function init(optCommandGroups?: CommendGroupDef[]) { bindKeys: c.bindKeys, deprecated: c.deprecated, }); + c.keys.forEach(k => saveShortcut(k, c.stopsPropagation)); } }); }); diff --git a/app/client/ui/App.ts b/app/client/ui/App.ts index 2b2b1484bc..65712003ff 100644 --- a/app/client/ui/App.ts +++ b/app/client/ui/App.ts @@ -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'; @@ -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));