In #521852: Local tasks lack semantic markup to indicate an active task we discussed adding an invisible heading, or some other textual indicator, before the list of local tasks (tabs), to provide purpose and context for the list of links. Providing this context will assist screen-reader users understand that the list of links is functionally a tabstrip, something that is communicated through colour / style alone. I'm not sure why we didn't follow up that issue at the time, but this is a simple fix.
The trick will be deciding upon proper wording for the text. "Tabstrip" seems to short to be meaningful "Tabs for this page" is a bit erroneous, as clicking on a tab switches the context to a new "page". "Tabs for [node title[" seems to repetitive, as this would normally come directly after the node-title.
The changes can likely be made in ">theme_menu_local_tasks().
| Comment | File | Size | Author |
|---|---|---|---|
| #24 | 883092-local-tasks-headings-5.patch | 4.43 KB | jacine |
| #20 | 883092-local-tasks-headings-4.patch | 4.48 KB | Everett Zufelt |
| #19 | 883092-local-tasks-headings-3.patch | 4.4 KB | Everett Zufelt |
| #17 | 883092-local-tasks-headings-2.patch | 3.5 KB | Everett Zufelt |
| #14 | 867114-14.patch | 1.26 KB | mgifford |
Comments
Comment #1
Everett Zufelt commentedNote that this is particularly necessary when:
1. Users are unfamiliar with Drupal and have no idea that a tabstrip exists anywhere on the page.
2. When distinguishing between the Primary and Secondary set of local tasks
3. When the local tasks are themed away from being directly after the node-title
I considered marking this issue critical, since the purpose of these links cannot always be determined from their context, especially when dealing with secondary local tasks , but we'll leave it at major for now to avoid any potential debates that critical priority may cause.
Comment #2
Everett Zufelt commentedHere is a first pass at a patch, it needs work.
This patch adds a heading before each of the unordered lists of Primary and Secondary tasks.
Heading level 2 hidden with the element-invisible class
Text is "Primary Tasks" and "Secondary Tasks" (please suggest something better).
Currently not working in Seven, so there must be a theme override that I need to find, I tested on /user/1
Comment #3
Everett Zufelt commentedI took a look at Seven, it doesn't override theme_menu_local_tasks(). I tested the page with Javascript disabled and the headings are still not present, so it isn't anything related to that either.
Comment #4
Jeff Burnz commentedSeven builds its own variables for primary and secondary tasks, bypassing the theme function. This won't work in Garland either. Its a bit weak to place this in the theme function since many themes split the tabs variable similar to how Seven and Garland are doing it, unfortunately I don't have a better solution in my brain right now.
Comment #5
Everett Zufelt commented@Jeff
Thanks for the info. Can you please point me to the functions that Garland and Seven use to produce their local tasks?
Comment #6
Jeff Burnz commentedSure, both do it in template.php.
Seven does this in seven_preprocess_page():
Garland on the other hand overrides the theme function:
For both it might just be easier to put the headers in page.tpl.php?
Comment #7
mgiffordI think it's going to have to be in the template.php because of the translation of the hidden header, t('Primary Tasks') & t('Secondary Tasks') can't be run from the tpl.php files, right.
@Everett, are you going to re-roll it?
Comment #8
Jeff Burnz commentedt() can be run from anywhere and I'm pretty sure you can extract translatable strings from tpl files - best to ask someone in the locale team maybe (Gabor?).
Comment #9
mgiffordOk, I think the problem is really that this hasn't been added for Seven. I think that headings are there in the other themes.
Patch for Seven is included. And Jeff, you were definitely right about t(). Not sure where I got that idea from that it wouldn't work there.
Comment #11
mgifford#9: 867114-9.patch queued for re-testing.
Comment #12
Everett Zufelt commentedThe most recent patch seems to contain information about the search module. I don't think it was a proper roll.
Comment #13
Jeff Burnz commentedYeah, its totally the wrong patch, lol, will take a look since Mikes gone on holiday.
Comment #14
mgiffordOk.. Maybe I've uploaded the right one this time..
Comment #15
Everett Zufelt commented@mgifford
The patch in #14 looks good. I'll merge it with the modification to the theme function in the patch in #2 and make sure that we don't need to correct this in other core themes.
Comment #16
Jeff Burnz commentedJust referencing an issue with Garlands local tasks: #903814: Some admin pages not displaying in the Overlay in Garland
Comment #17
Everett Zufelt commentedSetting to Needs Review, but it still needs a bit of work. This patch adds an h2 class="element-invisible" for primary and secondary local tasks for all core themes*.
1. Standardized on 'Primary tabs' and 'Secondary tabs'.
2. Default implementation in theme_menu_local_tasks()
3. Overridden in page.tpl.php for Garland and Seven.
* Not appearing in Overlay. I know that Overlay uses jQuery to move the local tasks within the DOM, but don't know how. Regardless, adding an unique id and getting an Overlay person in on this would be a really quick fix.
Comment #18
mgiffordThis looks pretty good to me. It applies nicely. I've added a tag for Overlay so hopefully someone there looks at this & can help.
Comment #19
Everett Zufelt commentedThis patch adds to the prior patch:
1. Adds a heading for Primary tabs in overlay.tpl.php
* note: I think that this is as much as we can do, it appears that Overlay only displays one level of local tasks.
overlay.module : template_preprocess_overlay(&$variables)
...
$variables['tabs'] = menu_primary_local_tasks();
So, if there are no bugs I think this is good to go.
So,
Comment #20
Everett Zufelt commentedCorrected spelling
Comment #22
Everett Zufelt commented#20: 883092-local-tasks-headings-4.patch queued for re-testing.
Comment #24
jacineI just tested this with Bartik, Stark, Garland and Seven as admin themes, and it's all good.
I'm not sure why the testbot doesn't like Everett's patch. I've attached a straight re-roll of it to see if the bot likes it any better.
Comment #25
mgiffordLooks good to me. Jacine, thanks for the re-roll. The overlay patch that Everett introduced works fine too.
Comment #26
dries commentedCommitted to CVS HEAD. Thanks.