Summary

Enable accessibility and item registration for the formatting toolbar.

Review Request #13212 — Created Aug. 14, 2023 and submitted Aug. 18, 2023, 5:16 p.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-6.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Items are now registered dynamically instead of hard-coded. This is both
to ensure consistent HTML (particularly with accessibility) and to later
allow extensions to add buttons to the toolbar (which was an original
goal of the project).

This does make one minor change to the toolbar contents, which is to mov
the Code button alongside Bold/Italic/Strikethrough. This is because
it's just setting an inline style like those (it's a code literal, not a
code block), and has more in common with those than linking/uploading
media.

Testing Done

Tested all of the toolbar functionality. It all works (except for image
uploads, which is a known problem already).

Commits

Summary	ID
Enable accessibility and item registration for the formatting toolbar. This updates the HTML for the formatting toolbar to ensure it's accessible, enabling screen readers to understand the groups and items on the toolbar in order to communicate it to the user, and to enable keyboard navigation. This involves setting ARIA attributes and using `<button>` instead of `<a>`. Items are now registered dynamically instead of hard-coded. This is both to ensure consistent HTML (particularly with accessibility) and to later allow extensions to add buttons to the toolbar (which was an original goal of the project). This does make one minor change to the toolbar contents, which is to mov the Code button alongside Bold/Italic/Strikethrough. This is because it's just setting an inline style like those (it's a code literal, not a code block), and has more in common with those than linking/uploading media.	38d04d603e76e16f1111f2f0e8948fc5a1272f01

Summary

Enable accessibility and item registration for the formatting toolbar.

This updates the HTML for the formatting toolbar to ensure it's accessible, enabling screen readers to understand the groups and items on the toolbar in order to communicate it to the user, and to enable keyboard navigation. This involves setting ARIA attributes and using `<button>` instead of `<a>`. Items are now registered dynamically instead of hard-coded. This is both to ensure consistent HTML (particularly with accessibility) and to later allow extensions to add buttons to the toolbar (which was an original goal of the project). This does make one minor change to the toolbar contents, which is to mov the Code button alongside Bold/Italic/Strikethrough. This is because it's just setting an inline style like those (it's a code literal, not a code block), and has more in common with those than linking/uploading media.

38d04d603e76e16f1111f2f0e8948fc5a1272f01

Issues

Description	From	Last Updated
Now that these aren't being referenced in events, can we rename the handlers to be private?	david	Aug. 15, 2023, 2:36 p.m.
For the image upload button, I'll need to listen to the file input's change event instead of the button's click …	maubin	Aug. 15, 2023, 3:05 p.m.
Typo "roup" -> "group"	maubin	Aug. 15, 2023, 2:36 p.m.

flake8 passed.

JSHint passed.

reviewboard/static/rb/js/ui/views/textEditorView.ts (Diff revision 1)

The issue has been resolved. Show all issues

For the image upload button, I'll need to listen to the file input's change event instead of the button's click event. We should give the toolbar items an EventsHash type option or something similar that would allow us to set other events and support setting events on specific elements in the $el, not just the button. This'll give more flexibility to extensions too.

chipx86 Aug. 15, 2023, 2:37 p.m.

I think once it's at the point of managing its own element, it should also set up its own event management. This can be pulled out into a standalone element that the event is registered on, and then passed in as $el.

I'll update this to prepare for your change.

reviewboard/static/rb/js/ui/views/textEditorView.ts (Diff revision 1)
The issue has been resolved. Show all issues
```
Typo "roup" -> "group"
```

reviewboard/static/rb/js/ui/views/textEditorView.ts (Diff revision 1)
The issue has been resolved. Show all issues
```
Now that these aren't being referenced in events, can we rename the handlers to be private?
```

Change Summary:


Changed the upload button to make it easier to attach event to the <input>, and removed the default click handler.
Converted all handlers to ES6 private methods.
Fixed a typo in an assertion message.

Commits:

	Summary	ID
	Enable accessibility and item registration for the formatting toolbar. This updates the HTML for the formatting toolbar to ensure it's accessible, enabling screen readers to understand the groups and items on the toolbar in order to communicate it to the user, and to enable keyboard navigation. This involves setting ARIA attributes and using `<button>` instead of `<a>`. Items are now registered dynamically instead of hard-coded. This is both to ensure consistent HTML (particularly with accessibility) and to later allow extensions to add buttons to the toolbar (which was an original goal of the project). This does make one minor change to the toolbar contents, which is to mov the Code button alongside Bold/Italic/Strikethrough. This is because it's just setting an inline style like those (it's a code literal, not a code block), and has more in common with those than linking/uploading media.	fb466905011e1070c101f571b6db189518b04c53
	Enable accessibility and item registration for the formatting toolbar. This updates the HTML for the formatting toolbar to ensure it's accessible, enabling screen readers to understand the groups and items on the toolbar in order to communicate it to the user, and to enable keyboard navigation. This involves setting ARIA attributes and using `<button>` instead of `<a>`. Items are now registered dynamically instead of hard-coded. This is both to ensure consistent HTML (particularly with accessibility) and to later allow extensions to add buttons to the toolbar (which was an original goal of the project). This does make one minor change to the toolbar contents, which is to mov the Code button alongside Bold/Italic/Strikethrough. This is because it's just setting an inline style like those (it's a code literal, not a code block), and has more in common with those than linking/uploading media.	38d04d603e76e16f1111f2f0e8948fc5a1272f01

Diff:

Revision 2 (+594 -96)

Show changes

	reviewboard/static/rb/css/ui/formatting-toolbar.less
	reviewboard/static/rb/js/ui/views/textEditorView.ts

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-6.x (36a6da5)