Desktop: Accessibility: Improve focus handling for plugin and prompt dialogs #10801
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ould have been out of the scope of this PR)
Comment on lines
-26
to
+27
| background-color: rgba(0,0,0,0.6); | ||
| background-color: rgba(0,0,0,0.5); |
There was a problem hiding this comment.
This CSS change prevents this pull request from changing the backdrop color for plugin dialogs.
Comment on lines
-247
to
+243
| // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied | ||
| public onKeyDown(event: any) { | ||
| if (event.keyCode === 27) { // ESCAPE | ||
| this.props.dispatch({ | ||
| pluginName: PLUGIN_NAME, | ||
| type: 'PLUGINLEGACY_DIALOG_SET', | ||
| open: false, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied | ||
| private modalLayer_onClick(event: any) { | ||
| if (event.currentTarget === event.target) { | ||
| this.props.dispatch({ | ||
| pluginName: PLUGIN_NAME, | ||
| type: 'PLUGINLEGACY_DIALOG_SET', | ||
| open: false, | ||
| }); | ||
| } | ||
| } | ||
| private modalLayer_onDismiss = () => { |
There was a problem hiding this comment.
Dismissing the dialog with escape or clicking outside of the dialog is now handled by Dialog.tsx. As such, this logic has been removed from GotoAnything.tsx.
|
That looks good, thanks for implementing this! The difference before/after looks good to me. Rounded corners on dialogs are fine I guess. |
This was referenced Oct 7, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This pull request improves focus handling for additional dialog types in a way similar to #10779. Specifically, for
This is done by migrating these dialogs to the
gui/Dialogcomponent. Since #10779,Dialogprevents items behind the dialog modal container from having keyboard focus.Why?
This change brings Joplin closer to compliance with WCAG 2.2 (issue #10795).
From the WCAG 2.2's "Understanding SC 2.4.11: Focus Not Obscured (Minimum) (Level AA)",
Before this pull request, it was possible for focus to be hidden by plugin, prompt, statistics, and the go-to-anything dialogs. This pull request uses a modal
<dialog>element to prevent keyboard focus from escaping, and thus potentially being hidden by, visible dialogs.Stylistic changes
Switching to the
Dialogcomponent also changes the CSS used to style the dialog container. These changes are shown below.Prompt dialog/text: Rounded corners and slightly more padding above:
Prompt dialog/date time: Rounded corners, the date time input now has focus by default. This currently causes the calendar to be visible by default. Previously, keyboard focus remained on the control that opened the dialog.
Plugin dialogs/one dialog open: When just one dialog is open, styles should be roughly the same:
Plugin dialogs/multiple dialogs open: When multiple dialogs are opened, the last-opened dialog now needs to be dismissed before the other dialog(s) can be used:
Go-to-anything dialog: Rounded corners:
Testing
This pull request includes automated tests for the following:
Dialog.tsx(rather than inGotoAnything.tsx). As such, this also tests the relevant logic inDialog.tsx.Dialog.tsx.:setTagsfrom "Go to anything".The following manual testing has also been done on Ubuntu 24.04:
joplin-plugin-contentusing the developer tools (set to 500px).:insertDrawing(from the Freehand Drawing plugin)