D7UX: Make list icons in admin interface clickable [#569516]

I was looking around in a fresh D7 install and on /admin/structure I clicked by reflex on the list item icon/arrow expecting it to activate the link. Nothing happened.

See attached file D7UX_2009-09-05_-_i1_-_admin-list_icon_is_not_clickable.png for reference.

This is the HTML for reference:

<ul class="admin-list">
  <li class="leaf"><a href="/admin/structure/block" title="Configure what block content appears in your site's sidebars and other regions.">Blocks</a><div class="description">Configure what block content appears in your site's sidebars and other regions.</div></li>
 [...] 
</ul>

The following CSS modifies the a element to also cover the icon (which is a background image of the li element). Since an a element could be present in div.description the CSS for an a element within div.description is reset.

ul.admin-list a {
  margin-left: -30px;
  padding: 0px 0 4px 30px;
}

ul.admin-list div.description a {
  margin-left: 0px;
  padding: 0px;
}

The code was inserted at the end of style.css in the Seven theme and then tested in the following browsers:

Safari 4.0.3
Firefox 3.5.2
Opera 10.0
Chromium 4.0.203.0

I have yet to test the snippet in Internet Explorer (IE). I tried with IE6 via Darwine but the login form did not work there.

This snippet also affects /admin/config.

See attached file D7UX_2009-09-05_-_i2_-_CSS_adjustment.png for reference.

I have not done an in depth look at the Seven theme so I do not know where the proper place for the snippet is thus no patch.

Regards,

Christian Larsen

Comment	File	Size	Author
#4	569516.patch	1.63 KB	p.brouwers
#1	569516.patch	2.17 KB	p.brouwers
	D7UX_2009-09-05_-_i2_-_CSS_adjustment.png	37.11 KB	Tiburón
	D7UX_2009-09-05_-_i1_-_admin-list_icon_is_not_clickable.png	39.88 KB	Tiburón

Comments

Comment #1

p.brouwers commented 10 September 2009 at 14:52

Assigned:	Unassigned	» p.brouwers
Status:	Active	» Needs review

Status	File	Size
new	569516.patch	2.17 KB

Made a patch for this. Modified the css a bit so it works for IE.
Also made a fix in template.php so it correctly adds the ie6.css (it did an @import which didn't work, and with drupal_get_css() modules can modify the css if needed)

Comment #2

10 September 2009 at 15:05

Status:

Needs review

» Needs work

The last submitted patch failed testing.

Comment #3

Tiburón commented 15 September 2009 at 00:15

I have not been able to test with IE6-8 yet so this quick patch review is based solely on looking at the changes in the patch attached to #1

+++ b/themes/seven/style.css	Thu Sep 10 16:48:17 2009 +0200
@@ -330,16 +330,27 @@
+#block-system-main ul.node-type-list li a, #block-system-main ul.admin-list li a {
+  margin-left: -30px;
+  padding: 0px 0 4px 30px;
+  min-height:0;
+  display: block;
+}
+
+#block-system-main ul.node-type-list li div.description a, #block-system-main ul.admin-list li div.description a {
+  margin-left: 0px;
+  padding: 0px;
+}

At the D7UX sprint the use of "display: block;" was suggested and tried. It made the whole line clickable and not just the icon and the link. This is inconsistent with how the rest of the interface works and thus was removed again.

Both the "min-height" and the "display" from the "[...] li a" rule should be reset in the "[...] div.description a" rule:

[...]
min-height: inherit;
display: inline;

If "display: block;" is needed for IE6 I think it should be moved to the "ie6.css" file. If not I think it should be removed based on what we found at the D7UX sprint.

+++ b/themes/seven/template.php	Thu Sep 10 16:48:17 2009 +0200
@@ -2,17 +2,24 @@
-  $vars['ie_styles'] = '<!--[if lt IE 7]><style type="text/css" media="screen">@import ' . path_to_theme() . '/ie6.css";</style><![endif]-->';
+  $vars['ie_styles'] = '<!--[if lt IE 7]>' .
+    drupal_get_css(array(path_to_theme() . '/ie6.css' => array(
+    'type' => 'file',
+    'media' => 'all',
+    'weight' => CSS_THEME,
+    'preprocess' => TRUE,
+    'data' => path_to_theme() . '/ie6.css'
+  ))) . '<![endif]-->';
 }

This fix should have been in its own issue. Though this issue probably will need this fix eventually this fix is not dependant on this issue.

The patch did not apply according to testbot. It looks like you have made the patch file using two local copies located in different folders. That might be the reason for the failure in this case as the root folder of the patch is one level above the Drupal root. Check http://drupal.org/patch/create on how to fix that.

Regards

Christian Larsen