launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #07332
[Merge] lp:~sinzui/launchpad/progressive-enhancement-ftw into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/progressive-enhancement-ftw into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #367533 in Launchpad itself: "Collapsible fieldsets don't degrade gracefully when Javascript isn't available"
https://bugs.launchpad.net/launchpad/+bug/367533
Bug #954072 in Launchpad itself: "IE users cannot use inline-pickers or find html forms to change their data"
https://bugs.launchpad.net/launchpad/+bug/954072
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/progressive-enhancement-ftw/+merge/103733
Several features fail for less common browsers because noscript is used
for the default markup. When progressive enhancement fails, there is no
default markup to fallback to. YUI assumes progressive enhancement works.
Over the last 3 years, we have seen IE, Opera, Konquorer and Firefox 3
break because progressive enhancement failed without fallback.
This branch fixes half of the issues which prevent users from completing
tasks.
--------------------------------------------------------------------
RULES
Pre-implementation: wgrant
* Remove all uses of noscript that have a YUI counterpart.
* Remove noscript that setups up visibility state.
QA
These acceptance tests require firefox, chromium, ie8 and knoquorer.
Also, one browser should have js disabled to verify a plain html link
is always available:
Person Picker
* Visit https://qastaging.launchpad.net/launchpad
* Verify there is an action change the maintainer and the driver.
The driver action also has a help link.
* Verify that the action shows the overlay or html page.
Person Picker and Picker Patcher
* Visit https://blueprints.qastaging.launchpad.net/gdp/+spec/gdplaunchpad
* Verify there the drafter and implementations status can be changed
Multucheckbox Widget
* Visit https://code.qastaging.launchpad.net/~sinzui/+recipe/pocket-lint-daily
* Verify there the distribution series can be changed.
Official bug tags
* Visit https://bugs.qastaging.launchpad.net/launchpad/+manage-official-tags
* Verify the plain html form is shown to non-js browsers.
* Verify that the js form is shown to js-enabled-browsers
Project group choose product
* Visit https://bugs.qastaging.launchpad.net/launchpad-project/+filebug
* Choose Launchpad, enter a summary, then choose Next.
* In a js-enabled browser verify the URL says that you are at
launchpad/+filebug
* In a non-js browser verify the URL is still at launchpad-project
and you can see Launchpad selected in the project field
Configuration Progress
* Visit https://qastaging.launchpad.net/launchpad/
* In a js-enabled browser, verify the Configuration Progress is shown,
/but/ the applications are collapsed.
* In a non-js browser, verify the Configuration Progress is shown,
/and/ the applications are shown too.
File Bug Extra Options
* Visit https://bugs.qastaging.launchpad.net/launchpad/+filebug
* Enter 'athena' in the summary summary and choose Next.
* In a js-enabled browser, verify extra options is collapsed.
* In a non-js browser, verify the extra options are visible.
LINT
lib/lp/app/doc/lazr-js-widgets.txt
lib/lp/app/javascript/picker/tests/test_personpicker.html
lib/lp/app/javascript/picker/tests/test_picker_patcher.html
lib/lp/app/javascript/tests/test_multicheckboxwidget.html
lib/lp/app/templates/inline-multicheckbox-widget.pt
lib/lp/app/templates/inline-picker.pt
lib/lp/bugs/javascript/official_bug_tags.js
lib/lp/bugs/javascript/tests/test_official_bug_tags.html
lib/lp/bugs/javascript/tests/test_official_bug_tags.js
lib/lp/bugs/templates/bugtarget-macros-filebug.pt
lib/lp/bugs/templates/official-bug-target-manage-tags.pt
lib/lp/registry/templates/product-index.pt
TEST
./bin/test -vv -t lazr-js-widgets lp.app.tests.test_doc
./bin/test -vv --layer=YUItest lp.app
./bin/test -vv --layer=YUItest lp.bugs
IMPLEMENTATION
The picker, the patcher and multicheckbox widget all used a button for
js browsers, and an anchor for non-js browsers. All three cases were
fixed by replacing the button with the anchor and adding "lazr-btn
yui3-activator-act" to the class. Note that the inline edit picker
doctest was wrong...it never tested that the widget rendered in edit
mode with all the proper css! Two of the widget templates needed a root
element so that I could lint them.
lib/lp/app/doc/lazr-js-widgets.txt
lib/lp/app/javascript/picker/tests/test_personpicker.html
lib/lp/app/javascript/picker/tests/test_picker_patcher.html
lib/lp/app/javascript/tests/test_multicheckboxwidget.html
lib/lp/app/templates/inline-multicheckbox-widget.pt
lib/lp/app/templates/inline-picker.pt
I removed the noscript around the plain official bug tags html form and
added a line to the script to hide the html form when setup completes.
Alas there was no YUI test for the script, and the script had lint. I
did the minimum to hush lint and test my line of code.
lib/lp/bugs/javascript/official_bug_tags.js
lib/lp/bugs/javascript/tests/test_official_bug_tags.html
lib/lp/bugs/javascript/tests/test_official_bug_tags.js
lib/lp/bugs/templates/official-bug-target-manage-tags.pt
I removed the noscript from the product widget because it is only shown
on the project group page. It might have been needed in the past, but
users with js browsers cannot see it anyway because when you make your
selection, your browser changes the context to the real product, so Lp
was hiding the widget on a page that js users never load. See below for
the explanation of why 'unseen' is not needed.
lib/lp/bugs/templates/bugtarget-macros-filebug.pt
The lp.app.expander module is our friend. It handles the setup and
fallback rules for blocks with the 'collapsible' class. Pages and
scripts do not need to set the initial state of visibility or add rules
to ensure non-js browsers can see the block. I removed the 'unseen'
class from +filebug extra options and the css redefines for collapsible
because the markup is visible by default. These rules are cruft that
could have been removed when we enabled lp.app.expander lp.app.expander.
lib/lp/registry/templates/product-index.pt
--
https://code.launchpad.net/~sinzui/launchpad/progressive-enhancement-ftw/+merge/103733
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/progressive-enhancement-ftw into lp:launchpad.
=== modified file 'lib/lp/app/doc/lazr-js-widgets.txt'
--- lib/lp/app/doc/lazr-js-widgets.txt 2012-04-10 14:01:17 +0000
+++ lib/lp/app/doc/lazr-js-widgets.txt 2012-04-26 16:34:44 +0000
@@ -249,18 +249,20 @@
huge, the different items are shown in the normal paginated way for the user
to select.
- >>> owner = IArchive['owner']
- >>> widget = InlineEditPickerWidget(archive, owner, default_text)
+ >>> ignore = login_person(product.owner)
+ >>> owner = IProduct['owner']
+ >>> widget = InlineEditPickerWidget(product, owner, default_text)
>>> print widget()
<span id="edit-owner">
<span class="yui3-activator-data-box">
- <a...>Eric</a>
+ <a href="/~eric" class="sprite person">Eric</a>
</span>
- <button class="lazr-btn yui3-activator-act yui3-activator-hidden">
- Edit
- </button>
- <div class="yui3-activator-message-box yui3-activator-hidden"></div>
- </span>
+ <span>
+ <a class="sprite edit lazr-btn yui3-activator-act"
+ href="http://launchpad.dev/widget/+edit";
+ title=""> <span class="invisible-link">Edit</span></a>
+ <div class="yui3-activator-message-box yui3-activator-hidden"></div>
+ </span> ...
Picker headings
@@ -462,15 +464,10 @@
<span id="edit-distroseries">
<dt>
Recipe distro series
- <button class="lazr-btn yui3-activator-act yui3-activator-hidden"
- id="edit-distroseries-btn">
- Edit
- </button>
- <noscript>
- <a class="sprite edit"
+ <a class="sprite edit lazr-btn yui3-activator-act"
href="http://code.launchpad.dev/~eric/+recipe/cake_recipe/+edit";
- title=""></a>
- </noscript>
+ id="edit-distroseries-btn"
+ title=""> <span class="invisible-link">Edit</span></a>
</dt>
<span class="yui3-activator-data-box">
<dl id='edit-distroseries-items'>
=== modified file 'lib/lp/app/javascript/picker/tests/test_personpicker.html'
--- lib/lp/app/javascript/picker/tests/test_personpicker.html 2012-03-14 04:41:36 +0000
+++ lib/lp/app/javascript/picker/tests/test_personpicker.html 2012-04-26 16:34:44 +0000
@@ -70,10 +70,10 @@
<span id="pickertest">
<span>
A picker widget test
- <button id="edit-pickertest-btn"
- class="lazr-btn yui3-activator-act yui3-activator-hidden">
- Edit
- </button>
+ <a id="edit-pickertest-btn"
+ class="sprite edit lazr-btn yui3-activator-act"
+ href="/fnord/+edit-people"
+ > Edit</a>
</span>
<span class="yui3-activator-data-box">
</span>
=== modified file 'lib/lp/app/javascript/picker/tests/test_picker_patcher.html'
--- lib/lp/app/javascript/picker/tests/test_picker_patcher.html 2012-03-14 04:41:36 +0000
+++ lib/lp/app/javascript/picker/tests/test_picker_patcher.html 2012-04-26 16:34:44 +0000
@@ -67,10 +67,10 @@
<span id="pickertest">
<span>
A picker widget test
- <button id="edit-pickertest-btn"
- class="lazr-btn yui3-activator-act yui3-activator-hidden">
- Edit
- </button>
+ <a id="edit-pickertest-btn"
+ class="sprite edit lazr-btn yui3-activator-act"
+ href="/fnord/+edit-people"
+ > Edit</a>
</span>
<span class="yui3-activator-data-box">
</span>
=== modified file 'lib/lp/app/javascript/tests/test_multicheckboxwidget.html'
--- lib/lp/app/javascript/tests/test_multicheckboxwidget.html 2012-03-14 04:41:36 +0000
+++ lib/lp/app/javascript/tests/test_multicheckboxwidget.html 2012-04-26 16:34:44 +0000
@@ -50,10 +50,9 @@
<span id="multicheckboxtest">
<span>
A multicheckbox widget test
- <button id="edit-multicheckboxtest-btn"
- class="lazr-btn yui3-activator-act yui3-activator-hidden">
- Edit
- </button>
+ <a id="edit-multicheckboxtest-btn"
+ class="sprite edit lazr-btn yui3-activator-act"
+ >Edit</a>
</span>
<span class="yui3-activator-data-box">
<span id="edit-test-items"></span>
=== modified file 'lib/lp/app/templates/inline-multicheckbox-widget.pt'
--- lib/lp/app/templates/inline-multicheckbox-widget.pt 2012-02-01 15:31:32 +0000
+++ lib/lp/app/templates/inline-multicheckbox-widget.pt 2012-04-26 16:34:44 +0000
@@ -3,16 +3,12 @@
<tal:label-open-tag replace="structure view/label_open_tag"/>
<span tal:replace="structure view/label"/>
<tal:has_choices condition="python:view.has_choices and view.can_write">
- <button tal:attributes="id string:${view/content_box_id}-btn"
- class="lazr-btn yui3-activator-act yui3-activator-hidden">
- Edit
- </button>
+ <a tal:attributes="id string:${view/content_box_id}-btn;
+ href view/edit_url;
+ title view/edit_title"
+ class="sprite edit lazr-btn yui3-activator-act"
+ > <span class="invisible-link">Edit</span></a>
</tal:has_choices>
- <noscript tal:condition="view/can_write">
- <a tal:attributes="href view/edit_url;
- title view/edit_title"
- class="sprite edit"></a>
- </noscript>
<tal:label-close-tag replace="structure view/label_close_tag"/>
<span class="yui3-activator-data-box">
<tal:items-open-tag replace="structure view/items_open_tag"/>
=== modified file 'lib/lp/app/templates/inline-picker.pt'
--- lib/lp/app/templates/inline-picker.pt 2012-03-21 01:26:45 +0000
+++ lib/lp/app/templates/inline-picker.pt 2012-04-26 16:34:44 +0000
@@ -1,33 +1,28 @@
-<span tal:attributes="id view/content_box_id">
+<tal:root
+ xmlns:tal="http://xml.zope.org/namespaces/tal";
+ xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+ omit-tag="">
+ <span tal:attributes="id view/content_box_id">
<span class="yui3-activator-data-box">
<tal:attribute replace="structure view/default_html"/>
</span>
- <button class="lazr-btn yui3-activator-act yui3-activator-hidden">
- Edit
- </button>
-
- <a tal:condition="view/help_link"
- tal:attributes="href view/help_link"
- target="help"
- class="sprite maybe">
- <span class="invisible-link">Driver help</span>
- </a>
-
- <noscript tal:condition="view/can_write">
+ <span tal:condition="view/can_write">
<a tal:attributes="href view/edit_url;
title view/edit_title"
- class="sprite edit"></a>
+ class="sprite edit lazr-btn yui3-activator-act"
+ > <span class="invisible-link">Edit</span></a>
+
<a tal:condition="view/help_link"
tal:attributes="href view/help_link"
target="help"
class="sprite maybe">
<span class="invisible-link">Driver help</span>
</a>
- </noscript>
- <div class="yui3-activator-message-box yui3-activator-hidden"></div>
-</span>
+ <div class="yui3-activator-message-box yui3-activator-hidden"></div>
+ </span>
+ </span>
-<script tal:condition="view/can_write"
+ <script tal:condition="view/can_write"
tal:content="structure string:
LPJS.use('lp.app.picker', 'lp.client', function(Y) {
var picker_config = ${view/json_config}
@@ -41,4 +36,5 @@
picker_config);
}, window);
});
-"/>
+ "/>
+</tal:root>
=== modified file 'lib/lp/bugs/javascript/official_bug_tags.js'
--- lib/lp/bugs/javascript/official_bug_tags.js 2012-03-15 05:40:30 +0000
+++ lib/lp/bugs/javascript/official_bug_tags.js 2012-04-26 16:34:44 +0000
@@ -427,6 +427,9 @@
e.halt();
save_tags();
});
+
+ // All went well, hide the plain html form.
+ Y.one('form[name=launchpadform]').setStyle('display', 'none');
};
}, "0.1", {
"requires": [
=== added file 'lib/lp/bugs/javascript/tests/test_official_bug_tags.html'
--- lib/lp/bugs/javascript/tests/test_official_bug_tags.html 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/javascript/tests/test_official_bug_tags.html 2012-04-26 16:34:44 +0000
@@ -0,0 +1,134 @@
+<!DOCTYPE html>
+<!--
+Copyright 2012 Canonical Ltd. This software is licensed under the
+GNU Affero General Public License version 3 (see the file LICENSE).
+-->
+
+<html>
+ <head>
+ <title>Test official_bug_tags</title>
+
+ <!-- YUI and test setup -->
+ <script type="text/javascript"
+ src="../../../../../build/js/yui/yui/yui.js">
+ </script>
+ <link rel="stylesheet"
+ href="../../../../../build/js/yui/console/assets/console-core.css" />
+ <link rel="stylesheet"
+ href="../../../../../build/js/yui/console/assets/skins/sam/console.css" />
+ <link rel="stylesheet"
+ href="../../../../../build/js/yui/test/assets/skins/sam/test.css" />
+
+ <script type="text/javascript"
+ src="../../../../../build/js/lp/app/testing/testrunner.js"></script>
+ <link rel="stylesheet" href="../../../app/javascript/testing/test.css" />
+
+ <!-- Dependencies -->
+ <script type="text/javascript" src="../../../../../build/js/yui/base/base.js"></script>
+ <script type="text/javascript" src="../../../../../build/js/yui/node/node.js"></script>
+ <script type="text/javascript" src="../../../../../eggs/build/widget/widget-position-ext.js"></script>
+ <script type="text/javascript" src="../../../../../build/js/yui/collection/collection.js"></script>
+ <script type="text/javascript" src="../../../../../build/js/yui/collection/array-extras.js"></script>
+ <script type="text/javascript" src="../../../../../build/js/yui/substitute/substitute.js"></script>
+ <script type="text/javascript" src="../../../../../build/js/lp/app/lazr/lazr.js"></script>
+ <script type="text/javascript" src="../../../../../build/js/lp/app/overlay/overlay.js"></script>
+
+ <!-- The module under test. -->
+ <script type="text/javascript" src="../official_bug_tags.js"></script>
+
+ <!-- The test suite. -->
+ <script type="text/javascript" src="test_official_bug_tags.js"></script>
+ </head>
+ <body class="yui3-skin-sam">
+ <ul id="suites">
+ <li>lp.bugs.official_bug_tags.test</li>
+ </ul>
+
+ <div id="fixture"></div>
+
+ <script type="text/javascript">
+ // The page must provide these three global variables.
+ var used_bug_tags = {"trivial": 1, "easy": 1};
+ var official_bug_tags = ["exception", "feature", "ui"];
+ var valid_name_pattern = "^[a-z0-9][a-z0-9\\+\\.\\-]+$";
+ </script>
+
+ <script type="text/x-template" id="form-template">
+ <div>
+ <form name="launchpadform">
+ <input type="text" name="tags" />
+ <input type="submit" name="Save" />
+ </form>
+ </div>
+
+ <table id="layout-table" class="official-tags-layout-table hidden">
+ <tr>
+ <th>Official tags</th>
+ <th></th>
+ <th>Other tags</th>
+ </tr>
+ <tr>
+ <td id="left-column" class="left-column">
+ <div class="list-scrollable-area">
+ <ul id="official-tags-list">
+ </ul>
+ </div>
+ </td>
+ <td id="middle-column" class="middle-column">
+ <input type="submit" value="←"
+ id="add-official-tags" class="arrow-button"
+ disabled="true" />
+ <br />
+ <input type="submit" value="→;"
+ id="remove-official-tags" class="arrow-button"
+ disabled="true" />
+ <br />
+ <form action="+manage-official-tags" method="post" id="save-form">
+ <input type="hidden"
+ id="field-official_bug_tags"
+ name="field.official_bug_tags" />
+ <input type="hidden" name="field.actions.save" value="Save" />
+ </form>
+ </td>
+ <td id="right-column" class="right-column">
+ <div class="list-scrollable-area">
+ <ul id="other-tags-list">
+ </ul>
+ </div>
+ </td>
+ </tr>
+ <tr>
+ <td class="left-column">
+ <br />
+ <table class="input-field-layout-table">
+ <tr>
+ <td colspan="2">
+ Add a new official tag:
+ </td>
+ </tr>
+ <tr>
+ <td style="width: 100%">
+ <input type="text" id="new-tag-text" style="width: 100%" />
+ </td>
+ <td>
+ <input type="submit" value="Add" id="new-tag-add"
+ disabled="true" />
+ </td>
+ </tr>
+ </table>
+ </td>
+ <td class="middle-column">
+ </td>
+ <td class="right-column">
+ </td>
+ </tr>
+ <tr>
+ <td class="actions" colspan="3">
+ <input type="submit" value="Save" id="save-button"
+ disabled="true" />
+ </td>
+ </tr>
+ </table>
+ </script>
+ </body>
+</html>
=== added file 'lib/lp/bugs/javascript/tests/test_official_bug_tags.js'
--- lib/lp/bugs/javascript/tests/test_official_bug_tags.js 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/javascript/tests/test_official_bug_tags.js 2012-04-26 16:34:44 +0000
@@ -0,0 +1,36 @@
+YUI().use('lp.testing.runner', 'base', 'test', 'console',
+ 'node', 'lp.bugs.official_bug_tags', function(Y) {
+
+var suite = new Y.Test.Suite("Official Bug Tags Tests");
+var module = Y.lp.bugs.official_bug_tags;
+
+
+suite.add(new Y.Test.Case({
+ name: 'Official Bug Tags',
+
+ setUp: function() {
+ this.fixture = Y.one('#fixture');
+ var tags_form = Y.Node.create(
+ Y.one('#form-template').getContent());
+ this.fixture.appendChild(tags_form);
+ },
+
+ tearDown: function() {
+ if (this.fixture !== null) {
+ this.fixture.empty();
+ }
+ delete this.fixture;
+ },
+
+ test_setup_bug_tags_table: function() {
+ // The bug tags table is visible and the html form is not.
+ module.setup_official_bug_tag_management();
+ html_form = Y.one('[name=launchpadform]');
+ tags_table = Y.one('#layout-table');
+ Y.Assert.areEqual('none', html_form.getStyle('display'));
+ Y.Assert.areEqual('block', tags_table.getStyle('display'));
+ }
+}));
+
+Y.lp.testing.Runner.run(suite);
+});
=== modified file 'lib/lp/bugs/templates/bugtarget-macros-filebug.pt'
--- lib/lp/bugs/templates/bugtarget-macros-filebug.pt 2012-04-20 01:12:05 +0000
+++ lib/lp/bugs/templates/bugtarget-macros-filebug.pt 2012-04-26 16:34:44 +0000
@@ -44,12 +44,10 @@
</td>
</tr>
- <noscript>
<tal:product tal:define="widget nocall:view/widgets/product|nothing"
tal:condition="widget">
<metal:widget use-macro="context/@@launchpad_form/widget_row" />
</tal:product>
- </noscript>
<metal:summary tal:define="widget nocall:view/widgets/title">
<metal:row use-macro="context/@@launchpad_form/widget_row" />
@@ -66,7 +64,7 @@
<td colspan="2">
<fieldset id="filebug-extra-options" class="collapsible">
<legend>Extra options</legend>
- <div class="unseen">
+ <div>
<table>
<tal:status
define="widget nocall:view/widgets/status|nothing"
=== modified file 'lib/lp/bugs/templates/official-bug-target-manage-tags.pt'
--- lib/lp/bugs/templates/official-bug-target-manage-tags.pt 2012-02-01 15:31:32 +0000
+++ lib/lp/bugs/templates/official-bug-target-manage-tags.pt 2012-04-26 16:34:44 +0000
@@ -17,9 +17,7 @@
<div metal:fill-slot="main">
<div class="yui-g">
<div class="yui-u first">
- <noscript>
- <div metal:use-macro="context/@@launchpad_form/form" />
- </noscript>
+ <div metal:use-macro="context/@@launchpad_form/form" />
<script tal:replace="structure view/tags_js_data" />
<script type="text/javascript">
=== modified file 'lib/lp/registry/templates/product-index.pt'
--- lib/lp/registry/templates/product-index.pt 2012-03-20 03:03:07 +0000
+++ lib/lp/registry/templates/product-index.pt 2012-04-26 16:34:44 +0000
@@ -14,11 +14,6 @@
<head>
<tal:head-epilogue metal:fill-slot="head_epilogue">
- <noscript>
- <style type="text/css">
- div.collapsible, div.collapsed {display: block;}
- </style>
- </noscript>
<tal:uses_launchpad_bugtracker
condition="context/bug_tracking_usage/enumvalue:LAUNCHPAD">
<script type="text/javascript">
Follow ups
