Desktop: Accessibility: Add a new shortcut to set focus to editor toolbar #11764

pedr · 2025-02-01T13:28:54Z

Related to #10795

Summary

When using the RTE only with the keyboard the tab was skipping the toolbar items. TinyMCE has a shortcut for it (Alt+F10) but it is not accessible on all systems.

On this PR I'm adding a new command that should fix that by setting the focus to the first item of the toolbar on the RTE or the Markdown editor, if that is the active one.

Potential problem:
While the feature it is working, it seems like sometimes the element of the toolbar with focus is not set properly. I would guess that has to do with how React re-renders the elements. To emulate the faulty behaviour:

Select Markdown/RTE editor
Set focus to the note body
Select the other editor
Set focus to the note body
Use the shortcut
Focus is on the toolbar but, sometimes, the element is not highlighted

WCAG

Guideline 2.1 Make all functionality available from a keyboard

Testing

Testing Markdown edtior

On Markdown editor
Put focus on the note body
Press Alt+F6
See focus being set to toolbar

Testing Rich text editor

On Rich text editor
Put focus on the note body
Press Alt+F6
See focus being set to toolbar

personalizedrefrigerator · 2025-02-01T18:39:38Z

packages/app-desktop/gui/NoteEditor/commands/focusToolbar.ts

+			}
+
+			const boldOnMarkdown = dependencies.editorContainerDomElement.querySelector(
+				'.button.toolbar-button[title=\'Bold\']',


Thank you for working on this!

Does this selector work in non-English locales? (When the title isn't "bold").

Thanks for the suggestion, you are right! I tried to avoid using deep selectors because I thought they could break more easily, but I didn't think about the language impact.

I updated the selector, it would be nice to hear what do you think about it.

personalizedrefrigerator · 2025-02-03T16:06:57Z

packages/app-desktop/gui/NoteEditor/commands/focusToolbar.ts

+			const boldOnMarkdown = dependencies.editorContainerDomElement.querySelector(
+				'#CodeMirrorToolbar div.group:nth-of-type(2) button.button.toolbar-button:nth-of-type(1)',
+			);


Since the Markdown toolbar is a custom component, here's one way this might be made simpler:

From the toolbar code: Add an -autofocus class (or similar) to the button that should be autofocused (probably near this line).

In focusToolbar.ts: Select the button to be focused with .button.-autofocus.

Alternatively, the button that should be autofocused might already be the only button that doesn't have tabindex=-1 (the Markdown toolbar should follow the toolbar pattern).

It seems like that Bold is only tabindex=0 when Forward and Backwards are disabled

I'm going to try to do something like this with a class directly where we want the focus to be, I guess that would work better and be less refactor-prone.

Thanks for the suggestions!

Instead of using another class I opted for using the [tabindex="0"]

I also use a :not(.disabled) so we skip the backwards and forwards button if they are disabled

personalizedrefrigerator · 2025-02-03T16:08:28Z

packages/app-desktop/gui/NoteEditor/commands/focusToolbar.ts

+			const boldOnRTE = dependencies.editorContainerDomElement.querySelector(
+				'.tox-toolbar-overlord div.tox-toolbar__group:nth-of-type(1) button:nth-of-type(1)',
+			);


Possible simplification: This may still work if querySelector doesn't uniquely resolve to an element. For example,

Suggested change

const boldOnRTE = dependencies.editorContainerDomElement.querySelector(

'.tox-toolbar-overlord div.tox-toolbar__group:nth-of-type(1) button:nth-of-type(1)',

);

const boldOnRTE = dependencies.editorContainerDomElement.querySelector(

'.tox-toolbar__group button',

);

pedr · 2025-02-03T18:45:49Z

I tested using { focusVisible: true } to fix the problem where the element was focused but we had no visual effect, but that didn't make any difference.
It seems like it has never been supported on Chrome, I guess that also means that we don't have support on Electron, right?
https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/focus#focusvisible

personalizedrefrigerator · 2025-02-03T19:08:16Z

It seems like it has never been supported on Chrome, I guess that also means that we don't have support on Electron, right?

That would mean it's not supported in Electron. Thank you for checking!

laurent22

@pedr, there's a also a focusElement command - please add yours to it for consistency.

laurent22 · 2025-02-06T17:41:39Z

packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx

@@ -814,6 +815,11 @@ const TinyMCE = (props: NoteBodyEditorProps, ref: any) => {
 					editor.addShortcut('Meta+Shift+7', '', () => editor.execCommand('InsertOrderedList'));
 					editor.addShortcut('Meta+Shift+8', '', () => editor.execCommand('InsertUnorderedList'));
 					editor.addShortcut('Meta+Shift+9', '', () => editor.execCommand('InsertJoplinChecklist'));
+					editor.addShortcut(
+						KeymapService.instance().getAccelerator('focusToolbar'),
+						'Focus toolbar',


What is the purpose of the "Focus toolbar" string here? Can it be omitted like in the other addShortcut calls? (especially since it's not translated)

laurent22 · 2025-02-06T17:43:14Z

packages/app-desktop/gui/NoteEditor/NoteEditor.tsx

 			setShowRevisions,
 			isInFocusedDocument: () => {
 				return containerRef.current?.ownerDocument?.hasFocus();
 			},
+			editorContainerDomElement: containerRef.current,


editorContainerDomElement => editorContainerElement

laurent22 · 2025-02-06T17:45:33Z

packages/lib/services/KeymapService.ts

@@ -112,6 +113,7 @@ const defaultKeymapItems = {
 		{ accelerator: 'Ctrl+Alt+3', command: 'switchProfile3' },
 		{ accelerator: 'Ctrl+Alt+N', command: 'openNoteInNewWindow' },
 		{ accelerator: 'Ctrl+M', command: 'toggleTabMovesFocus' },
+		{ accelerator: 'Alt+F6', command: 'focusToolbar' },


Could you please put this shortcut below all the other focusXxx shortcuts?

Also what was the reason for choosing a completely different keyboard shortcut for this? All the other ones are "Ctrl+Shift+Something"

laurent22 · 2025-02-06T17:47:17Z

packages/app-desktop/gui/NoteEditor/commands/focusToolbar.ts

@@ -0,0 +1,36 @@
+import { CommandRuntime, CommandDeclaration } from '@joplin/lib/services/CommandService';


Whenever implementing a change, please check how other similar things are named. For example, all focus shortcuts are named "focusElement...". So yours should also be called "focusElementToolbar" for consistency. Could you please update the name?

…bar-shorcut

pedr · 2025-02-07T20:35:55Z

I finally update this PR. I end up changing a lot because I realized that there is no reason why I should be calling the command from the editor specifically (Before I would call from NoteEditor and TinyMCE).

I end up adding the command to the useWindowCommandHandler.

pedr added 3 commits January 31, 2025 11:54

WIP hardcode shortcut on tinymce

e3233b9

add a shortcut to set focus to editor toolbar

f7c1557

removing dependency that is not required anymore

de3f9cc

pedr added desktop All desktop platforms accessibility Related to accessibility labels Feb 1, 2025

pedr requested review from laurent22 and personalizedrefrigerator February 1, 2025 13:28

pedr mentioned this pull request Feb 1, 2025

Accessibility: Comply with WCAG 2.2 #10795

Open

personalizedrefrigerator reviewed Feb 1, 2025

View reviewed changes

changing querySelector to not be dependable of locale

46442e9

personalizedrefrigerator reviewed Feb 3, 2025

View reviewed changes

simplifying css selector

8cfbf9a

renaming variables and improving css selector for markdown toolbar

9288890

laurent22 reviewed Feb 6, 2025

View reviewed changes

pedr added 6 commits February 7, 2025 10:05

removing unnecessary naming

31ce7f1

updating default keymapping

32e443c

remove command from editor

bc39b4b

refactoring command to not be called from note editor

427b1d8

Merge remote-tracking branch 'upstream/dev' into change-focus-on-tool…

ced0c11

…bar-shorcut

removing unnecessary changes

81550d4

laurent22 merged commit 1230e1b into laurent22:dev Feb 7, 2025
7 checks passed

pedr deleted the change-focus-on-toolbar-shorcut branch February 7, 2025 20:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Desktop: Accessibility: Add a new shortcut to set focus to editor toolbar #11764

Desktop: Accessibility: Add a new shortcut to set focus to editor toolbar #11764

pedr commented Feb 1, 2025

personalizedrefrigerator Feb 1, 2025

pedr Feb 3, 2025

personalizedrefrigerator Feb 3, 2025

pedr Feb 3, 2025

pedr Feb 3, 2025

personalizedrefrigerator Feb 3, 2025

pedr commented Feb 3, 2025

personalizedrefrigerator commented Feb 3, 2025

laurent22 left a comment

laurent22 Feb 6, 2025

laurent22 Feb 6, 2025

laurent22 Feb 6, 2025

laurent22 Feb 6, 2025

pedr commented Feb 7, 2025

		@@ -0,0 +1,36 @@
		import { CommandRuntime, CommandDeclaration } from '@joplin/lib/services/CommandService';

Desktop: Accessibility: Add a new shortcut to set focus to editor toolbar #11764

Desktop: Accessibility: Add a new shortcut to set focus to editor toolbar #11764

Conversation

pedr commented Feb 1, 2025

Summary

WCAG

Testing

Testing Markdown edtior

Testing Rich text editor

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pedr commented Feb 3, 2025

personalizedrefrigerator commented Feb 3, 2025

laurent22 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pedr commented Feb 7, 2025