launchpad-reviewers team mailing list archive

[Henning Eggers <henning.eggers@xxxxxxxxxxxxx>]

Henning Eggers has proposed merging lp:~henninge/launchpad/devel-bugjamming-1 into lp:launchpad.

Requested reviews:
  Edwin Grubbs (edwin-grubbs): ui
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #619121 Inactive template in rosetta: Confusing wording
  https://bugs.launchpad.net/bugs/619121
  #670821 translator comment formating lost
  https://bugs.launchpad.net/bugs/670821


= Summary =

Fixes two little ui bugs, bug 619121 and bug 670821.

== Proposed fix ==

Change the title of a template's 'iscurrent' property to "Template is
active". This is consistent with templates being called "inactive" in the
UI.

Add a <pre> tag around source comments on the translate page to preserve
their formatting. This is mostly important if the comments are multilined.

== Pre-implementation notes ==

Discussed with Danilo. He would also like to normalize leading spaces on
source comments but that would take this branch beyound 'trivial' (and it is
Friday night ;). So we agreed on this solution.

== Implementation details ==

There is a screenshot in the bug that shows a rather large comment that lost
all of its formatting.
https://bugs.launchpad.net/launchpad/+bug/670821/+attachment/1722666/+files/rosetta.png

Here is one I prepared myself, before and after.
http://people.canonical.com/~henninge/screenshots/source_comment_before.png
http://people.canonical.com/~henninge/screenshots/source_comment_after.png

The <pre> tag adds some of its own formatting but the discreet class makes
it look better in my opinion.

== Tests ==

No tests affected.

== Demo and Q/A ==

Demo for bug 619121:
 1. Go here: https://translations.launchpad.dev/evolution/trunk/+templates
 2. Click "Edit" on the first template.
 3. Scroll down to find the "Template is active" checkbox.
 4. Unchcheck it and click "Change"
 5. The template is now "inactive"

QA for bug 670821:
Any translate page of the chromioum-browser project should show multi-line
comments.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/interfaces/potemplate.py
  lib/lp/translations/templates/currenttranslationmessage-translate-one.pt
-- 
https://code.launchpad.net/~henninge/launchpad/devel-bugjamming-1/+merge/44070
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~henninge/launchpad/devel-bugjamming-1 into lp:launchpad.

=== modified file 'lib/lp/translations/interfaces/potemplate.py'
--- lib/lp/translations/interfaces/potemplate.py	2010-12-01 11:26:57 +0000
+++ lib/lp/translations/interfaces/potemplate.py	2010-12-17 17:16:30 +0000
@@ -134,7 +134,7 @@
         required=True)
 
     iscurrent = exported(Bool(
-        title=_("Accept translations?"),
+        title=_("Template is active"),
         description=_(
             "If unchecked, people can no longer change the template's "
             "translations."),

=== modified file 'lib/lp/translations/templates/currenttranslationmessage-translate-one.pt'
--- lib/lp/translations/templates/currenttranslationmessage-translate-one.pt	2010-12-01 11:26:57 +0000
+++ lib/lp/translations/templates/currenttranslationmessage-translate-one.pt	2010-12-17 17:16:30 +0000
@@ -102,8 +102,10 @@
   <th class="icon left right">
     <span class="sprite info" title="Developer's note to translators"></span>
   </th>
-  <td tal:content="view/source_comment">
+  <td>
+    <pre class="discreet" tal:content="view/source_comment">
       This refers to the content type.
+    </pre>  
   </td>
 </tr>
 <tr