TodoMVC: first step to improve accessibility #5580
Conversation
| @@ -34,6 +34,7 @@ export component CreateTaskView { | |||
| HorizontalLayout { | |||
| IconButton { | |||
| icon: Icons.close; | |||
| accessible-label: @tr("close"); | |||
There was a problem hiding this comment.
Perhaps this should be a little more description in saying what is closed? (like a button to play music should say "play music")
@DataTriny What do you think?
(the same comment applies to most of the other labels as well, of course)
There was a problem hiding this comment.
Accessible names should be capitalized just like any user-facing text. Of course people using speech systhesis won't notice, but others might use a Braille display for instance.
I would probably add a description saying that this button goes back to the task list.
| @@ -46,6 +46,7 @@ export component TaskListView { | |||
| SelectionListView { | |||
| width: 100%; | |||
| model: TaskListAdapter.tasks; | |||
| accessible-label: @tr("task listview"); | |||
There was a problem hiding this comment.
@DataTriny Could you help us out here: Should accessible labels be capitalised? (my feeling is yes, but it would be good to confirm :)
There was a problem hiding this comment.
To be fair, capitalization is the least of my concerns here. Why do you talk about "listview" to your users? "List of tasks" would be far more appropriate.
There was a problem hiding this comment.
Other things worth trying:
- Put the "Create" button just after the time combobox in the tab index order,
- Place the focus on the task name edit after the "Add" button is clicked,
- Put the focus on the task list when the program starts and when coming back from the create task screen.
- Both controls to pick the due date and time seem to not be labelled.
| @@ -34,6 +34,7 @@ export component CreateTaskView { | |||
| HorizontalLayout { | |||
| IconButton { | |||
| icon: Icons.close; | |||
| accessible-label: @tr("close"); | |||
There was a problem hiding this comment.
Accessible names should be capitalized just like any user-facing text. Of course people using speech systhesis won't notice, but others might use a Braille display for instance.
I would probably add a description saying that this button goes back to the task list.
| @@ -46,6 +46,7 @@ export component TaskListView { | |||
| SelectionListView { | |||
| width: 100%; | |||
| model: TaskListAdapter.tasks; | |||
| accessible-label: @tr("task listview"); | |||
There was a problem hiding this comment.
To be fair, capitalization is the least of my concerns here. Why do you talk about "listview" to your users? "List of tasks" would be far more appropriate.
| @@ -61,6 +62,7 @@ export component TaskListView { | |||
|
|
|||
| ActionButton { | |||
| icon: Icons.add; | |||
| accessible-label: @tr("add"); | |||
There was a problem hiding this comment.
A bit more context could be useful. "Create new task" or something along those lines would be clearer.
| @@ -48,6 +48,7 @@ export component SelectionListViewItemDelegate { | |||
| check-box := CheckBox { | |||
| horizontal-stretch: 0; | |||
| y: (parent.height - self.height) / 2; | |||
| accessible-label: @tr("selected"); | |||
There was a problem hiding this comment.
This checkbox indicates whether a task is done right? "selected" is misleading.
The proper solution here would be to build the checkbox name from the task name and the due date: ", due by "
There was a problem hiding this comment.
This is a little tricky. I think for now I'll change the label to indicate whether the task is done. Regarding a proper solution, I'm curious: Would you build the checkbox to have an accessible label that says "Task with description XYZ, due by ABC" and then set the accessibility role of the separate description Text label to none?
|
|
||
| IconButton { | ||
| y: (parent.height - self.height) / 2; | ||
| icon: Icons.remove; | ||
| accessible-label: @tr("remove"); |
There was a problem hiding this comment.
It would be far more practical to support the delete key as well. In a proper TODO list app you would use the up and down arrow keys to select listview elements, pressing tab could put the focus to the currently selected item's remove button, but preferably not the other ones.
There was a problem hiding this comment.
True, full keyboard navigation support would be nice to have. For now I've changed this to "Delete Task".
| @@ -91,6 +92,8 @@ export component SelectionListView inherits ListView { | |||
| callback toggle(/* index */ int); | |||
| callback remove(/* index */ int); | |||
|
|
|||
| accessible-role: list; | |||
There was a problem hiding this comment.
Just a reminder that the accessible role is not the right one. At least for the winit/AccessKit backend you are using the role for semantic lists, as you would use in a paragraph of text. The role to use is Role::ListBox and each element in the list should have Role::ListBoxOption.
There was a problem hiding this comment.
Ok, we don't have support for ListBox yet :(. Maybe not for this PR.
From look at the ARIA description for list vs. listbox, it sounds like to me that ListView should default to listbox as a role, as more often than not it's something the user interacts with and selects from, as opposed to a static list just for display (like a bullet list). Would you agree?
|
@DataTriny thank you. Your feedback helps a lot. |
|
Due to the merge conflict I'll take the liberty of rebasing this. |
Use more elaborate label text.
tronical
force-pushed
the
florian/todomvc-accessibility
branch
from
03e4be0
to
03f1e58
Compare
No description provided.