GNOME Bugzilla – Bug 667434
Calendar is not keyboard navigable
Last modified: 2013-05-29 22:40:37 UTC
STEPS TO REPRODUCE 1. Give the focus to the top panel (Ctrl+Alt+Tab) 2. Navigate to Calendar, and press down EXPECTED OUTCOME Being able to navigate on the calendar menu contents ACTUAL OUTCOME Calendar menu is being opened, but you can't navigate in any element of their content (like the calendar table itself, "Date and Time Settings" Menu Item, etc) NOTES As their content is not available at all, for the moment the accessible role for that top panel icon will be ATK_ROLE_LABEL (instead of the theorically correct ATK_ROLE_CALENDAR), as the label with the current date is the only thing available. See bug 667376 FWIW, GtkCalendar is also not keyboard navigable/accessible (bug 321123). Anyway, IMHO, after a quick look on the calendar.js code, it seems far more easy to solve. GtkCalendar is a mixture of Gtk and Gdk elements, while the calendar on GNOME Shell is all clutter-based. Probably the most problematic part is the table. Anyway, it would be good to at least provide keyboard navigation to some elements (like the Settings menu item), so this can be done step by step.
Created attachment 239583 [details] [review] calendar: Set can_focus for change month buttons
Created attachment 239584 [details] [review] theme: Set focus styles for calendar change month buttons
Created attachment 239585 [details] [review] dateMenu: Allow focus on 'Date & Time Settings' menu item
Created attachment 239586 [details] [review] dateMenu: Set can_focus for other menu items
Created attachment 239587 [details] [review] calendar: Add keynav to Calendar Items in the calendar table override the menu's typical behavior for left/right key presses and redirect them to navigate focus within the table.
Created attachment 239588 [details] [review] theme: Add focus style for calendar-day-base
Created attachment 239589 [details] [review] dateMenu: Don't override accessible_role inherited from PanelMenu.Button The menu is keyboard navigable now, so allow it to be advertised as such. Should this actually be advertised as ATK_ROLE_CALENDAR instead of PanelMenu's default ATK_ROLE_MENU?
Created attachment 239590 [details] [review] calendar: Add focus traps around edge of calendar The focus traps only care about the direction which would cause focus to leave the calendar table. These traps allow focus handling to pass back to the parent when the user hits an edge of the calendar.
Created attachment 239591 [details] [review] calendar: Add function for translating Clutter key direction to GTK direction
Review of attachment 239583 [details] [review]: OK.
Review of attachment 239584 [details] [review]: You can squash this with the previous patch; I don't mind.
Review of attachment 239585 [details] [review]: You should just be able to remove the item, as it's focusable by default.
Review of attachment 239586 [details] [review]: Again, same thing. Feel free to squash with previous patch as well.
Review of attachment 239587 [details] [review]: Have you considered using the standard StFocusManager stuff and doing global.focus_manager.add_group(calendarActor); ?
Review of attachment 239588 [details] [review]: OK. Feel free to squash in a giant theme patch at the end that adds focus styles for the calendar.
Review of attachment 239589 [details] [review]: OK.
Review of attachment 239590 [details] [review]: You shouldn't have to do this.
Review of attachment 239591 [details] [review]: This doesn't make sense to me; also, it doesn't handle tab/shift+tab. I'd split out the one in st-focus-manager.c and make it public if you really think you need this.
Created attachment 240562 [details] [review] calendar: Set can_focus on change month buttons
Created attachment 240563 [details] [review] calendar: Set can_focus on calendar days
Created attachment 240564 [details] [review] dateMenu: Allow focus on menu items
Created attachment 240565 [details] [review] dateMenu: Don't override accessible_role inherited from PanelMenu.Button The menu is keyboard navigable now, so allow it to be advertised as such.
Created attachment 240566 [details] [review] theme: Add focus styles for the calendar menu
Created attachment 240567 [details] [review] dateMenu: Override the default key handling for the date menu Let normal keynav take over for this more complex layout. The parent, PanelMenu.Button, grabs all left/right key presses, but we want to navigate horizontally for this menu, so don't let it do so.
Created attachment 240568 [details] [review] dateMenu: Add focus traps at edges of the menu to move focus to other menus When key focus passes to an edge of the menu, move to the next menu in that direction (like normal menu behavior). --- Basically apply PanelMenu.Button's _onMenuKeyPress behavior (which the previous patch blocks from applying all the time) when we hit the edge of the menu.
Review of attachment 240568 [details] [review]: Hm. I don't really like this and would prefer if we could implement this by selectively blocking the key press event (see PopupMenuSliderItem). If it's not easy to tell if we should block by checking the currently focused item, we should try to modify StFocusManager to see if we can make a "can_navigate" function that takes an event and tells if we can navigate within a focus group.
(In reply to comment #26) > Hm. I don't really like this and would prefer if we could implement this by > selectively blocking the key press event (see PopupMenuSliderItem). Well, we are selectively blocking the key press...for the whole menu (^_^). With two columns (the calendar and the event list) we need the left/right keys not only within the individual items (like what PopupMenuSliderItem does), but also between the items, i.e., for the whole menu. This is so we can move from the calendar or 'Date & Time Settings' item to the event column and back. > If it's not easy to tell if we should block by checking the currently > focused item, we should try to modify StFocusManager to see if we can make a > "can_navigate" function that takes an event and tells if we can navigate > within a focus group. I guess my point is we *always* need to block for items in this menu and actually even for the menu itself. So going through and blocking the key presses for every item just so it will move focus normally seems a bit extreme/more complicated to me.
Created attachment 243099 [details] [review] calendar: Set can_focus on change month buttons
Created attachment 243100 [details] [review] calendar: Set can_focus on calendar days
Created attachment 243101 [details] [review] dateMenu: Allow focus on menu items
Created attachment 243102 [details] [review] dateMenu: Don't override accessible_role inherited from PanelMenu.Button The menu is keyboard navigable now, so allow it to be advertised as such.
Created attachment 243103 [details] [review] theme: Add focus styles for the calendar menu
Created attachment 243104 [details] [review] calendar: Enable keynav in the calendar Any items on the left or right edge of the calendar (change month buttons or dates) should only capture the key presses that point inwards and return false for the ones pointing outward to allow the key presses to bubble up to the menu. Items in the middle have the freedom to capture both directions.
Created attachment 243105 [details] [review] calendar: Ensure clicked calendar item retains key focus The date actors get destroyed and recreated on every date change which drops key focus for the selected date. Restore key focus in such a case. But the selected date shouldn't always grab key focus, such as when the next/prev month buttons are clicked, in which case those buttons should retain key focus.
Review of attachment 243099 [details] [review]: OK.
Review of attachment 243100 [details] [review]: OK. Feel free to squash with previous.
Review of attachment 243101 [details] [review]: OK. Feel free to squash with previous.
Review of attachment 243102 [details] [review]: OK.
Review of attachment 243103 [details] [review]: OK.
Review of attachment 243104 [details] [review]: So, the thing is that I'm still not happy with this. The explanation is going to be a bit convoluted, so I'd rather talk to you on IRC about it when you have a chance.
Created attachment 243269 [details] [review] calendar, dateMenu: Allow focus on menu items
Created attachment 243270 [details] [review] dateMenu: Don't override accessible_role inherited from PanelMenu.Button The menu is keyboard navigable now, so allow it to be advertised as such.
Created attachment 243271 [details] [review] theme: Add focus styles for the calendar menu
Review of attachment 243269 [details] [review]: ++
Review of attachment 243271 [details] [review]: ++
Review of attachment 243270 [details] [review]: ++
Created attachment 243273 [details] [review] calendar: Ensure clicked calendar item retains key focus The date actors get destroyed and recreated on every date change which drops key focus for the selected date. Restore key focus in such a case, but only when the selected date was actually clicked. Whenever the next/prev month buttons code is used (for scrolling, mouse click, or keyboard click), have the corresponding button grab focus. Changing months currently causes the calendar to update twice as the eventSource gets changed, so key focus gets lost if it is on a date when the month changes. --- This implements reasonable behavior until we can update the calendar more granularly. Key focus will be lost from the menu completely if the user activates a date from a different month directly. If using the scroll wheel, or the next/prev buttons key focus will be retained across the months (on the next/prev buttons).
Created attachment 243274 [details] [review] focus-manager: Add navigate_from_event method This will be used to implement key navigation inside the calendar without having to listen to key events manually.
Created attachment 243275 [details] [review] panelMenu: Attempt to navigate menus only if we can't navigate inside the menu This enables key navigation in the calendar.
Review of attachment 243273 [details] [review]: I'm not overly happy with the solution (I'd prefer if we didn't rebuild the calendar when clicking on an item), but this is fine for now.
Review of attachment 243274 [details] [review]: ++
Review of attachment 243275 [details] [review]: ++
Attachment 243269 [details] pushed as c1240d3 - calendar, dateMenu: Allow focus on menu items Attachment 243270 [details] pushed as c29810b - dateMenu: Don't override accessible_role inherited from PanelMenu.Button Attachment 243271 [details] pushed as cc7630c - theme: Add focus styles for the calendar menu Attachment 243273 [details] pushed as 31478e9 - calendar: Ensure clicked calendar item retains key focus Attachment 243274 [details] pushed as 092586c - focus-manager: Add navigate_from_event method Attachment 243275 [details] pushed as d9a4688 - panelMenu: Attempt to navigate menus only if we can't navigate inside the menu Thanks for the patches, and sorry for the original troubles.
*** Bug 641252 has been marked as a duplicate of this bug. ***