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,Mobile: Accessibility: Announce formatting changes to the Markdown editor for screen readers #11441

Conversation

personalizedrefrigerator
Copy link
Collaborator

@personalizedrefrigerator personalizedrefrigerator commented Nov 26, 2024

Summary

This pull request causes screen readers to describe certain formatting changes in the Markdown editor after they are made. For example, pressing ctrl-b in an empty now announces "four star characters, added bold Markup" (Web client/Chromium/Linux+Orca). Previously, it only announced "four star characters".

Related to #10795.

Example announcements

  • Adding new italic text: Announces Added italic markup
  • Adding a level 2 header on the current line: Added header level 2 markup at the start of the current line.
  • Indenting 5 lines: Added indent markup on 5 lines.
  • Deindenting 5 lines: Removed indent markup on 5 lines.

Implementation notes

On mobile, code in packages/editor runs in a WebView is bundled separately from the main application. If it uses @joplin/lib/locale directly, the bundle size increases from roughly 2 MB to 5 MB. To avoid this size increase, this pull request:

  • Uses a different implementation of the _(...) localization function in packages/editor.
  • When building the application, packages/editor uses the TypeScript compiler API to find all uses of _(...) in the editor package. The format strings passed to _(...) are then written to
  • When loading the editor, strings from the current locale used in packages/editor are saved to localizationPatterns.ts.
  • Just before creating the editor, the localizations for all strings in localizations/localizationPatterns.ts are looked up and sent to the editor's localization logic.

To summarize, at buildtime:

flowchart TD
      Find["Find use of <code>_()</code>"]
            WriteToFile["Save strings from <code>_()</code>s to <code>localizationPatterns.ts</code>"]
            Find-->WriteToFile
Loading

Then at runtime:

flowchart TD
		subgraph Main
			Localize["Localize strings from <code>localizationPatterns.ts</code>"]
		end
		subgraph WebView
			Configure["Configure _() to look up strings"]
			BuildEditor["Start the editor"]
			Configure --> BuildEditor
		end
		Localize-->Configure
Loading

Testing plan

Desktop/Fedora 41/Ocra:

  1. Start the Orca screen reader.
  2. Create a new note with ctrl-n.
  3. Press ctrl-b.
  4. Verify that Orca reads "added four star characters" then "added bold markup".
  5. Type "test"
  6. Press ctrl-b.
  7. Verify that Orca reads "moved cursor out of bold markup".
  8. Press ctrl-alt--.
  9. Verify that Orca reads "Added one bullet list items".
  10. Repeat step 8.
  11. Verify that Orca reads "Removed one bullet list items".
  12. Move focus to the "header" button in the toolbar.
  13. Verify that Orca explains that header markup has been added to the beginning of the line.

Web/Chromium/Linux+Orca:

  1. Start Joplin web.
  2. Enable the screen reader.
  3. Create a new note.
  4. Move focus to the note editor.
  5. Press ctrl-b.
  6. Verify that "four star characters" then "added bold markup" is read.
  7. Select and delete everything.
  8. Press ctrl-`.
  9. Verify that "added inline code markup" is read by the screen reader.
  10. Press ctrl-` again.
  11. Verify that "converted inline code to block code" is read.

Additional testing:

  • In packages/app-mobile/components/NoteEditor.tsx, replace makeLocalizations(rawStringInCurrentLocale) with makeLocalizations(() => 'PASS!').
  • Reload Chromium.
  • Open a note and press ctrl-b.
  • Verify that Orca reads "four star characters", then "pass".

This commit announces changes (e.g. "added block code") made by most
Markdown commands. Currently:
- This is done on both mobile and desktop.
- The goal is to give screen reader users a brief description of how an
  action changed the document.
- It's possible that this change will be more annoying than useful (more
  content read by the screen reader?)

Related to laurent22#10795.
@personalizedrefrigerator personalizedrefrigerator added mobile All mobile platforms desktop All desktop platforms accessibility Related to accessibility labels Nov 26, 2024
Comment on lines +120 to +131
let announcement = '';
if (charsAdded > 0) {
announcement = _('Added %s markup', accessibleName);
} else if (charsAdded < 0) {
announcement = _('Removed %s markup', accessibleName);
}

if (changedLineCount > 1) {
announcement += ` ${_('on %d lines', changedLineCount)}`;
} else {
announcement += ` ${_('to line start')}`;
}
Copy link
Owner

Choose a reason for hiding this comment

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

We cannot concatenate localised strings since the concatenation might not work in certain languages due to words being in a different order.

I'm not too sure what's the solution here, but maybe simplifying the message would help even if we lose a bit of information. It would also make the work of translators easier.

@personalizedrefrigerator
Copy link
Collaborator Author

I'm closing this due to concerns about whether these announcements are useful/necessary (and maintainability concerns). If such screen reader announcements would be useful, I can re-open this pull request.

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 mobile All mobile platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants