Merge lp:~huwshimi/launchpad/remove-icon-alt-528367 into lp:launchpad

[Huw Wilkins]

[r=allenap][bug=528367] Added alt tag to remove icons.

Added alt tag to remove icons.

To test find a remove icon (e.g. subscribe to bug and there will be a red remove icon on the right of your name) and view the source (or inspect the element) and the <img> tag should have the "alt" property set to "Remove".

[Jonathan Lange (jml)]

Is there a way we can do this so that we don't have to remember to add the alt="" every time?

[Gavin Panella (allenap)]

+1, but I'm also interested to know if there's a simple answer to jml's question.

It looks like you've mixed up the Description of the Change and Commit Message field in the proposal. Before landing you might want to switch them around.

Also, there's no need to mention the bug number - though it's harmless - in the commit message. You've linked the bug to the branch already, and bin/ec2 land and bzr lp-land will add a [bug=$] part to your commit message automatically.

review: Approve

[Huw Wilkins (huwshimi)]

> Is there a way we can do this so that we don't have to remember to add the
> alt="" every time?

There's certainly no standard way of doing that. I guess we could do something like have a whole bunch of templates with img tags in them set up for each image and we just include that. However, I think it's just the nature of html to have alt tags on images. Personally I think it's something that should be picked up in UI reviews.

Gavin, thanks for your comments. I've fixed those things.

[Gavin Panella (allenap)]

> There's certainly no standard way of doing that. I guess we could do
> something like have a whole bunch of templates with img tags in them
> set up for each image and we just include that. However, I think
> it's just the nature of html to have alt tags on images. Personally
> I think it's something that should be picked up in UI reviews.

Agreed, I don't think the template thing would work; we'd have to
check that they're used in review anyway. But your review point made
me think...

We could have a linter to check these things. Without looking I'm
going to guess that there are over 100,000 html lint tools out there,
all of which melt when exposed to TAL.

However, a linter for just this could be botched together fairly
easily, and additional checks added as we go along. (Btw, I'm not
suggesting you do it.)

Consider filing a bug to add a template linter with an importance
roughly equal to how important alt attributes are to have on img tags.

=== modified file 'lib/canonical/launchpad/templates/launchpad-graphics.pt'
--- lib/canonical/launchpad/templates/launchpad-graphics.pt	2010-10-31 20:18:45 +0000
+++ lib/canonical/launchpad/templates/launchpad-graphics.pt	2011-02-03 05:22:27 +0000
@@ -413,7 +413,7 @@
       </tr>
       <tr>
         <td>remove</td>
-        <td><img src="/@@/remove" /></td>
+        <td><img src="/@@/remove" alt="Remove" /></td>
         <td>Can be clicked to "remove" data from the system. Is sometimes
         used as a bullet on an action menu item which results in data
         removal, or in lists where clicking it will remove the data in that

=== modified file 'lib/lp/answers/templates/person-answer-contact-for.pt'
--- lib/lp/answers/templates/person-answer-contact-for.pt	2009-08-20 14:37:15 +0000
+++ lib/lp/answers/templates/person-answer-contact-for.pt	2011-02-03 05:22:27 +0000
@@ -16,7 +16,7 @@
           tal:attributes="href question_target/fmt:url:answers"
           tal:content="question_target/title">Project Title</a>
         <tal:link condition="view/showRemoveYourselfLink">
-          <img src="/@@/remove" />&mdash;
+          <img src="/@@/remove" alt="Remove" />&mdash;
           <a id="project-setanswercontact" href="#"
             tal:attributes="href string:${question_target/fmt:url}/+support-contact;
             id string:${question_target/name}-setdirectanswercontact">

=== modified file 'lib/lp/bugs/javascript/bugtask_index.js'
--- lib/lp/bugs/javascript/bugtask_index.js	2011-01-21 05:10:55 +0000
+++ lib/lp/bugs/javascript/bugtask_index.js	2011-02-03 05:22:27 +0000
@@ -830,7 +830,7 @@
     if (subscription.can_be_unsubscribed_by_user()) {
         var icon_html = Y.Node.create(
             '<a href="+subscribe">' +
-            '<img class="unsub-icon" src="/@@/remove" /></a>');
+            '<img class="unsub-icon" src="/@@/remove" alt="Remove" /></a>');
         icon_html
             .set('id', 'unsubscribe-' + terms.css_name)
             .set('title', 'Unsubscribe ' + terms.full_name);

=== modified file 'lib/lp/bugs/templates/bug-branch.pt'
--- lib/lp/bugs/templates/bug-branch.pt	2009-11-13 03:05:26 +0000
+++ lib/lp/bugs/templates/bug-branch.pt	2011-02-03 05:22:27 +0000
@@ -23,7 +23,7 @@
      tal:attributes="
        href string:${branch/fmt:url}/+bug/${bug/id}/+delete;
        id string:bugbranch-${bug_branch/id}-delete;">
-    <img src="/@@/remove"/>
+    <img src="/@@/remove" alt="Remove"/>
   </a>
   <div tal:repeat="proposal view/merge_proposals" class="reviews">
     <tal:merge-fragment tal:replace="structure proposal/@@+summary-fragment"/>

=== modified file 'lib/lp/bugs/templates/bug-portlet-dupe-subscribers-content.pt'
--- lib/lp/bugs/templates/bug-portlet-dupe-subscribers-content.pt	2010-09-02 22:55:21 +0000
+++ lib/lp/bugs/templates/bug-portlet-dupe-subscribers-content.pt	2011-02-03 05:22:27 +0000
@@ -41,7 +41,7 @@
                  class python: view.getSubscriptionClassForUser(subscription.person)
                "
             >
-              <img class="unsub-icon" src="/@@/remove"
+              <img class="unsub-icon" src="/@@/remove" alt="Remove"
                 tal:attributes="id string:unsubscribe-icon-${subscription/css_name}" />
             </a>
     </div>

=== modified file 'lib/lp/bugs/templates/bug-portlet-subscribers-content.pt'
--- lib/lp/bugs/templates/bug-portlet-subscribers-content.pt	2009-12-18 20:54:13 +0000
+++ lib/lp/bugs/templates/bug-portlet-subscribers-content.pt	2011-02-03 05:22:27 +0000
@@ -39,7 +39,7 @@
                  class python: view.getSubscriptionClassForUser(subscription.person)
                "
             >
-              <img class="unsub-icon" src="/@@/remove"
+              <img class="unsub-icon" src="/@@/remove" alt="Remove"
                 tal:attributes="id string:unsubscribe-icon-${subscription/css_name}" />
             </a>
 

=== modified file 'lib/lp/code/templates/branch-macros.pt'
--- lib/lp/code/templates/branch-macros.pt	2011-01-20 04:27:38 +0000
+++ lib/lp/code/templates/branch-macros.pt	2011-02-03 05:22:27 +0000
@@ -123,7 +123,7 @@
              class="delete-buglink"
              tal:attributes="href string:+bug/${bug/id}/+delete;
                              id string:delete-buglink-${bug/id}">
-            <img src="/@@/remove"/>
+            <img src="/@@/remove" alt="Remove"/>
           </a>
         </td>
       </tr>

=== modified file 'lib/lp/code/templates/branch-related-bugs-specs.pt'
--- lib/lp/code/templates/branch-related-bugs-specs.pt	2010-10-19 01:24:36 +0000
+++ lib/lp/code/templates/branch-related-bugs-specs.pt	2011-02-03 05:22:27 +0000
@@ -22,7 +22,7 @@
                  class="delete-buglink"
                  tal:attributes="href string:+bug/${bug/id}/+delete;
                                  id string:delete-buglink-${bug/id}">
-                <img src="/@@/remove"/>
+                <img src="/@@/remove" alt="Remove"/>
               </a>
             </tal:buglink-edit>
           </li>