launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/inline-picker-underscore-1005324 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1005324 in Launchpad itself: "Edit icon has trailing underscore when mouse hovers over it"
  https://bugs.launchpad.net/launchpad/+bug/1005324

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/inline-picker-underscore-1005324/+merge/107702

The picker activation icons were showing an underscore when hovered over. This was because a   had been added in various places to ensure the sprite icons show, since in some browsers the icons are not rendered if the anchor tag is empty.

== Implementation ==

I added css to suppress the underscore for any links the are marked as sprites.

I also fixed the issue in Opera where by the inline text editor activation icon had a funny Unicode char next to it. It turns out this was an attempt in the old lazr js library to get around the icon rendering issue for empty anchors. I replaced the Unicode char used with a   like what is used everywhere else.

== QA ==

I tested in Opera, Chrome, Firefox, Konquerer. 

Konquerer did not show icons for *anything* - not menu portal links, not person links, nothing. So the work in this branch fixes everything for all other browsers but Konquerer still has issues all over the place, not just in the areas covered by this branch. I'm not sure what its problem is.


-- 
https://code.launchpad.net/~wallyworld/launchpad/inline-picker-underscore-1005324/+merge/107702
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/inline-picker-underscore-1005324 into lp:launchpad.

=== modified file 'lib/canonical/launchpad/icing/css/typography.css'
--- lib/canonical/launchpad/icing/css/typography.css	2012-03-10 15:33:33 +0000
+++ lib/canonical/launchpad/icing/css/typography.css	2012-05-29 02:06:18 +0000
@@ -77,6 +77,9 @@
 a.help.icon, a.sprite.maybe.help {
     border: none;
 }
+a.sprite:hover {
+    text-decoration: none;
+    }
 a.invalid-link {
     disabled: True;
     color: #909090;

=== modified file 'lib/lp/app/javascript/inlineedit/assets/editor-core.css'
--- lib/lp/app/javascript/inlineedit/assets/editor-core.css	2011-12-21 07:54:40 +0000
+++ lib/lp/app/javascript/inlineedit/assets/editor-core.css	2012-05-29 02:06:18 +0000
@@ -22,24 +22,3 @@
 .yui3-ieditor-input textarea:focus {
     outline: none;
 }
-
-/* Konqueror doesn't render the multiline editor's button if there's
- * no apparent content in it (the sprite we use is a background image).
- * This bit of CSS is meaningless; it sets the edit link's content to
- * a zero-width non-joiner (an invisible, odorless Unicode character).
- *
- * Browsers should ignore the content attribute for :link and :visited
- * pseudo-classes, but Konqueror doesn't.  Setting non-empty text
- * content here tricks it into rendering the button.
- *
- * Other things we tried instead of this hack:
- *  - Insert an HTML comment in the <a>.  No effect.
- *  - Insert a &nbsp; in the <a>.  No effect.
- *  - Insert whitespace in the <a>.  No effect.
- *  - Insert a span or div in the <a>.  No effect.
- *  - Use a regular <img> tag instead of a sprite.  Ugly in all browsers.
- *  - Set the content to ".".  Ugly in Konqueror.
- */
-.yui3-editable_text-trigger:link, .yui3-editable_text-trigger:visited {
-    content: "\200c";
-}

=== modified file 'lib/lp/app/templates/text-line-editor.pt'
--- lib/lp/app/templates/text-line-editor.pt	2012-02-01 15:31:32 +0000
+++ lib/lp/app/templates/text-line-editor.pt	2012-05-29 02:06:18 +0000
@@ -3,7 +3,7 @@
   <a tal:condition="view/can_write"
      tal:attributes="href view/edit_url;
                      title view/edit_title"
-     class="yui3-editable_text-trigger sprite edit"></a>
+     class="yui3-editable_text-trigger sprite edit">&nbsp;</a>
 <tal:close-tag replace="structure view/close_tag"/>
 
 <script tal:condition="view/can_write"