GNOME Bugzilla – Bug 706903
Calendar has some key-navigation and accessibility issues
Last modified: 2015-03-03 18:54:30 UTC
Created attachment 253269 [details] Program that prints the name/role each time a new gnome-shell object gets focused/selected I have been reviewing these days the calendar, after the work done on bug 667434 (thanks!). Most of the work got solved, but there are some nits: 1. Left/right arrows doesn't have an accessible name 2. Full date and current month name are not keyboard navigable FWIW, making those navigable follows the same rationale of non-reactive items being navigable. See bug 667439 comment 1 for more information. 3. Abbreviated day name are not keyboard navigable 4. The right part of the container (events list) is not keyboard navigable IMHO, I think that a good way to navigate to that container would be using TAB. So you use key arrows to navigate between siblings, and use TAB to switch between the calendar and the events list. Anyway, that is arguable. NOTE: As you can see, some of the current issues are still key-navigation related, but I think that it doesn't make sense to reopen bug 667434. That one fixed the core of calendar navigation, so we are talking here just about some refining, and also because I prefer to not open a different bug just for the name. NOTE2: in order to test 1. I'm attaching a little program written in C. It just print the name and role of current object focused/selected.
Created attachment 253270 [details] [review] Add accessible name for next/prev month buttons
Created attachment 253271 [details] [review] Month name should be navigable
Created attachment 253272 [details] [review] Day abbreviation should be navigable Note: not fully sure about this one.
Created attachment 253273 [details] [review] Full date string should be navigable
Some extra notes: These patches adds some UI changes: * Some of the elements are navigable, so also highlighted * Patch at comment 1 adds new strings. We could workaround "Next month" as the po files already include the string "Next". But there isn't a "Prev" And we are already on UI/String freeze. In the same way, although all those patches were trivial, it seems that is not the case for the events list. Just setting can_focus to true on their labels doesn't work, and in fact makes the calendar not key navigable at all. For all this, I'm not proposing this as a change for 3.10 yet. But I thinks that it was better to upload the patches before I forget. Also CCing Tanner Doshier, as was the one that provided most of the patches for bug 667434.
(In reply to comment #5) > These patches adds some UI changes: > * Some of the elements are navigable, so also highlighted This sounds minor enough to not be subject to the freeze - it's not anything that'll show up in any screenshots or documentation. Maybe just get some r-t approval on IRC when in doubt? > * Patch at comment 1 adds new strings. We could workaround "Next month" as > the po files already include the string "Next". But there isn't a "Prev" Yeah, that'd require a freeze break exception ...
Review of attachment 253270 [details] [review]: ::: js/ui/calendar.js @@ +446,2 @@ this._backButton = new St.Button({ style_class: 'calendar-change-month-back', + accessible_name: _('Previous month'), translatable names should be in double quotes so xgettext finds them
Review of attachment 253271 [details] [review]: ::: data/theme/gnome-shell.css @@ +1212,3 @@ } +.calendar-month-label:hover, not sure why you have hover here, since track_hover isn't set.
Review of attachment 253272 [details] [review]: ::: js/ui/calendar.js @@ +475,3 @@ + text: customDayAbbrev, + can_focus: true, + accessible_name: customDayAbbrev}); shouldn't the label automatically have an accessible name due to its text? or do we need to set label_actor somewhere.
Review of attachment 253273 [details] [review]: OK.
Created attachment 253390 [details] [review] Add accessible name for next/prev month buttons Using " instead of '
Created attachment 253391 [details] [review] Month name should be navigable .calendar-month-label:hover removed from css file
Created attachment 253393 [details] [review] Day abbreviation should be navigable (In reply to comment #9) > Review of attachment 253272 [details] [review]: > > ::: js/ui/calendar.js > @@ +475,3 @@ > + text: customDayAbbrev, > + can_focus: true, > + accessible_name: customDayAbbrev}); > > shouldn't the label automatically have an accessible name due to its text? Yes > or do we need to set label_actor somewhere. No it is not needed. Setting accessible_name or label_actor is not needed. This patch removes accessible_name assignment.
(In reply to comment #5) > Some extra notes: > > These patches adds some UI changes: > * Some of the elements are navigable, so also highlighted > * Patch at comment 1 adds new strings. We could workaround "Next month" as > the po files already include the string "Next". But there isn't a "Prev" > > And we are already on UI/String freeze. BTW, just realized that this sentence is wrong. We are on UI Freeze, but String freeze starts with 3.9.91. So not freeze request is needed, as those strings are not part of any UI at all.
Review of attachment 253390 [details] [review]: OK.
Review of attachment 253391 [details] [review]: OK.
Review of attachment 253393 [details] [review]: OK.
(In reply to comment #17) > Review of attachment 253393 [details] [review]: > > OK. Just changed the state of the patch to needs-work. Doing a last test just before landing the patch I realize an odd bug. Setting can_focus to true to the abbreviated names make them navigable, but if are focused on one, and navigate to the left, it moves in diagonal (to the left down), so moving to the new row.
Does this still have any remaining work on it?
(In reply to comment #19) > Does this still have any remaining work on it? Yes: (In reply to comment #0) > 3. Abbreviated day name are not keyboard navigable This is still valid (see comment 18) > 4. The right part of the container (events list) is not keyboard navigable > IMHO, I think that a good way to navigate to that container would be using > TAB. So you use key arrows to navigate between siblings, and use TAB to switch > between the calendar and the events list. Anyway, that is arguable. This is still valid. After talking with Joanmarie, a different option would be use TAB to switch between the main containers of the calendar. Sorry I didn't have time to look to this. Anyway, probably you are asking that because some of the commits were committed. What I could do is close this bug, and open individual bugs for the missing stuff.
Moving off the blocker list. We can easily improve keynav after 3.10.0
Created attachment 298394 [details] [review] dateMenu: Include "Today" button in keynav
Created attachment 298395 [details] [review] calendar: Allow keynav to day headers and week numbers While those elements cannot be activated, they still provide useful information to screen readers, so include them in the focus chain. For the same purpose, set a more verbose accessible name, given that it is not bound by the same space constraints as the visible label.
Comment on attachment 298394 [details] [review] dateMenu: Include "Today" button in keynav Attachment 298394 [details] pushed as be52ad9 - dateMenu: Include "Today" button in keynav
(In reply to Alejandro Piñeiro Iglesias (IRC: infapi00) from comment #18) > Just changed the state of the patch to needs-work. Doing a last test just > before landing the patch I realize an odd bug. Setting can_focus to true to > the abbreviated names make them navigable, but if are focused on one, and > navigate to the left, it moves in diagonal (to the left down), so moving to > the new row. I can no longer reproduce this, so I'd suggest we go ahead now and do that change. While we don't have any special handling for the message list, it does now work with normal keynav, so the last missing bit in the calendar drop-down is the world clock, which is handled in bug 745393 ...
Attachment 298395 [details] pushed as afc2253 - calendar: Allow keynav to day headers and week numbers Pushing after quick review on #a11y