From 00084c57981d19d90fc95bf1d5044de6ecffcbef Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Mon, 1 Apr 2024 15:34:22 +0100 Subject: [PATCH] Desktop, Mobile: Improve focus handling --- .eslintignore | 4 +- .eslintrc.js | 11 +++++ .gitignore | 4 +- packages/app-cli/app/app-gui.js | 1 + packages/app-clipper/popup/src/App.js | 1 + packages/app-desktop/ElectronAppWrapper.ts | 1 + .../gui/EditFolderDialog/Dialog.tsx | 3 +- .../NoteBody/CodeMirror/v5/CodeMirror.tsx | 29 ++----------- .../NoteBody/CodeMirror/v5/Editor.tsx | 3 +- .../NoteBody/CodeMirror/v6/CodeMirror.tsx | 24 ----------- .../CodeMirror/v6/useEditorCommands.ts | 5 ++- .../NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx | 3 +- .../NoteBody/TinyMCE/utils/openEditDialog.ts | 3 +- .../commands/focusElementNoteTitle.ts | 3 +- .../NoteEditor/commands/showLocalSearch.ts | 3 +- .../gui/NoteEditor/utils/useFormNote.ts | 3 +- .../gui/NoteEditor/utils/useNoteSearchBar.ts | 3 +- .../app-desktop/gui/NoteList/NoteList2.tsx | 5 ++- .../gui/NoteList/utils/useFocusNote.ts | 5 ++- .../app-desktop/gui/NotePropertiesDialog.tsx | 7 ++-- packages/app-desktop/gui/NoteSearchBar.tsx | 5 ++- packages/app-desktop/gui/NoteTextViewer.tsx | 3 +- packages/app-desktop/gui/PromptDialog.tsx | 3 +- .../app-desktop/gui/SearchBar/SearchBar.tsx | 5 ++- packages/app-desktop/gui/Sidebar/Sidebar.tsx | 3 +- .../Sidebar/commands/focusElementSideBar.ts | 5 ++- .../services/plugins/UserWebview.tsx | 3 +- .../services/plugins/UserWebviewDialog.tsx | 3 +- packages/app-desktop/tsconfig.json | 2 +- .../components/NoteEditor/EditLinkDialog.tsx | 3 +- .../app-mobile/components/screens/Note.tsx | 8 +--- .../editorCommands/editorCommands.ts | 3 +- packages/lib/utils/focusHandler.ts | 41 +++++++++++++++++++ packages/pdf-viewer/PdfDocument.ts | 3 +- 34 files changed, 119 insertions(+), 92 deletions(-) create mode 100644 packages/lib/utils/focusHandler.ts diff --git a/.eslintignore b/.eslintignore index fbc2029a1..8eed64aae 100644 --- a/.eslintignore +++ b/.eslintignore @@ -322,9 +322,7 @@ packages/app-desktop/gui/NoteEditor/utils/useNoteSearchBar.js packages/app-desktop/gui/NoteEditor/utils/usePluginServiceRegistration.js packages/app-desktop/gui/NoteEditor/utils/useSearchMarkers.js packages/app-desktop/gui/NoteEditor/utils/useWindowCommandHandler.js -packages/app-desktop/gui/NoteList/NoteList.js packages/app-desktop/gui/NoteList/NoteList2.js -packages/app-desktop/gui/NoteList/NoteListSource.js packages/app-desktop/gui/NoteList/commands/focusElementNoteList.js packages/app-desktop/gui/NoteList/commands/index.js packages/app-desktop/gui/NoteList/utils/canManuallySortNotes.js @@ -350,7 +348,6 @@ packages/app-desktop/gui/NoteListHeader/utils/getColumnTitle.js packages/app-desktop/gui/NoteListHeader/utils/useContextMenu.js packages/app-desktop/gui/NoteListHeader/utils/validateColumns.test.js packages/app-desktop/gui/NoteListHeader/utils/validateColumns.js -packages/app-desktop/gui/NoteListItem.js packages/app-desktop/gui/NoteListItem/NoteListItem.js packages/app-desktop/gui/NoteListItem/utils/getNoteTitleHtml.js packages/app-desktop/gui/NoteListItem/utils/prepareViewProps.test.js @@ -1148,6 +1145,7 @@ packages/lib/types.js packages/lib/utils/ActionLogger.test.js packages/lib/utils/ActionLogger.js packages/lib/utils/credentialFiles.js +packages/lib/utils/focusHandler.js packages/lib/utils/ipc/RemoteMessenger.test.js packages/lib/utils/ipc/RemoteMessenger.js packages/lib/utils/ipc/TestMessenger.js diff --git a/.eslintrc.js b/.eslintrc.js index 9c039c163..7bf277e19 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -101,6 +101,17 @@ module.exports = { 'no-unneeded-ternary': 'error', 'github/array-foreach': ['error'], + 'no-restricted-properties': ['error', + { + 'property': 'focus', + 'message': 'Please use focusHandler::focus() instead', + }, + { + 'property': 'blur', + 'message': 'Please use focusHandler::blur() instead', + }, + ], + // ------------------------------- // Formatting // ------------------------------- diff --git a/.gitignore b/.gitignore index 3a6bf91c9..e921c3a3d 100644 --- a/.gitignore +++ b/.gitignore @@ -302,9 +302,7 @@ packages/app-desktop/gui/NoteEditor/utils/useNoteSearchBar.js packages/app-desktop/gui/NoteEditor/utils/usePluginServiceRegistration.js packages/app-desktop/gui/NoteEditor/utils/useSearchMarkers.js packages/app-desktop/gui/NoteEditor/utils/useWindowCommandHandler.js -packages/app-desktop/gui/NoteList/NoteList.js packages/app-desktop/gui/NoteList/NoteList2.js -packages/app-desktop/gui/NoteList/NoteListSource.js packages/app-desktop/gui/NoteList/commands/focusElementNoteList.js packages/app-desktop/gui/NoteList/commands/index.js packages/app-desktop/gui/NoteList/utils/canManuallySortNotes.js @@ -330,7 +328,6 @@ packages/app-desktop/gui/NoteListHeader/utils/getColumnTitle.js packages/app-desktop/gui/NoteListHeader/utils/useContextMenu.js packages/app-desktop/gui/NoteListHeader/utils/validateColumns.test.js packages/app-desktop/gui/NoteListHeader/utils/validateColumns.js -packages/app-desktop/gui/NoteListItem.js packages/app-desktop/gui/NoteListItem/NoteListItem.js packages/app-desktop/gui/NoteListItem/utils/getNoteTitleHtml.js packages/app-desktop/gui/NoteListItem/utils/prepareViewProps.test.js @@ -1128,6 +1125,7 @@ packages/lib/types.js packages/lib/utils/ActionLogger.test.js packages/lib/utils/ActionLogger.js packages/lib/utils/credentialFiles.js +packages/lib/utils/focusHandler.js packages/lib/utils/ipc/RemoteMessenger.test.js packages/lib/utils/ipc/RemoteMessenger.js packages/lib/utils/ipc/TestMessenger.js diff --git a/packages/app-cli/app/app-gui.js b/packages/app-cli/app/app-gui.js index 93c46fb69..c22958751 100644 --- a/packages/app-cli/app/app-gui.js +++ b/packages/app-cli/app/app-gui.js @@ -441,6 +441,7 @@ class AppGui { if (cmd === 'activate') { const w = this.widget('mainWindow').focusedWidget; if (w.name === 'folderList') { + // eslint-disable-next-line no-restricted-properties this.widget('noteList').focus(); } else if (w.name === 'noteList' || w.name === 'noteText') { this.processPromptCommand('edit $n'); diff --git a/packages/app-clipper/popup/src/App.js b/packages/app-clipper/popup/src/App.js index f204e3aee..afd76481d 100644 --- a/packages/app-clipper/popup/src/App.js +++ b/packages/app-clipper/popup/src/App.js @@ -243,6 +243,7 @@ class AppComponent extends Component { if (!ref) break; lastRef = ref; } + // eslint-disable-next-line no-restricted-properties if (lastRef) lastRef.focus(); } } diff --git a/packages/app-desktop/ElectronAppWrapper.ts b/packages/app-desktop/ElectronAppWrapper.ts index 61699a14f..3b3c8e7ff 100644 --- a/packages/app-desktop/ElectronAppWrapper.ts +++ b/packages/app-desktop/ElectronAppWrapper.ts @@ -428,6 +428,7 @@ export default class ElectronAppWrapper { if (!win) return; if (win.isMinimized()) win.restore(); win.show(); + // eslint-disable-next-line no-restricted-properties win.focus(); if (process.platform !== 'darwin') { const url = argv.find((arg) => isCallbackUrl(arg)); diff --git a/packages/app-desktop/gui/EditFolderDialog/Dialog.tsx b/packages/app-desktop/gui/EditFolderDialog/Dialog.tsx index 5baf449e9..689166876 100644 --- a/packages/app-desktop/gui/EditFolderDialog/Dialog.tsx +++ b/packages/app-desktop/gui/EditFolderDialog/Dialog.tsx @@ -13,6 +13,7 @@ import Button from '../Button/Button'; import bridge from '../../services/bridge'; import shim from '@joplin/lib/shim'; import FolderIconBox from '../FolderIconBox'; +import { focus } from '@joplin/lib/utils/focusHandler'; interface Props { themeId: number; @@ -46,7 +47,7 @@ export default function(props: Props) { }, [props.dispatch]); useEffect(() => { - titleInputRef.current.focus(); + focus('Dialog::titleInputRef', titleInputRef.current); setTimeout(() => { titleInputRef.current.select(); diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v5/CodeMirror.tsx b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v5/CodeMirror.tsx index fb339e0c6..79e1e6f94 100644 --- a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v5/CodeMirror.tsx +++ b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v5/CodeMirror.tsx @@ -32,6 +32,7 @@ import useStyles from '../utils/useStyles'; import useContextMenu from '../utils/useContextMenu'; import useWebviewIpcMessage from '../utils/useWebviewIpcMessage'; import useEditorSearchHandler from '../utils/useEditorSearchHandler'; +import { focus } from '@joplin/lib/utils/focusHandler'; function markupRenderOptions(override: MarkupToHtmlOptions = null): MarkupToHtmlOptions { return { ...override }; @@ -142,7 +143,7 @@ function CodeMirror(props: NoteBodyEditorProps, ref: ForwardedRef= 0) { - editorRef.current.focus(); + focus('v5/CodeMirror::editor.focus', editorRef.current); } else { // If we just call focus() then the iframe is focused, // but not its content, such that scrolling up / down @@ -188,7 +189,7 @@ function CodeMirror(props: NoteBodyEditorProps, ref: ForwardedRef wrapSelectionWithStrings('*', '*', _('emphasised text')), textLink: async () => { const url = await dialogs.prompt(_('Insert Hyperlink')); - editorRef.current.focus(); + focus('v5/CodeMirror::textLink', editorRef.current); if (url) wrapSelectionWithStrings('[', `](${url})`); }, textCode: () => { @@ -699,30 +700,6 @@ function CodeMirror(props: NoteBodyEditorProps, ref: ForwardedRef= 0; - - // useEffect(() => { - // if (!editorRef.current) return; - - // // Anytime the user toggles the visible panes AND the editor is visible as a result - // // we should focus the editor - // // The intuition is that a panel toggle (with editor in view) is the equivalent of - // // an editor interaction so users should expect the editor to be focused - // if (editorPaneVisible) { - // editorRef.current.focus(); - // } - // }, [editorPaneVisible]); - useEffect(() => { if (!editorRef.current) return; diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v5/Editor.tsx b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v5/Editor.tsx index 04c232e8b..1fde3e254 100644 --- a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v5/Editor.tsx +++ b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v5/Editor.tsx @@ -33,6 +33,7 @@ import Setting from '@joplin/lib/models/Setting'; // import eventManager from '@joplin/lib/eventManager'; import { reg } from '@joplin/lib/registry'; +import { focus } from '@joplin/lib/utils/focusHandler'; // Based on http://pypl.github.io/PYPL.html const topLanguages = [ @@ -137,7 +138,7 @@ function Editor(props: EditorProps, ref: any) { // eslint-disable-next-line no-unused-vars, @typescript-eslint/no-unused-vars const editor_drop = useCallback((cm: any, _event: any) => { - cm.focus(); + focus('v5/Editor::editor_drop', cm); }, []); const editor_drag = useCallback((cm: any, event: any) => { diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/CodeMirror.tsx b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/CodeMirror.tsx index 1867c44b8..9a8bf559a 100644 --- a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/CodeMirror.tsx +++ b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/CodeMirror.tsx @@ -311,30 +311,6 @@ const CodeMirror = (props: NoteBodyEditorProps, ref: ForwardedRef= 0; - - // useEffect(() => { - // if (!editorRef.current) return; - - // // Anytime the user toggles the visible panes AND the editor is visible as a result - // // we should focus the editor - // // The intuition is that a panel toggle (with editor in view) is the equivalent of - // // an editor interaction so users should expect the editor to be focused - // if (editorPaneVisible) { - // editorRef.current.focus(); - // } - // }, [editorPaneVisible]); - useEditorSearchHandler({ setLocalSearchResultCount: props.setLocalSearchResultCount, searchMarkers: props.searchMarkers, diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/useEditorCommands.ts b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/useEditorCommands.ts index e998d016e..e1956252a 100644 --- a/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/useEditorCommands.ts +++ b/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/useEditorCommands.ts @@ -8,6 +8,7 @@ import { EditorCommandType } from '@joplin/editor/types'; import Logger from '@joplin/utils/Logger'; import CodeMirrorControl from '@joplin/editor/CodeMirror/CodeMirrorControl'; import { MarkupLanguage } from '@joplin/renderer'; +import { focus } from '@joplin/lib/utils/focusHandler'; const logger = Logger.create('CodeMirror 6 commands'); @@ -88,7 +89,7 @@ const useEditorCommands = (props: Props) => { }, textLink: async () => { const url = await dialogs.prompt(_('Insert Hyperlink')); - editorRef.current.focus(); + focus('useEditorCommands::textLink', editorRef.current); if (url) wrapSelectionWithStrings(editorRef.current, '[', `](${url})`); }, insertText: (value: any) => editorRef.current.insertText(value), @@ -116,7 +117,7 @@ const useEditorCommands = (props: Props) => { }, 'editor.focus': () => { if (props.visiblePanes.indexOf('editor') >= 0) { - editorRef.current.editor.focus(); + focus('useEditorCommands::editor.focus', editorRef.current.editor); } else { // If we just call focus() then the iframe is focused, // but not its content, such that scrolling up / down diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx b/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx index 6139c4d9d..cf98577b8 100644 --- a/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx +++ b/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx @@ -32,6 +32,7 @@ import markupRenderOptions from '../../utils/markupRenderOptions'; import { DropHandler } from '../../utils/useDropHandler'; import Logger from '@joplin/utils/Logger'; import useWebViewApi from './utils/useWebViewApi'; +import { focus } from '@joplin/lib/utils/focusHandler'; const md5 = require('md5'); const { clipboard } = require('electron'); const supportedLocales = require('./supportedLocales'); @@ -204,7 +205,7 @@ const TinyMCE = (props: NoteBodyEditorProps, ref: any) => { const result = await markupToHtml.current(MarkupToHtml.MARKUP_LANGUAGE_MARKDOWN, cmd.value, markupRenderOptions({ bodyOnly: true })); editor.insertContent(result.html); } else if (cmd.name === 'editor.focus') { - editor.focus(); + focus('TinyMCE::editor.focus', editor); } else if (cmd.name === 'editor.execCommand') { if (!('ui' in cmd.value)) cmd.value.ui = false; if (!('value' in cmd.value)) cmd.value.value = null; diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/openEditDialog.ts b/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/openEditDialog.ts index c0a6d7f50..071104f16 100644 --- a/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/openEditDialog.ts +++ b/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/openEditDialog.ts @@ -1,6 +1,7 @@ import { _ } from '@joplin/lib/locale'; import { MarkupToHtml } from '@joplin/renderer'; import { TinyMceEditorEvents } from './types'; +import { focus } from '@joplin/lib/utils/focusHandler'; const taboverride = require('taboverride'); interface SourceInfo { @@ -13,7 +14,7 @@ interface SourceInfo { function dialogTextArea_keyDown(event: any) { if (event.key === 'Tab') { - window.requestAnimationFrame(() => event.target.focus()); + window.requestAnimationFrame(() => focus('openEditDialog::dialogTextArea_keyDown', event.target)); } } diff --git a/packages/app-desktop/gui/NoteEditor/commands/focusElementNoteTitle.ts b/packages/app-desktop/gui/NoteEditor/commands/focusElementNoteTitle.ts index 3b912a72f..32c4e7b5d 100644 --- a/packages/app-desktop/gui/NoteEditor/commands/focusElementNoteTitle.ts +++ b/packages/app-desktop/gui/NoteEditor/commands/focusElementNoteTitle.ts @@ -1,5 +1,6 @@ import { CommandRuntime, CommandDeclaration } from '@joplin/lib/services/CommandService'; import { _ } from '@joplin/lib/locale'; +import { focus } from '@joplin/lib/utils/focusHandler'; export const declaration: CommandDeclaration = { name: 'focusElementNoteTitle', @@ -11,7 +12,7 @@ export const runtime = (comp: any): CommandRuntime => { return { execute: async () => { if (!comp.titleInputRef.current) return; - comp.titleInputRef.current.focus(); + focus('focusElementNoteTitle', comp.titleInputRef.current); }, enabledCondition: 'oneNoteSelected', }; diff --git a/packages/app-desktop/gui/NoteEditor/commands/showLocalSearch.ts b/packages/app-desktop/gui/NoteEditor/commands/showLocalSearch.ts index fa436e1e8..8f12408a4 100644 --- a/packages/app-desktop/gui/NoteEditor/commands/showLocalSearch.ts +++ b/packages/app-desktop/gui/NoteEditor/commands/showLocalSearch.ts @@ -1,5 +1,6 @@ import { CommandRuntime, CommandDeclaration } from '@joplin/lib/services/CommandService'; import { _ } from '@joplin/lib/locale'; +import { focus } from '@joplin/lib/utils/focusHandler'; export const declaration: CommandDeclaration = { name: 'showLocalSearch', @@ -13,7 +14,7 @@ export const runtime = (comp: any): CommandRuntime => { comp.editorRef.current.execCommand({ name: 'search' }); } else { if (comp.noteSearchBarRef.current) { - comp.noteSearchBarRef.current.focus(); + focus('showLocalSearch', comp.noteSearchBarRef.current); } else { comp.setShowLocalSearch(true); } diff --git a/packages/app-desktop/gui/NoteEditor/utils/useFormNote.ts b/packages/app-desktop/gui/NoteEditor/utils/useFormNote.ts index e346abceb..204361326 100644 --- a/packages/app-desktop/gui/NoteEditor/utils/useFormNote.ts +++ b/packages/app-desktop/gui/NoteEditor/utils/useFormNote.ts @@ -14,6 +14,7 @@ import { reg } from '@joplin/lib/registry'; import ResourceFetcher from '@joplin/lib/services/ResourceFetcher'; import DecryptionWorker from '@joplin/lib/services/DecryptionWorker'; import { NoteEntity } from '@joplin/lib/services/database/types'; +import { focus } from '@joplin/lib/utils/focusHandler'; export interface OnLoadEvent { formNote: FormNote; @@ -191,7 +192,7 @@ export default function useFormNote(dependencies: HookDependencies) { requestAnimationFrame(() => { if (Setting.value(focusSettingName) === 'title') { - if (titleInputRef.current) titleInputRef.current.focus(); + if (titleInputRef.current) focus('useFormNote::handleAutoFocus', titleInputRef.current); } else { if (editorRef.current) editorRef.current.execCommand({ name: 'editor.focus' }); } diff --git a/packages/app-desktop/gui/NoteEditor/utils/useNoteSearchBar.ts b/packages/app-desktop/gui/NoteEditor/utils/useNoteSearchBar.ts index db3c91a25..1db80f0f0 100644 --- a/packages/app-desktop/gui/NoteEditor/utils/useNoteSearchBar.ts +++ b/packages/app-desktop/gui/NoteEditor/utils/useNoteSearchBar.ts @@ -1,6 +1,7 @@ import { useState, useCallback, MutableRefObject, useEffect } from 'react'; import Logger from '@joplin/utils/Logger'; import { SearchMarkers } from './useSearchMarkers'; +import { focus } from '@joplin/lib/utils/focusHandler'; const CommandService = require('@joplin/lib/services/CommandService').default; const logger = Logger.create('useNoteSearchBar'); @@ -36,7 +37,7 @@ export default function useNoteSearchBar({ noteSearchBarRef }: UseNoteSearchBarP useEffect(() => { if (showLocalSearch && noteSearchBarRef.current) { - noteSearchBarRef.current.focus(); + focus('useNoteSearchBar', noteSearchBarRef.current); } }, [showLocalSearch, noteSearchBarRef]); diff --git a/packages/app-desktop/gui/NoteList/NoteList2.tsx b/packages/app-desktop/gui/NoteList/NoteList2.tsx index 3c2cf634c..0d78505bc 100644 --- a/packages/app-desktop/gui/NoteList/NoteList2.tsx +++ b/packages/app-desktop/gui/NoteList/NoteList2.tsx @@ -24,6 +24,7 @@ import useDragAndDrop from './utils/useDragAndDrop'; import usePrevious from '../hooks/usePrevious'; import { itemIsInTrash } from '@joplin/lib/services/trash'; import Folder from '@joplin/lib/models/Folder'; +import { focus } from '@joplin/lib/utils/focusHandler'; const { connect } = require('react-redux'); const commands = { @@ -159,7 +160,9 @@ const NoteList = (props: Props) => { makeItemIndexVisible(i); if (doRefocus) { const ref = itemRefs.current[id]; - if (ref) ref.focus(); + if (ref) { + focus('NoteList::doRefocus', ref); + } } break; } diff --git a/packages/app-desktop/gui/NoteList/utils/useFocusNote.ts b/packages/app-desktop/gui/NoteList/utils/useFocusNote.ts index 70eb4a2cd..9df60aea6 100644 --- a/packages/app-desktop/gui/NoteList/utils/useFocusNote.ts +++ b/packages/app-desktop/gui/NoteList/utils/useFocusNote.ts @@ -1,5 +1,6 @@ import shim from '@joplin/lib/shim'; import { useRef, useCallback, MutableRefObject } from 'react'; +import { focus } from '@joplin/lib/utils/focusHandler'; export type FocusNote = (noteId: string)=> void; @@ -16,14 +17,14 @@ const useFocusNote = (itemRefs: MutableRefObject> if (focusItemIID.current) shim.clearInterval(focusItemIID.current); focusItemIID.current = shim.setInterval(() => { if (itemRefs.current[noteId]) { - itemRefs.current[noteId].focus(); + focus('useFocusNote1', itemRefs.current[noteId]); shim.clearInterval(focusItemIID.current); focusItemIID.current = null; } }, 10); } else { if (focusItemIID.current) shim.clearInterval(focusItemIID.current); - itemRefs.current[noteId].focus(); + focus('useFocusNote2', itemRefs.current[noteId]); } }, [itemRefs]); diff --git a/packages/app-desktop/gui/NotePropertiesDialog.tsx b/packages/app-desktop/gui/NotePropertiesDialog.tsx index e2877b28d..d2ed420ce 100644 --- a/packages/app-desktop/gui/NotePropertiesDialog.tsx +++ b/packages/app-desktop/gui/NotePropertiesDialog.tsx @@ -7,6 +7,7 @@ import Note from '@joplin/lib/models/Note'; import bridge from '../services/bridge'; import shim from '@joplin/lib/shim'; import { NoteEntity } from '@joplin/lib/services/database/types'; +import { focus } from '@joplin/lib/utils/focusHandler'; const Datetime = require('react-datetime').default; const { clipboard } = require('electron'); const formatcoords = require('formatcoords'); @@ -77,7 +78,7 @@ class NotePropertiesDialog extends React.Component { public componentDidUpdate() { if (this.state.editedKey === null) { - if (this.okButton.current) this.okButton.current.focus(); + if (this.okButton.current) focus('NotePropertiesDialog::componentDidUpdate', this.okButton.current); } } @@ -220,7 +221,7 @@ class NotePropertiesDialog extends React.Component { if ((this.refs.editField as any).openCalendar) { (this.refs.editField as any).openCalendar(); } else { - (this.refs.editField as any).focus(); + focus('NotePropertiesDialog::editPropertyButtonClick', (this.refs.editField as any)); } }, 100); } @@ -255,7 +256,7 @@ class NotePropertiesDialog extends React.Component { public async cancelProperty() { // eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied return new Promise((resolve: Function) => { - if (this.okButton.current) this.okButton.current.focus(); + if (this.okButton.current) focus('NotePropertiesDialog::focus', this.okButton.current); this.setState({ editedKey: null, editedValue: null, diff --git a/packages/app-desktop/gui/NoteSearchBar.tsx b/packages/app-desktop/gui/NoteSearchBar.tsx index 55c8ec116..4dd101145 100644 --- a/packages/app-desktop/gui/NoteSearchBar.tsx +++ b/packages/app-desktop/gui/NoteSearchBar.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import { themeStyle } from '@joplin/lib/theme'; import { _ } from '@joplin/lib/locale'; +import { focus } from '@joplin/lib/utils/focusHandler'; interface Props { themeId: number; @@ -32,6 +33,8 @@ class NoteSearchBar extends React.Component { this.previousButton_click = this.previousButton_click.bind(this); this.nextButton_click = this.nextButton_click.bind(this); this.closeButton_click = this.closeButton_click.bind(this); + + // eslint-disable-next-line no-restricted-properties this.focus = this.focus.bind(this); this.backgroundColor = undefined; @@ -125,7 +128,7 @@ class NoteSearchBar extends React.Component { } public focus() { - (this.refs.searchInput as any).focus(); + focus('NoteSearchBar::focus', this.refs.searchInput as any); (this.refs.searchInput as any).select(); } diff --git a/packages/app-desktop/gui/NoteTextViewer.tsx b/packages/app-desktop/gui/NoteTextViewer.tsx index c2dbd3637..b60dc7ff3 100644 --- a/packages/app-desktop/gui/NoteTextViewer.tsx +++ b/packages/app-desktop/gui/NoteTextViewer.tsx @@ -1,6 +1,7 @@ import PostMessageService, { MessageResponse, ResponderComponentType } from '@joplin/lib/services/PostMessageService'; import * as React from 'react'; import { reg } from '@joplin/lib/registry'; +import { focus } from '@joplin/lib/utils/focusHandler'; interface Props { // eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied @@ -119,7 +120,7 @@ export default class NoteTextViewerComponent extends React.Component public focus() { if (this.webviewRef_.current) { - this.webviewRef_.current.focus(); + focus('NoteTextViewer::focus', this.webviewRef_.current); } } diff --git a/packages/app-desktop/gui/PromptDialog.tsx b/packages/app-desktop/gui/PromptDialog.tsx index e28bdf2d4..4757a25a1 100644 --- a/packages/app-desktop/gui/PromptDialog.tsx +++ b/packages/app-desktop/gui/PromptDialog.tsx @@ -6,6 +6,7 @@ const Datetime = require('react-datetime').default; import CreatableSelect from 'react-select/creatable'; import Select from 'react-select'; import makeAnimated from 'react-select/animated'; +import { focus } from '@joplin/lib/utils/focusHandler'; interface Props { themeId: number; defaultValue: any; @@ -67,7 +68,7 @@ export default class PromptDialog extends React.Component { } public componentDidUpdate() { - if (this.focusInput_ && this.answerInput_.current) this.answerInput_.current.focus(); + if (this.focusInput_ && this.answerInput_.current) focus('PromptDialog::componentDidUpdate', this.answerInput_.current); this.focusInput_ = false; } diff --git a/packages/app-desktop/gui/SearchBar/SearchBar.tsx b/packages/app-desktop/gui/SearchBar/SearchBar.tsx index d759f139f..5cb8b06b1 100644 --- a/packages/app-desktop/gui/SearchBar/SearchBar.tsx +++ b/packages/app-desktop/gui/SearchBar/SearchBar.tsx @@ -8,6 +8,7 @@ import uuid from '@joplin/lib/uuid'; const { connect } = require('react-redux'); import Note from '@joplin/lib/models/Note'; import { AppState } from '../../app.reducer'; +import { blur, focus } from '@joplin/lib/utils/focusHandler'; const debounce = require('debounce'); const styled = require('styled-components').default; @@ -117,7 +118,7 @@ function SearchBar(props: Props) { const onKeyDown = useCallback((event: any) => { if (event.key === 'Escape') { - if (document.activeElement) (document.activeElement as any).blur(); + if (document.activeElement) blur('SearchBar::onKeyDown', document.activeElement as any); void onExitSearch(); } }, [onExitSearch]); @@ -127,7 +128,7 @@ function SearchBar(props: Props) { void onExitSearch(); } else { setSearchStarted(true); - props.inputRef.current.focus(); + focus('SearchBar::onSearchButtonClick', props.inputRef.current); props.dispatch({ type: 'FOCUS_SET', field: 'globalSearch', diff --git a/packages/app-desktop/gui/Sidebar/Sidebar.tsx b/packages/app-desktop/gui/Sidebar/Sidebar.tsx index 80008a28c..228d10a21 100644 --- a/packages/app-desktop/gui/Sidebar/Sidebar.tsx +++ b/packages/app-desktop/gui/Sidebar/Sidebar.tsx @@ -29,6 +29,7 @@ import { RuntimeProps } from './commands/focusElementSideBar'; const { connect } = require('react-redux'); import { renderFolders, renderTags } from '@joplin/lib/components/shared/side-menu-shared'; import { getTrashFolderIcon, getTrashFolderId } from '@joplin/lib/services/trash'; +import { focus } from '@joplin/lib/utils/focusHandler'; const { themeStyle } = require('@joplin/lib/theme'); const bridge = require('@electron/remote').require('./bridge').default; const Menu = bridge().Menu; @@ -674,7 +675,7 @@ const SidebarComponent = (props: Props) => { id: focusItem.id, }); - focusItem.ref.current.focus(); + focus('SideBar::onKeyDown', focusItem.ref.current); } if (keyCode === 9) { diff --git a/packages/app-desktop/gui/Sidebar/commands/focusElementSideBar.ts b/packages/app-desktop/gui/Sidebar/commands/focusElementSideBar.ts index 624a240c5..7c0913327 100644 --- a/packages/app-desktop/gui/Sidebar/commands/focusElementSideBar.ts +++ b/packages/app-desktop/gui/Sidebar/commands/focusElementSideBar.ts @@ -2,6 +2,7 @@ import { CommandRuntime, CommandDeclaration, CommandContext } from '@joplin/lib/ import { _ } from '@joplin/lib/locale'; import layoutItemProp from '../../ResizableLayout/utils/layoutItemProp'; import { AppState } from '../../../app.reducer'; +import { focus } from '@joplin/lib/utils/focusHandler'; export const declaration: CommandDeclaration = { name: 'focusElementSideBar', @@ -24,10 +25,10 @@ export const runtime = (props: RuntimeProps): CommandRuntime => { const item = props.getSelectedItem(); if (item) { const anchorRef = props.anchorItemRefs.current[item.type][item.id]; - if (anchorRef) anchorRef.current.focus(); + if (anchorRef) focus('focusElementSideBar1', anchorRef.current); } else { const anchorRef = props.getFirstAnchorItemRef('folder'); - if (anchorRef) anchorRef.current.focus(); + if (anchorRef) focus('focusElementSideBar2', anchorRef.current); } } }, diff --git a/packages/app-desktop/services/plugins/UserWebview.tsx b/packages/app-desktop/services/plugins/UserWebview.tsx index ecdada73a..d914d8aae 100644 --- a/packages/app-desktop/services/plugins/UserWebview.tsx +++ b/packages/app-desktop/services/plugins/UserWebview.tsx @@ -9,6 +9,7 @@ import useWebviewToPluginMessages from './hooks/useWebviewToPluginMessages'; import useScriptLoader from './hooks/useScriptLoader'; import Logger from '@joplin/utils/Logger'; import styled from 'styled-components'; +import { focus } from '@joplin/lib/utils/focusHandler'; const logger = Logger.create('UserWebview'); @@ -99,7 +100,7 @@ function UserWebview(props: Props, ref: any) { } }, focus: function() { - if (viewRef.current) viewRef.current.focus(); + if (viewRef.current) focus('UserWebView::focus', viewRef.current); }, }; }); diff --git a/packages/app-desktop/services/plugins/UserWebviewDialog.tsx b/packages/app-desktop/services/plugins/UserWebviewDialog.tsx index 39c2cb7ba..dc1c4e36f 100644 --- a/packages/app-desktop/services/plugins/UserWebviewDialog.tsx +++ b/packages/app-desktop/services/plugins/UserWebviewDialog.tsx @@ -5,6 +5,7 @@ import PluginService from '@joplin/lib/services/plugins/PluginService'; import WebviewController from '@joplin/lib/services/plugins/WebviewController'; import UserWebview, { Props as UserWebviewProps } from './UserWebview'; import UserWebviewDialogButtonBar from './UserWebviewDialogButtonBar'; +import { focus } from '@joplin/lib/utils/focusHandler'; const styled = require('styled-components').default; interface Props extends UserWebviewProps { @@ -101,7 +102,7 @@ export default function UserWebviewDialog(props: Props) { // We focus the dialog once it's ready to make sure that the ESC/Enter // keyboard shortcuts are working. // https://github.com/laurent22/joplin/issues/4474 - if (webviewRef.current) webviewRef.current.focus(); + if (webviewRef.current) focus('UserWebviewDialog', webviewRef.current); }, []); return ( diff --git a/packages/app-desktop/tsconfig.json b/packages/app-desktop/tsconfig.json index 6a82e0316..863c27743 100644 --- a/packages/app-desktop/tsconfig.json +++ b/packages/app-desktop/tsconfig.json @@ -2,7 +2,7 @@ "extends": "../../tsconfig.json", "include": [ "**/*.ts", - "**/*.tsx", + "**/*.tsx", "../lib/utils/focusHandler.ts", ], "exclude": [ "**/node_modules", diff --git a/packages/app-mobile/components/NoteEditor/EditLinkDialog.tsx b/packages/app-mobile/components/NoteEditor/EditLinkDialog.tsx index 2e01a3718..7eb0545c3 100644 --- a/packages/app-mobile/components/NoteEditor/EditLinkDialog.tsx +++ b/packages/app-mobile/components/NoteEditor/EditLinkDialog.tsx @@ -11,6 +11,7 @@ import { _ } from '@joplin/lib/locale'; import { EditorControl } from './types'; import { useCallback } from 'react'; import SelectionFormatting from '@joplin/editor/SelectionFormatting'; +import { focus } from '@joplin/lib/utils/focusHandler'; interface LinkDialogProps { editorControl: EditorControl; @@ -100,7 +101,7 @@ const EditLinkDialog = (props: LinkDialogProps) => { autoFocus onSubmitEditing={() => { - linkInputRef.current.focus(); + focus('EditLinkDialog::onSubmitEditing', linkInputRef.current); }} onChangeText={(text: string) => setLinkLabel(text)} /> diff --git a/packages/app-mobile/components/screens/Note.tsx b/packages/app-mobile/components/screens/Note.tsx index 192c9f8a3..4093b2514 100644 --- a/packages/app-mobile/components/screens/Note.tsx +++ b/packages/app-mobile/components/screens/Note.tsx @@ -61,6 +61,7 @@ import { getDisplayParentTitle } from '@joplin/lib/services/trash'; import { PluginStates } from '@joplin/lib/services/plugins/reducer'; import pickDocument from '../../utils/pickDocument'; import debounce from '../../utils/debounce'; +import { focus } from '@joplin/lib/utils/focusHandler'; const urlUtils = require('@joplin/lib/urlUtils'); const emptyArray: any[] = []; @@ -1328,13 +1329,8 @@ class NoteScreenComponent extends BaseScreenComponent implements B // Avoid writing `this.titleTextFieldRef.current` -- titleTextFieldRef may // be undefined. if (fieldToFocus === 'title' && this.titleTextFieldRef?.current) { - this.titleTextFieldRef.current.focus(); + focus('Note::focusUpdate', this.titleTextFieldRef.current); } - // if (fieldToFocus === 'body' && this.markdownEditorRef.current) { - // if (this.markdownEditorRef.current) { - // this.markdownEditorRef.current.focus(); - // } - // } } private async folderPickerOptions_valueChanged(itemValue: any) { diff --git a/packages/editor/CodeMirror/editorCommands/editorCommands.ts b/packages/editor/CodeMirror/editorCommands/editorCommands.ts index 86ad605da..8666b324b 100644 --- a/packages/editor/CodeMirror/editorCommands/editorCommands.ts +++ b/packages/editor/CodeMirror/editorCommands/editorCommands.ts @@ -11,6 +11,7 @@ import swapLine, { SwapLineDirection } from './swapLine'; import duplicateLine from './duplicateLine'; import sortSelectedLines from './sortSelectedLines'; import { closeSearchPanel, findNext, findPrevious, openSearchPanel, replaceAll, replaceNext } from '@codemirror/search'; +import { focus } from '@joplin/lib/utils/focusHandler'; export type EditorCommandFunction = (editor: EditorView, ...args: any[])=> void|any; @@ -22,7 +23,7 @@ const editorCommands: Record = { [EditorCommandType.Undo]: undo, [EditorCommandType.Redo]: redo, [EditorCommandType.SelectAll]: selectAll, - [EditorCommandType.Focus]: editor => editor.focus(), + [EditorCommandType.Focus]: editor => focus('editorCommands::focus', editor), [EditorCommandType.ToggleBolded]: toggleBolded, [EditorCommandType.ToggleItalicized]: toggleItalicized, diff --git a/packages/lib/utils/focusHandler.ts b/packages/lib/utils/focusHandler.ts new file mode 100644 index 000000000..6fc8db075 --- /dev/null +++ b/packages/lib/utils/focusHandler.ts @@ -0,0 +1,41 @@ +// The purpose of this handler is to have all focus/blur calls go through the same place, which +// makes it easier to log what happens. This is useful when one unknown component is stealing focus +// from another component. Potentially it could also be used to resolve conflict situations when +// multiple components try to set the focus at the same time. + +import Logger from '@joplin/utils/Logger'; + +const logger = Logger.create('setFocus'); + +enum ToggleFocusAction { + Focus = 'focus', + Blur = 'blur', +} + +interface FocusableElement { + focus: ()=> void; + blur: ()=> void; +} + +const toggleFocus = (source: string, element: FocusableElement, action: ToggleFocusAction) => { + if (!element) { + logger.warn(`Tried action "${action}" on an undefined element: ${source}`); + return; + } + + if (!element[action]) { + logger.warn(`Element does not have a "${action}" method: ${source}`); + return; + } + + logger.debug(`Action "${action}" from "${source}"`); + element[action](); +}; + +export const focus = (source: string, element: any) => { + toggleFocus(source, element, ToggleFocusAction.Focus); +}; + +export const blur = (source: string, element: any) => { + toggleFocus(source, element, ToggleFocusAction.Blur); +}; diff --git a/packages/pdf-viewer/PdfDocument.ts b/packages/pdf-viewer/PdfDocument.ts index d592ba2d7..a1f4cca6f 100644 --- a/packages/pdf-viewer/PdfDocument.ts +++ b/packages/pdf-viewer/PdfDocument.ts @@ -146,7 +146,8 @@ export default class PdfDocument { frame.contentWindow.onafterprint = () => { frame.remove(); }; - frame.focus(); + console.warn('frame.focus() has been disabled!! Use focusHandler instead'); + // frame.focus(); frame.contentWindow.print(); }; frame.src = this.url as string;