Add support for keyboard navigation
stanislav.mueller - November 9, 2008 - 03:12
| Project: | Administration menu |
| Version: | 6.x-3.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Description
Hi guys,
here is an patch to give this awesome module more awesomeness: keyboard navigation. Just press escape key and go on.
It's clone of the default Leopard keyboard menu navigation, so i think it should be intuitive.
Would be nice to see mine in the next public release. :)
Tested in Safari, Firefox 3, IE7: works ;)
Thank you.
Stanislav
| Attachment | Size |
|---|---|
| admin_menu.css_.patch | 555 bytes |
| admin_menu.js_.patch | 4.21 KB |
#1
Nice idea.
A potential issue with the ESC-key is that some browsers could understand pressing it as a command to a reset form values of a focused form.
#2
First of all: Code clean-up. ;)
This indeed works pretty nice. However, the code needs much more inline documentation, because I have no idea why and how those variables are shuffled around.
Additionally, why do we have to use two private functions?
Please try hard to simplify and document this code. Based on the code (and no real benchmarks) I would say this is a performance-killer currently.
#3
Hi,
it looks like code is fully documented now and private functions are kicked.
You have mentioned that two recursive loops are performance killers. They look wild, but they don't. Benchmarks of the loops equal to benchmarks of current release initialization code. ;)
I've done some performance tests and rewrapped the initialization procedure,
this will be now invoked during first menu activation.
> A potential issue with the ESC-key is that some browsers could understand pressing
> it as a command to a reset form values of a focused form.
Idea for another activation key?
Please take a look :)
Thank you.
Stanislav
#4
Hi Stanislav,
I've implemented this patch into the patch at #226012: Merge Administration Menu Dropdown module with a few (very) minor changes.
I've been testing the keyboard navigation functionality and I have a few things I've noticed:
- The order of my menu is "[druplicon], Content management, Site building, Site configuration, User management, Reports, Help, [users], Log out" but the order of navigation is "[druplicon], Log out, Content management, Site building, Site configuration, User management, Reports"
- Items without sub-items (Help & [users]) aren't selectable.
- Pushing up at the top of a menu will loop to the bottom of the menu, but pushing down at the bottom of the menu won't loop to the top.
- There is no identifier of which item is selected other than the default browser outlining of an active link.
- Looking at the code I noticed there was actually a css class for selected items, but css code was not included in the patch.
- When adding the css class into the code manually I noticed that when traversing to a sub-menu, the parent items weren't maintaining the css class.
I think that's all I've noticed so far, will be sure to post more if I find it.
As for the issue with the ESC key, I would suggest making the key/combo definable as is the Key Combo behavior of Admin Menu Dropdown.
If my patch is approved or green lighted for further development it shouldn't be to difficult to do.
Cheers,
Deciphered.
#5
Hi Deciphered,
I've fixed issues you noticed.
We have got new key/combo this is: SHIFT+SPACE (The same combo as browser's scroll to top)
There are some very minor changes in the admin_menu.inc, needed to get right keyboard order during the keyboard navigation.
This patch goes on top of the current admin_menu dev release: 6.x-1.x-dev
Best,
Stanislav
#6