Increase touch target size of admin menu lists

Tagging this as novice.

Tested, and works nicely, but because of the new markup nesting, the hover states looked ugly on the description text. This patch:

• Cleans up description text underlines on hover
• Adds a background color on hover and active states

I tested out the patch and it works for me in the latest Chrome, Opera, and IE 8.

Using proper mobile-novice tag

There's no reason we have to change the wrapper of the description from a div to a span. In HTML5, <a> tags can wrap block elements and still validate.

+ul.admin-list li a:hover .label,
+ul.admin-list.compact li a:hover .label {

We need an a:active selector there as well (as the earlier rulesets already include). :hover is only for desktop.

First try here...so let me know if I'm doing this wrong.

Attached is the new patch and the interdiff.

Oops, should have named that interdiff.txt.

I'm not crazy about the yellow styling on hover; it's not a part of Seven's current color palette. I tried white for hover styling it worked much better.

I looked at the CSS for this and realized that the CSS before the patch was more complicated then it needed to be. There were lots of unused selectors and redundant properties.

So I took the patch from #9 (Thanks, Joel!) and simplified the entire set of CSS. That's why there's so many minuses in the patch.

I like the simplicity and harmony of the white color background as well.

I tried out the patch in #12 and it is working fine. Thanks, John Albin!

@jtwalters Maybe mark it as RTBC? :-D

+++ b/core/themes/seven/style.cssundefined
@@ -377,50 +376,41 @@ ul.inline li {
+  padding: 2px 19px;

This is not left aligned, looks a bit strange to me. If this is increase the click area. I suggest add more top&bottom padding. and it also adding extra space to all admin pages.

Other admin pages have not hover effect. It broken theme style's consistency.

Can I get a screen? Please mark it for Usability whenever it changes anything visually.

Here are some screenshots with the patch from #12

@jtwalters,

what's your system ?? your fonts are bigger than mine a lot.

@droplet this was from a retina macbook pro so I guess the screenshots were dimension doubled. e.g. 200% zoom

#12 looks good to go.
Have tested in IE8/9, FF, Chrome, iOS Simulator.

I tested this quickly and it seems fine to me. I'm not 100% about underlining the links on hover as well as the white background, but no-one else brought it up so it might just be me, and it's very minor compared to the rest of the changes in the patch. Committed/pushed to 8.x.

Actually no, I wrote that comment before I committed/pushed the patch, and it no longer applies.

Rerolled patch.

#24 applied cleanly to latest 8.x and looks good to go

Wait, wtf, why are we adding this styling?

After discussion with @bojhan, removed background change on hover.

Just a quick chime in that it applied well here too and looks like a general usability win.

confirmed #28 applies. looks good to me. i don't really care either way about the background color, since the underline is enough of a hover indicator.

Patch looks great so far. It does not, however, apply as you would expect to the node/add page (the node add page has its own theme function theme_node_add_list) as the a tag in the node add list does not wrap the description.

This patch changes the markup from seven_node_add_list to match the output of seven_admin_block_content this patch creates.

Well noticed :-)

It does perhaps raise the question of whether we should have theme_node_add_list() and not use theme_admin_block_content()…

Patch changes vertical tabs on the node/add form to collapsible fieldsets. Was this cross-pollination from another patch?

Re-rolled to include changes to seven_node_add_list() from #32.

Patch changes vertical tabs on the node/add form to collapsible fieldsets. Was this cross-pollination from another patch?

Well spotted :)

Thanks for fixing that.

Looks good to me.

- Confirmed #34 applies.
- Touch targets have been enlarged as previous.
- Vertical tabs on node/add form remain vertical tabs.

+++ b/core/themes/seven/template.phpundefined
@@ -70,11 +72,14 @@ function seven_admin_block_content($variables) {
-      $output .= '<li class="leaf">';
-      $output .= l($item['title'], $item['href'], $item['localized_options']);
+      $output .= '<li>';
+      $content = '<span class="label">'.$item['title'].'</span>';

Missing spaces around $item['title'].

Also, if 'html' is TRUE, text is no longer sanitized. I think his patch may be introducing a security vulnerability.

Patch attached corrects missing spaces around $item['title']

$content, which is the first argument for l(), uses $item['title'] and $item['description']. These are now both passed through filter_xss_admin().

These lists will be deemphasized by the mobile toolbar but this is still worth doing.

#38: 1488962-38-admin-list.patch queued for re-testing.

In the testing we did so far, these listings are by no means deemphasized. People end up on them all the time, especially in configuration where we have no depth beyond the items.

+++ b/core/themes/seven/template.phpundefined
@@ -47,8 +47,10 @@ function seven_node_add_list($variables) {
+      $content = '<span class="label">' . $type->name . '</span>';

Hm. While we're at it, does this need to be check_plain()ed?

check_plain sounds like a good idea to me. Patch re-rolled.

#1798732 was reverted, so the test actually succeeded.

Looks sensible. Committed/pushed to 8.x.