launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/no-write-sharing-lies into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1023427 in Launchpad itself: "Sharing implies you can edit"
  https://bugs.launchpad.net/launchpad/+bug/1023427

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/no-write-sharing-lies/+merge/118800

Summary
=======
We can make +sharing read-only via flags, but the choicesource remains a
clickable item -- it sets the cursor and text-decoration as a link, but
doesn't do anything when clicked.

ChoiceSource has a clickable_content attribute--when false, the content
should not be styled as though it is clickable.

Preimp
======
Spoke with Curtis Hovey.

Implementation
==============
A new css class for the choicesource is added -- no-click. It's not named
yui3-choicesource-no-click b/c we might want to use this same css elsewhere.
It sets cursor to default and text decoration to none on hover.

On init in choicesource, if clickable_content is false, no-click is added to
the contentBox's classes.

Tests
=====
bin/test -t choiceedit --layer=YUI

QA
==
Make sure that the choicesources on +sharing aren't clickable when the
writable flag is off.

LoC
===
This is part of disclosure.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/launchpad/icing/css/forms.css
  lib/lp/app/javascript/choiceedit/choiceedit.js
  lib/lp/app/javascript/choiceedit/tests/test_choiceedit.js

./lib/canonical/launchpad/icing/css/forms.css
< ... snip ... >

There are a bunch of errors in forms.css, but it's not a file I want to
reformat. I assume if every line lints wrongly the same way, there's a reason.
-- 
https://code.launchpad.net/~jcsackett/launchpad/no-write-sharing-lies/+merge/118800
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/no-write-sharing-lies into lp:launchpad.

=== modified file 'lib/canonical/launchpad/icing/css/forms.css'
--- lib/canonical/launchpad/icing/css/forms.css	2012-07-31 08:29:31 +0000
+++ lib/canonical/launchpad/icing/css/forms.css	2012-08-08 17:18:30 +0000
@@ -264,6 +264,12 @@
     text-decoration: underline;
     cursor: pointer;
     }
+
+.no-click:hover {
+    text-decoration: none !important;
+    cursor: default !important;
+    }
+    
 .yui3-buglisting-config-util a {
     position: relative;
     top: 3px;

=== modified file 'lib/lp/app/javascript/choiceedit/choiceedit.js'
--- lib/lp/app/javascript/choiceedit/choiceedit.js	2012-06-29 19:36:57 +0000
+++ lib/lp/app/javascript/choiceedit/choiceedit.js	2012-08-08 17:18:30 +0000
@@ -171,6 +171,10 @@
          * @preventable _saveData
          */
         this.publish(SAVE);
+        if (!this.get('clickable_content')) {
+            var content = this.get('contentBox');
+            content.addClass('no-click');
+        };
     },
 
     /**

=== modified file 'lib/lp/app/javascript/choiceedit/tests/test_choiceedit.js'
--- lib/lp/app/javascript/choiceedit/tests/test_choiceedit.js	2012-06-15 18:35:20 +0000
+++ lib/lp/app/javascript/choiceedit/tests/test_choiceedit.js	2012-08-08 17:18:30 +0000
@@ -331,8 +331,13 @@
               "ChoiceList object is not being created");
             Assert.isNotNull(Y.one(document).one(".yui3-ichoicelist"),
               "ChoiceList HTML is not being added to the page");
+        },
+
+        test_choicesource_has_no_click: function () {
+            var contentBox = this.choice_edit.get('contentBox');
+            Assert.isTrue(contentBox.hasClass('no-click'),
+                          "no-click not applied to choicelist html.");
         }
-
     }));
 
     /**