From 38a916de906f65ce61d71209b968ee07a8995bde 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 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. --- app/client/components/Clipboard.js | 97 ++++++++++++++++++++---------- app/client/components/commands.ts | 73 ++++++++++++++++++++++ app/client/ui/App.ts | 18 +----- 3 files changed, 138 insertions(+), 50 deletions(-) diff --git a/app/client/components/Clipboard.js b/app/client/components/Clipboard.js index decd3f163a5..f5484a66929 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,32 @@ 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(); + } + }); + // 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 +121,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 +144,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 +223,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 +236,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 +303,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 +325,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) { diff --git a/app/client/components/commands.ts b/app/client/components/commands.ts index 4fa7daaf579..454e582f463 100644 --- a/app/client/components/commands.ts +++ b/app/client/components/commands.ts @@ -53,6 +53,77 @@ export const allCommands: { [key in CommandName]: Command } = {} as any; */ const _allKeys: Record = {}; +export const allShortcuts: Array = []; + +const saveShortcut = (key: string | string[]) => { + const keyString = typeof key === 'string' ? key.toLowerCase() : key.join('+').toLowerCase(); + if (keyString === "+" || !keyString.includes('+')) { + allShortcuts.push(keyString); + } else { + const splitKeys = keyString.split('+'); + allShortcuts.push(splitKeys.slice(0, -1).sort().join('+') + '+' + splitKeys[splitKeys.length - 1]); + } +}; + +const _keyAliases: Record = { + '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; + // some commands dont stop propagation, meaning we consider the keypress never collapses with them + // @TODO: have a better test than listing precise commands here + if (shortcut === "=") { + return false; + } + if (!checkModKey) { + return allShortcuts.includes(shortcut); + } + if (isMac) { + return allShortcuts.includes(shortcut.replace('meta', 'mod')); + } + return allShortcuts.includes(shortcut.replace('ctrl', 'mod')); +}; + +export const keypressIsPrintableCharacter = (event: KeyboardEvent) => { + const isOneChar = [...event.key].length === 1; + // 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; + } + return isOneChar; +}; + /** * 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 +139,7 @@ export function init(optCommandGroups?: CommendGroupDef[]) { Object.keys(_allKeys).forEach(function(k) { delete _allKeys[k as CommandName]; }); + allShortcuts.length = 0; commandGroups.forEach(function(commandGroup) { commandGroup.commands.forEach(function(c) { @@ -78,6 +150,7 @@ export function init(optCommandGroups?: CommendGroupDef[]) { bindKeys: c.bindKeys, deprecated: c.deprecated, }); + c.keys.forEach(k => saveShortcut(k)); } }); }); diff --git a/app/client/ui/App.ts b/app/client/ui/App.ts index 2b2b1484bc8..65712003ff2 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));