Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Desktop: Improve notification accessibility #11752

Open
wants to merge 22 commits into
base: dev
Choose a base branch
from

Conversation

personalizedrefrigerator
Copy link
Collaborator

@personalizedrefrigerator personalizedrefrigerator commented Jan 30, 2025

Summary

Goal: Make toast notifications shown in the desktop app more accessible and easier to create/update.

This pull request replaces Notyf with a custom component. This allows fixing a few issues:

  • Previously, there was no way to review old notifications that had auto-dismissed and it was difficult to move keyboard focus to these notifications.
    • This pull request adds a "Recent notifications" dialog (currently only openable from the command palette). This allows reviewing notifications without a time limit and moves keyboard focus to the notifications list.
  • Previously, notifications could only be shown in the main window. This pull request allows notifications to be shown in all windows.

Note

This pull request makes an effort for the new notifications to appear roughly the same as the Notyf notifications.

Fixes #11742. Related to #10795.

Screenshots

Screen recording

The below screen recording shows two "moved to the trash" notifications being shown, then auto-dismissing after several seconds. When both are visible, one notification is shown above the other, with a vertical gap between the two:

Screencast.from.2025-02-19.11-34-35.webm

Before/after

Before After
screenshot: Trash notification with check mark screenshot: Trash notification text is slightly larger, the check inside the checked circle icon is green (matching the background) instead of black

Accessibility

Timing Adjustable (WCAG 2.2 SC 2.2.1)

The "Timing Adjustable" success criterion states:

For each time limit that is set by the content, at least one of the following is true:

  • Turn off
    The user is allowed to turn off the time limit before encountering it; or

  • Adjust
    The user is allowed to adjust the time limit before encountering it over a wide range that is at least ten times the length of the default setting; or

  • Extend
    The user is warned before time expires and given at least 20 seconds to extend the time limit with a simple action (for example, "press the space bar"), and the user is allowed to extend the time limit at least ten times; or

  • Real-time Exception
    The time limit is a required part of a real-time event (for example, an auction), and no alternative to the time limit is possible; or

  • Essential Exception
    The time limit is essential and extending it would invalidate the activity; or

  • 20 Hour Exception
    The time limit is longer than 20 hours.

Warning

This pull request's "Recent notifications" dialog may not be sufficient for this. Although it does allow users to review recent notifications without a time limit,

  1. It's difficult to discover.
  2. The notifications list (outside the dialog) still includes notifications that auto-hide.
  • Joplin's toast notifications auto-dismiss after a few seconds. This is a time limit set by Joplin. Thus, some change needs to be made.

  • For comparison, VSCode also shows popup notifications. However, actionable notifications don't seem to auto-dismiss. Instead, they must be dismissed manually by the user. Unactionable progress notifications do auto-dismiss.

Screen reader/keyboard accessibility

Prior to this pull request, it was very difficult to move the keyboard focus to a popup notification. This is necessary, for example, to select an option when auto-updates are enabled, or to click "cancel" when deleting to the trash.

VSCode has a notification center with a list of recent notifications. Opening this notification center with the keyboard causes focus to jump to the first notification:
screenshot: Notifications dialog shows a single notification. A small blue arrow indicates that the dialog is associated with the bell icon at the bottom right corner of the screen

Additionally, a notification center is an approach discussed in this blog post and this other blog post.

This pull request adds a notification center to Joplin (this was removed):
screenshot: Recent notifications dialog shows an update notification

This notification center can currently only be opened from the command palette. Ideally, however, there would be a button in the UI (perhaps similar to VSCode?) to open this dialog. Suggestions on what to do here would be welcome!

Important

The notification history screen has been removed for now.

Details

This pull request replaces Notyf with a custom component with similar styles/animations to Notyf. Unlike Notyf,

  • API:
    • React components can be shown in the notifications. Notyf seems to have required raw HTML.
    • Notifications are automatically shown in all windows.
  • Accessibility:
    • The custom notifications should be more accessible. See the "to-do" section.

To-do

  • Don't announce raw HTML when deleting a note (Desktop: Screen reader accessibility: Deleting a note announces raw HTML #11742)
  • Remove notifications from the DOM after they've been dismissed.
  • Allow notifications to be shown in secondary windows.
  • Replace links that act like buttons with <buttons that are styled to look like links.
  • Meet Timing Adjustable (WCAG 2.2 SC 2.2.1)
    • In VSCode, there's a command to show old notifications.
    • This pull request does include a command to show old notifications. However, it isn't very discoverable.
    • This blog post discusses making toasts accessible (referencing specific WCAG 2.1 guidelines).
  • Check whether alert is the correct role for these notifications.
    • At least with Orca on Linux, the "Cancel" button isn't announced with the alert's other content.
  • Add screenshots and a "Testing plan" section to the pull request description.

Testing plan

Note: This pull request adds an automated test that verifies that a "deleted" notification is shown when deleting to the trash.

  1. Patch Joplin such that the update notification is visible.
  2. Enable the Orca screen reader.
  3. Delete a note to the trash.
  4. Verify that the deletion was announced.
  5. Open the recent notifications dialog by typing :showNotificationDialog in the command palette.
  6. Verify that the trash and deletion notifications are listed.
Screen reader transcript
navigation Note list
Notes multi dash-select List with 2 items
For each time limit that is set by the content, at least one of the following is.
down
editor dash-test.
delete
For each time limit that is set by the content, at least one of the following is.
The note was successfully moved to the trash.
left control
left shift
P
 colon 
Type a note title or part of its content to jump to it.
 Or type  number  followed by a tag name, or  at  followed by a notebook name.
 Or type  colon  to search for commands.
 entry  colon 
Search results List with 11 items
Attach file  left paren(attachFile right paren)
n
Change application layout  left paren(toggleLayoutMoveMode right paren)
o
t
Delete note  left paren(deleteNote right paren)
i
f
Recent notifications  left paren(showNotificationDialog right paren)
i
c
a
t
i
o
n
s
return
List with 2 items
See changelog button
Browse mode
;
l
Wrapping to top.
l
Wrapping to top.
up
Recent notifications heading 1
up
down
List with 2 items
A new update  left paren(test right paren) is available.
down
See changelog button
down
Restart now button
Update later button
down
Success image
down
The note was successfully moved to the 
down
trash.
down
Cancel button
down
Done button
down
down
up
List with 2 items
Cancel button
return
Cancelling...
Joplin  left paren(DEV  dash-  slash home slash self slash  dot config slash joplindev dash-desktop slash profile dash-yo35nu8f right paren)
Joplin document web
down
A new update  left paren(test right paren) is available.

Issues observed during the above test:

  • If multiple items are deleted, their deletion notifications all have buttons with the same label ("Cancel").
  • Clicking "Cancel" from the dialog moves focus back to the dialog.
  • The initial "note deleted" announcement doesn't announce that it's possible to cancel the deletion.
    • The "Cancel" button is skipped.

Comment on lines -95 to -99
const messageHtml = `
<div class="update-notification" style="color: ${theme.color2};">
${noUpdateMessageHtml}
const notification = popupManager.createPopup(() => (
<div className='update-notification'>
{_('No updates available')}
</div>
`;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API: With this change, notifications use React components instead of raw HTML.

@personalizedrefrigerator personalizedrefrigerator added desktop All desktop platforms accessibility Related to accessibility labels Jan 30, 2025
GitHub's diff is having trouble with the changes to the licenses.md file
(probably due to the file size).
Adds a command that shows a dialog with recent notifications. Inspiered
by VSCode.

Goal: Partially satisfy the "provide enough time" requirement.
notifications (to show the decorative background element)
@personalizedrefrigerator personalizedrefrigerator marked this pull request as ready for review February 3, 2025 14:13
@laurent22
Copy link
Owner

I wish this change had been discussed in advance because it's huge and it's not clear it's worth the effort. Not to mention that's a lot of extra code that will need to be maintained vs a lib that did most of what we needed for free.

@personalizedrefrigerator
Copy link
Collaborator Author

  • Post before/after screenshots.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility Related to accessibility desktop All desktop platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Desktop: Screen reader accessibility: Deleting a note announces raw HTML
2 participants