Navigation block does not have keyboard support

a few things have been done to improve accessibility regarding navigation.

additional to test:

• keyboard navigation of dock
• hide/show docked block panel
• undock blocks

note: most spacebar was used as the action on the menus. Arrows weren't used as they seem useful in scrolling the page (left/right). Maybe this sort of choice would make for a nice accessibility configuration or perhaps theres a standard?

Hi Apu,

I've just been looking at this now and made the following notes:

In regards to blocks/dock.js I think the code here could be further improved although you are certainly heading in the correct direction.

1. Minor point calling e.stopPropogation then e.preventDefault is the same as calling e.halt. But read on before you go changing them
2. I've tried to keep all event callbacks as properly defined functions as such I think that the stopPropogration/preventDefault/halt calls should be moved into the callback functions and then wrapped to check the event type within the callback function so that they get called is required (if that check is in fact required).
3. The reason that the dock isn't removed when you remove the last item is because of the stopPropogration call you've added. The dock is an event driven component which defines and makes use of its own events, when an item is removed it fires and event to signal the removal of an item, there is another part of the dock listening to this event that then checks if there are any docks left and if there arn't removes the dock. Calling stopPropogation of course prevents this event from being fired.
4. Check out http://developer.yahoo.com/yui/3/event/#keylistener which shows how to attach a key event with the desired key(s) in the definition. Doing this will allow you to remove the anonymous callbacks functions altogether.
5. I also think it would be wise to watch for the enter key for the three cases you have there.

blocks/navigation/yui/navigation/navigation.js

1. Same deal as above really, not sure if you can properly delegate a key event with the included keycode however, YUI docs are pretty sketchy in that area you may need to either read the YUI code or just test it, it is doesn't look possible I'd put my +1 towards creating a dedicated function to mediate key events.
2. I would also consider listening for enter to toggle as well as right arrow for expand, and left arrow for collapse... possibly a rtl usability issue there but would be more useful for ltr language users.

blocks/navigation/renderer.php

1. No need to create $spanattr - the attributes array is reset on interation just add the tabindex to that at the right time.

One final note - make sure you test this on a number of browsers - very particually IE7 and Safari which tend to be ... unique

Cheers
Sam

Sounds like great progress on this, well done.

I believe the standard action key is Enter, not Space - same as triggering links. It's probably okay if both work.

Hey yall ,
i've updated the repo in the issue with changes, have a look and see how it goes. i've tested on chrom, ie8, safari for windows 5.0.5.

SamM, i've used Enter as an activation sort of meaning, and Space as a toggle-ish meaning but in any case they should be easily modifiable from this point. Meanings can be modified in the event definition, the rest of the code operates on the mapped meanings.
The difference at this point is: Enter follows the link in a menu branch while Space (and respective arrows) would operate the branch.

Aha - thanks.

I agree that may be the best easy option in this case - it is not ideal as this probably needs to be documented so that keyboard users understand it (esp. screenreader users) but obviously if you have only one link and it can do two things, there's not much option.

Imo it would be slightly preferable to have different links but I can see this could be difficult to arrange...

SamM,
i agree, making another separate focus to separate branches and their link content does make accessibility easier. (might make clicking more susceptible to clicking within non-eventful gaps but i'm sure that can be handled somehow.)
Perhaps another issue to think about it?

This patch now works for all keyboard naviation. one annoyance left is that the XHR branch loading doesn't trigger. working on this..

new to upcoming 2.1 ! improved keyboard accessibility to navigation menus! :-D

SamH, this should be ready for integration.

Hi Apu,

I've been reviwing this and thinking it over for a while now, the following things I think need consideration:

blocks/dock.js

1. 640: The remove_all event shouldn't be triggered by the right arrow, in fact I would lean towards only triggering it with the event key.
2. You note the actionkey is required to help with Y.Delegate however you don't delegate any events with it. Need to clarify the docs

blocks/navigation/yui/navigation/navigation.js

1. 261, 263: Define branchp without tabindex and then is !link setAttribute('tabIndex', '0')
2. 345, 350: e.type = 'actionkey' ... you are missing an equals.
3. 353: Should return true for consistency.

One more thing - I don't like the if statements where you are checking the event isn't of a certain type.
I think all of these statements need to be clarified. I think the best way through is to check that the event is of the desired type only. This way if in the future someone adds more keys to this they don't need to review all of the previous uses of the actionkey event.
I can see two ways of doing this:

1. Write it into the if statements
2. Require a forth argument on actionkey attach events where you specify the desired keys/actions and then cross reference it within the keyHandler

I think this really needs to be cleaned up and clarified.

The rest of the patch looks fine.

Cheers
Sam

Ahh one more thing, in regards to the point Sam was raising about having multiple operations on one link I also agree.
I think this should be tied in with changes to add informative titles to the navigation structures so that the likes of screenreaders can be aware of the options.
Presently with one link this would have been a hard task, however if we split the branch from the link it should be much easier.

Cheers
Sam

Thanks SamH,
I think
'Require a forth argument on actionkey attach events where you specify the desired keys/actions and then cross reference it within the keyHandler'
is an awesome suggestion, working on that now.

I think i've commented somewhere in patch the area to split up the branch from the link, this is by using a tabindex for the branch and a separate one for the link too. I can add in the title field to a branch, any suggestions on what it should contain ? ( 'branch'? node text ? )

I've a nagging doubt about screenreaders and how they would operate on that though.

Hm, are there any recommended screenreaders to test with?
Cheers,
Apu

1) For info, at the OU we test with JAWS screenreader and Internet Explorer, which is basically the industry standard, but (a) it's very expensive - I think there is a limited demo but last time I installed that it did something horrible to my PC, and (b) I leave that to somebody else, see above.

2) My understanding is that screenreaders generally do not read the 'title' tag by default. If you want screenreaders to read some extra information for a link that isn't visible on screen, you can either add the text using <span class="accesshide"> - or if there's an icon in the link or something like that, you can put the text in the 'alt' tag.

3) I am happy to request that our expert accessibility tester retests the standard Moodle navigation block using a screen reader once you think it's working (i,e. if there's anything we are unsure about how it ought to be done), but this is likely to take several weeks at least until she has time - it might be best if it doesn't hold up integration. Once the code is ready, should I ask her?

Failing it for this week so that the last few things can be cleaned up.

updated my repo details to https://github.com/nebgor/moodle/compare/mistress...MDL-27428_2

• added the action-ables as an argument
• clarified bit more.

Mr Hemelryk, could you review that?
Mr Marshall, when the time is right, please do!

Thanks Apu this has been rebased now.
I did interactively rebase your work before integration in order to tidy up the commit messages, your last commit message started with a / and all of your commit messages differed slightly.

Could you please improve the testing instructions as well, the tests need to be more specific (keys, places to test) and I think testers will likely need to clear caches in order to see the changes.

Cheers
Sam

Thanks Sam, the test has been updated to test the patch.

Looks good although there was a minor annoyance that Ill open a separate low priority MDL for.

update: MDL-27604 (using the calendar with the keyboard kind of sucks)

And this is now part of Moodle upstream, many thanks!

Q: This is exclusively for 2.1? No interesting to backport to 20_STABLE?