launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/sprite-icon-spacing-1007263 into lp:launchpad.

Requested reviews:
  Curtis Hovey (sinzui)
Related bugs:
  Bug #1007263 in Launchpad itself: "No space between text and associated action sprites"
  https://bugs.launchpad.net/launchpad/+bug/1007263

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/sprite-icon-spacing-1007263/+merge/108291

== Implementation ==

Instead of adding back into the markup some   to ensure the sprite icons did not abut the text, I thought it better to add a css style for a.sprite to introduce a bit of left margin padding. The main place where the effect is seen is on the bugtask index page.

+a.sprite {
+ margin-left: 6px;
+ }

And then I went through and removed all the "... &nbsp;<a ...></a>" markup.

I think this approach is much nicer ie using css rather than nbsp everywhere.

Everything seems to render ok as far as I can tell. I would love a second opinion though.
For some reason Chrome appears to render the icons with a very slightly larger gap between the icon and text. I'm not sure why.

-- 
https://code.launchpad.net/~wallyworld/launchpad/sprite-icon-spacing-1007263/+merge/108291
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

=== modified file 'lib/canonical/launchpad/icing/css/modifiers.css'
--- lib/canonical/launchpad/icing/css/modifiers.css	2012-03-10 01:35:20 +0000
+++ lib/canonical/launchpad/icing/css/modifiers.css	2012-06-01 06:57:21 +0000
@@ -200,6 +200,10 @@
     line-height: 18px;
     }
 
+a.sprite {
+    margin-left: 6px;
+    }
+
 .sprite-after {
     padding: 2px 18px 0 0;
     }

=== modified file 'lib/lp/app/doc/lazr-js-widgets.txt'
--- lib/lp/app/doc/lazr-js-widgets.txt	2012-05-30 02:33:22 +0000
+++ lib/lp/app/doc/lazr-js-widgets.txt	2012-06-01 06:57:21 +0000
@@ -165,7 +165,6 @@
       <div id="edit-description" class="lazr-multiline-edit">
         <div class="clearfix">
           <div class="edit-controls">
-            &nbsp;
             <a class="yui3-editable_text-trigger sprite edit"
                href="http://launchpad.dev/~eric/+archive/ppa/+edit";
                title=""></a>
@@ -332,7 +331,6 @@
     <span id="edit-hide_email_addresses">
     My email: <span class="value">Don't hide it</span>
       <span>
-        &nbsp;
         <a class="editicon sprite edit"
            href="http://launchpad.dev/~eric/+edit";
            title=""></a>
@@ -391,7 +389,7 @@
     >>> print widget()
     <span id="edit-lifecycle_status">
         <span class="value branchstatusDEVELOPMENT">Development</span>
-        <span>&nbsp;
+        <span>
           <a class="editicon sprite edit"
              href="http://.../+edit";
              title=""></a>

=== modified file 'lib/lp/app/javascript/choice.js'
--- lib/lp/app/javascript/choice.js	2012-05-30 21:54:13 +0000
+++ lib/lp/app/javascript/choice.js	2012-06-01 06:57:21 +0000
@@ -62,7 +62,7 @@
 
     var choice_node = Y.Node.create([
         '<span id="' + field_name + '-content"><span class="value"></span>',
-        '&nbsp;<a class="sprite edit editicon" href="#"></a></span>'
+        '<a class="sprite edit editicon" href="#"></a></span>'
         ].join(' '));
 
     legacy_field_node.insertBefore(choice_node, legacy_field_node);

=== modified file 'lib/lp/app/javascript/formwidgets/formwidgets.js'
--- lib/lp/app/javascript/formwidgets/formwidgets.js	2012-05-31 02:20:41 +0000
+++ lib/lp/app/javascript/formwidgets/formwidgets.js	2012-06-01 06:57:21 +0000
@@ -117,7 +117,7 @@
     initializer: function(config) {
         this.labelNode = Y.Node.create("<label />");
         this.helpNode = Y.Node.create(('<span class="helper unseen">'+
-            '&nbsp;<a href=""' +
+            '<a href=""' +
             'target="help" class="sprite maybe">' +
             '<span class="invisible-link"></span></a></span>'));
         this.fieldNode = Y.Node.create("<div></div>");

=== modified file 'lib/lp/app/templates/boolean-choice-widget.pt'
--- lib/lp/app/templates/boolean-choice-widget.pt	2012-02-01 15:31:32 +0000
+++ lib/lp/app/templates/boolean-choice-widget.pt	2012-06-01 06:57:21 +0000
@@ -1,7 +1,6 @@
 <tal:open-tag replace="structure view/open_tag"/>
 <tal:prefix replace="view/prefix"/><span class="value" tal:content="view/value">Unset</span>
     <span tal:condition="view/can_write">
-      &nbsp;
       <a tal:attributes="href view/edit_url;
                          title view/edit_title"
          class="editicon sprite edit"></a>

=== modified file 'lib/lp/app/templates/enum-choice-widget.pt'
--- lib/lp/app/templates/enum-choice-widget.pt	2012-02-01 15:31:32 +0000
+++ lib/lp/app/templates/enum-choice-widget.pt	2012-06-01 06:57:21 +0000
@@ -1,7 +1,7 @@
 <span tal:attributes="id view/content_box_id">
   <span tal:attributes="class view/css_class"
         tal:content="structure view/value" />
-  <span tal:condition="view/can_write">&nbsp;
+  <span tal:condition="view/can_write">
     <a tal:attributes="href view/edit_url;
                        title view/edit_title"
        class="editicon sprite edit"></a>

=== modified file 'lib/lp/app/templates/text-area-editor.pt'
--- lib/lp/app/templates/text-area-editor.pt	2012-05-12 03:19:06 +0000
+++ lib/lp/app/templates/text-area-editor.pt	2012-06-01 06:57:21 +0000
@@ -4,7 +4,6 @@
                        class view/tag_class">
   <div class="clearfix">
     <div class="edit-controls" tal:condition="view/can_write">
-      &nbsp;
       <a tal:attributes="href view/edit_url;
                          title view/edit_title"
          class="yui3-editable_text-trigger sprite edit"></a>

=== modified file 'lib/lp/bugs/templates/bug-portlet-privacy.pt'
--- lib/lp/bugs/templates/bug-portlet-privacy.pt	2012-05-17 09:45:29 +0000
+++ lib/lp/bugs/templates/bug-portlet-privacy.pt	2012-06-01 06:57:21 +0000
@@ -10,7 +10,7 @@
 >
   <div id="privacy-text">
     <tal:information_type tal:condition="view/show_information_type_in_ui">
-      This report contains <strong id="information-type" tal:content="view/information_type"></strong> information&nbsp;
+      This report contains <strong id="information-type" tal:content="view/information_type"></strong> information
         <a class="sprite edit" id="privacy-link" tal:attributes="href link/path" tal:condition="link/enabled"></a>
         <div id='information-type-description' style='padding-top: 5px'
                 tal:content="view/information_type_description"></div>

=== modified file 'lib/lp/bugs/templates/bug-portlet-subscription.pt'
--- lib/lp/bugs/templates/bug-portlet-subscription.pt	2012-02-01 15:31:32 +0000
+++ lib/lp/bugs/templates/bug-portlet-subscription.pt	2012-06-01 06:57:21 +0000
@@ -42,9 +42,9 @@
                   'hidden' if not view.user_should_see_mute_link else None"
             id="mute-link-container">
           <span tal:replace="structure context_menu/mute_subscription/render"
-          />&nbsp;<a target="help" class="sprite maybe mute-help"
+          /><a target="help" class="sprite maybe mute-help"
               href="/+help-bugs/subscription-mute.html"
-            >&nbsp;<span class="invisible-link">Mute help</span></a>
+            ><span class="invisible-link">Mute help</span></a>
         </div>
       </li>
       <li tal:content="structure context_menu/editsubscriptions/render" />

=== modified file 'lib/lp/bugs/templates/bug-portlet-watch.pt'
--- lib/lp/bugs/templates/bug-portlet-watch.pt	2011-04-14 16:37:22 +0000
+++ lib/lp/bugs/templates/bug-portlet-watch.pt	2012-06-01 06:57:21 +0000
@@ -11,7 +11,7 @@
       <a tal:replace="structure watch/fmt:external-link" />
       <tal:block tal:condition="watch/remotestatus">
         <br />[<span tal:content="watch/remotestatus" />]
-        </tal:block>&nbsp;<a
+        </tal:block><a
         tal:attributes="href watch/fmt:url"
         class="sprite edit" alt="Change watch details"
         ><span class="invisible-link">[edit]</span></a>

=== modified file 'lib/lp/bugs/templates/bugtarget-macros-filebug.pt'
--- lib/lp/bugs/templates/bugtarget-macros-filebug.pt	2012-05-25 14:38:42 +0000
+++ lib/lp/bugs/templates/bugtarget-macros-filebug.pt	2012-06-01 06:57:21 +0000
@@ -387,9 +387,8 @@
                       content="bug/date_last_updated/fmt:approximatedate">
                     2007-07-03
                   </tal:last-updated>
-                  &nbsp;&nbsp;
                   <a class="sprite new-window view-bug-link" target="_blank"
-		      tal:attributes="href bug/fmt:url"
+                  tal:attributes="href bug/fmt:url"
                   >view this bug</a>
                 </span>
               </div>

=== modified file 'lib/lp/code/templates/daily-builds-listing.pt'
--- lib/lp/code/templates/daily-builds-listing.pt	2012-04-27 19:17:39 +0000
+++ lib/lp/code/templates/daily-builds-listing.pt	2012-06-01 06:57:21 +0000
@@ -20,8 +20,7 @@
           tal:attributes="action view/form_action|request/URL">
       Show packages with a successful recipe <a
         href="https://help.launchpad.net/Packaging/SourceBuilds/GettingStarted";
-        class="sprite maybe"
-        >&nbsp;<span class="invisible-link">Tag help</span></a>build
+        class="sprite maybe"><span class="invisible-link">Tag help</span></a>build
       <tal:build-age-selector replace="structure view/widgets/when_completed_filter"/>
       <input id="filter_form_submit" type="submit" value="Filter"/>
     </form>

=== modified file 'lib/lp/translations/templates/translationgroup-index.pt'
--- lib/lp/translations/templates/translationgroup-index.pt	2010-09-03 06:37:26 +0000
+++ lib/lp/translations/templates/translationgroup-index.pt	2012-06-01 06:57:21 +0000
@@ -84,7 +84,7 @@
                   </span>
                   <tal:notadmin
                      condition="not:view/user_can_edit">
-                    &nbsp;<a tal:condition="
+                    <a tal:condition="
                          translator/context/required:launchpad.Edit"
                          tal:attributes="href string:${translator/code}/+edit"
                          class="edit sprite"></a>