Merge lp:~rharding/launchpad/1024866_buglink into lp:launchpad

Proposed by Richard Harding
Status: Merged
Approved by: Richard Harding
Approved revision: no longer in the source branch.
Merged at revision: 15632
Proposed branch: lp:~rharding/launchpad/1024866_buglink
Merge into: lp:launchpad
Diff against target: 127 lines (+43/-11)
5 files modified
lib/lp/app/javascript/listing_navigator.js (+18/-2)
lib/lp/app/javascript/tests/test_listing_navigator.js (+20/-6)
lib/lp/bugs/javascript/buglisting.js (+1/-1)
lib/lp/bugs/javascript/tests/test_buglisting.js (+3/-1)
lib/lp/registry/javascript/sharing/shareetable.js (+1/-1)
To merge this branch: bzr merge lp:~rharding/launchpad/1024866_buglink
Reviewer Review Type Date Requested Status
Benji York (community) code Approve
Review via email: mp+115117@code.launchpad.net

Commit message

Fixes 1024866 to limit listing navigator selector scope.

Description of the change

= Summary =

There was an update to the portlet html that the bug listing code ended up
grabbing html from the portlet and trying to turn it into nex/prev links. This
corrects that.

== Implementation Notes ==

This adds helpers to the listing navigator to add a parent node to limit work
within replacing usage of Y.one/all with it's own version.

== Tests ==

test_listing_navigator.js

== LoC Qualification ==

I've got a small credit from recent YUI 3.5 work see branch:

https://code.launchpad.net/~rharding/launchpad/soyuz_yui35

To post a comment you must log in.
Revision history for this message
Benji York (benji) wrote :

Looks good.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/app/javascript/listing_navigator.js'
--- lib/lp/app/javascript/listing_navigator.js 2012-06-27 14:09:43 +0000
+++ lib/lp/app/javascript/listing_navigator.js 2012-07-16 18:00:36 +0000
@@ -28,6 +28,22 @@
28 return url.substr(0, cut_at - 1);28 return url.substr(0, cut_at - 1);
29}29}
3030
31
32/**
33 * Generate selector helpers so that we only find nodes within our own
34 * UI.
35 */
36module.one = function (parent, selector) {
37 var updated_selector = parent + " " + selector;
38 return Y.one(updated_selector);
39};
40
41module.all = function (parent, selector) {
42 var updated_selector = parent + " " + selector;
43 return Y.all(updated_selector);
44};
45
46
31/**47/**
32 * Constructor.48 * Constructor.
33 *49 *
@@ -520,9 +536,9 @@
520 * Rewrite all nodes with navigation classes so that they are hyperlinks.536 * Rewrite all nodes with navigation classes so that they are hyperlinks.
521 * Content is retained.537 * Content is retained.
522 */538 */
523module.linkify_navigation = function() {539module.linkify_navigation = function(parent_selector) {
524 Y.each(['previous', 'next', 'first', 'last'], function(class_name) {540 Y.each(['previous', 'next', 'first', 'last'], function(class_name) {
525 Y.all('.' + class_name).each(function(node) {541 module.all(parent_selector, '.' + class_name).each(function(node) {
526 new_node = Y.Node.create('<a href="#"></a>');542 new_node = Y.Node.create('<a href="#"></a>');
527 new_node.addClass(class_name);543 new_node.addClass(class_name);
528 new_node.setContent(node.getContent());544 new_node.setContent(node.getContent());
529545
=== modified file 'lib/lp/app/javascript/tests/test_listing_navigator.js'
--- lib/lp/app/javascript/tests/test_listing_navigator.js 2012-06-27 20:04:57 +0000
+++ lib/lp/app/javascript/tests/test_listing_navigator.js 2012-07-16 18:00:36 +0000
@@ -181,22 +181,36 @@
181 */181 */
182 test_linkify_navigation: function() {182 test_linkify_navigation: function() {
183 Y.one('#fixture').setContent(183 Y.one('#fixture').setContent(
184 '<span id="notme" class="first"></span>' +
185 '<div id="bugs-table-listing">' +
184 '<span class="previous">PreVious</span>' +186 '<span class="previous">PreVious</span>' +
185 '<span class="next">NeXt</span>' +187 '<span class="next">NeXt</span>' +
186 '<span class="first">FiRST</span>' +188 '<span class="first">FiRST</span>' +
187 '<span class="last">lAst</span>');189 '<span class="last">lAst</span>' +
188 module.linkify_navigation();190 '</div>');
191 module.linkify_navigation('#bugs-table-listing');
189 function checkNav(selector, content) {192 function checkNav(selector, content) {
190 var node = Y.one(selector);193 var nodelist = Y.all(selector);
191 Y.Assert.areEqual('a', node.get('tagName').toLowerCase());194 nodelist.each(function (node) {
192 Y.Assert.areEqual(content, node.getContent());195 if (node.get('id') != 'notme') {
193 Y.Assert.areEqual('#', node.get('href').substr(-1, 1));196 Y.Assert.areEqual('a',
197 node.get('tagName').toLowerCase());
198 Y.Assert.areEqual(content, node.getContent());
199 Y.Assert.areEqual('#',
200 node.get('href').substr(-1, 1));
201 } else {
202 Y.Assert.areEqual('span',
203 node.get('tagName').toLowerCase(),
204 'Ignore nodes outside of the bug listing table');
205 }
206 });
194 }207 }
195 checkNav('.previous', 'PreVious');208 checkNav('.previous', 'PreVious');
196 checkNav('.next', 'NeXt');209 checkNav('.next', 'NeXt');
197 checkNav('.first', 'FiRST');210 checkNav('.first', 'FiRST');
198 checkNav('.last', 'lAst');211 checkNav('.last', 'lAst');
199 },212 },
213
200 /**214 /**
201 * Render should update the navigation_indices with the result info.215 * Render should update the navigation_indices with the result info.
202 */216 */
203217
=== modified file 'lib/lp/bugs/javascript/buglisting.js'
--- lib/lp/bugs/javascript/buglisting.js 2012-03-22 19:08:39 +0000
+++ lib/lp/bugs/javascript/buglisting.js 2012-07-16 18:00:36 +0000
@@ -176,7 +176,7 @@
176 var navigation_indices = Y.all('.batch-navigation-index');176 var navigation_indices = Y.all('.batch-navigation-index');
177 var pre_fetch = Y.lp.app.listing_navigator.get_feature_flag(177 var pre_fetch = Y.lp.app.listing_navigator.get_feature_flag(
178 'bugs.dynamic_bug_listings.pre_fetch');178 'bugs.dynamic_bug_listings.pre_fetch');
179 Y.lp.app.listing_navigator.linkify_navigation();179 Y.lp.app.listing_navigator.linkify_navigation('#bugs-table-listing');
180 var navigator = new module.BugListingNavigator({180 var navigator = new module.BugListingNavigator({
181 current_url: window.location,181 current_url: window.location,
182 cache: LP.cache,182 cache: LP.cache,
183183
=== modified file 'lib/lp/bugs/javascript/tests/test_buglisting.js'
--- lib/lp/bugs/javascript/tests/test_buglisting.js 2012-06-28 13:18:14 +0000
+++ lib/lp/bugs/javascript/tests/test_buglisting.js 2012-07-16 18:00:36 +0000
@@ -103,8 +103,10 @@
103 },103 },
104 test_from_page_with_client: function() {104 test_from_page_with_client: function() {
105 Y.one('#fixture').setContent(105 Y.one('#fixture').setContent(
106 '<div id="bugs-table-listing">' +
106 '<a class="previous" href="http://example.org/">PreVious</span>' +107 '<a class="previous" href="http://example.org/">PreVious</span>' +
107 '<div id="client-listing"></div>');108 '<div id="client-listing"></div>' +
109 '</div>');
108 Y.Assert.areSame('http://example.org/', this.getPreviousLink());110 Y.Assert.areSame('http://example.org/', this.getPreviousLink());
109 module.BugListingNavigator.from_page();111 module.BugListingNavigator.from_page();
110 Y.Assert.areNotSame('http://example.org/', this.getPreviousLink());112 Y.Assert.areNotSame('http://example.org/', this.getPreviousLink());
111113
=== modified file 'lib/lp/registry/javascript/sharing/shareetable.js'
--- lib/lp/registry/javascript/sharing/shareetable.js 2012-07-07 14:00:30 +0000
+++ lib/lp/registry/javascript/sharing/shareetable.js 2012-07-16 18:00:36 +0000
@@ -100,7 +100,7 @@
100100
101 make_navigator: function() {101 make_navigator: function() {
102 var navigation_indices = Y.all('.batch-navigation-index');102 var navigation_indices = Y.all('.batch-navigation-index');
103 Y.lp.app.listing_navigator.linkify_navigation();103 Y.lp.app.listing_navigator.linkify_navigation('#sharee-table-listing');
104 var ns = Y.lp.registry.sharing.shareelisting_navigator;104 var ns = Y.lp.registry.sharing.shareelisting_navigator;
105 var cache = LP.cache;105 var cache = LP.cache;
106 cache.total = this.get('sharees').length;106 cache.total = this.get('sharees').length;