Merge lp:~matthew-constable/tiqit/tiqit into lp:tiqit

Proposed by Matthew Constable
Status: Needs review
Proposed branch: lp:~matthew-constable/tiqit/tiqit
Merge into: lp:tiqit
Diff against target: 67 lines (+35/-0)
2 files modified
static/scripts/coledit.js (+4/-0)
static/scripts/results.js (+31/-0)
To merge this branch: bzr merge lp:~matthew-constable/tiqit/tiqit
Reviewer Review Type Date Requested Status
Matthew Hall Needs Fixing
Review via email: mp+278934@code.launchpad.net

Description of the change

Enhanced coledit.js and results.js to produce the correct URL in the taskbar (and history) when a column is added/removed on a results page.

To post a comment you must log in.
Revision history for this message
Matthew Hall (matt.hall) wrote :

This is a good change, but unfortunately there's currently inconsistent behaviour displayed when search results are grouped by one of the fields.

* When a column is added using the buttons above any group, it is added to all groups and the URL updated. I think this is fine.

* When a column is removed using the buttons above any group but the first, it is removed from only that group and the URL isn't updated. The existing behaviour of the group isn't ideal, but at least your new URL handling is being consistent with it.

* When a column is remove using the buttons above the first group, it is removed from only that group and the URL *is* updated. Again, the existing grouping behavior arguably isn't ideal, but now there's also an inconsistency between the union of all columns displayed on the page and the columns listed in the URL.

We either need to avoid updating the URL when removing a column from the first group, or change the grouping behavior so that removing a column always affects all groups.

review: Needs Fixing

Unmerged revisions

6. By Matthew Constable <email address hidden>

Enhanced coledit.js and results.js to produce the correct URL in the taskbar (and history) when a column is added/removed on a results page.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'static/scripts/coledit.js' (properties changed: -x to +x)
--- static/scripts/coledit.js 2014-06-05 14:38:53 +0000
+++ static/scripts/coledit.js 2015-11-30 09:54:32 +0000
@@ -113,6 +113,7 @@
113 // - Do some magic to make the auto-updater know about the new field113 // - Do some magic to make the auto-updater know about the new field
114 // - Force a background reload to get the first info114 // - Force a background reload to get the first info
115 // - Suppress initial diffs for this col (but only this col)115 // - Suppress initial diffs for this col (but only this col)
116 // - Update the URL
116117
117 var tables = getTables('results');118 var tables = getTables('results');
118 var newIndex = -1;119 var newIndex = -1;
@@ -165,6 +166,7 @@
165 tgt.tableName = 'results';166 tgt.tableName = 'results';
166 generateCustomEvent('NewCol', tgt);167 generateCustomEvent('NewCol', tgt);
167168
169 updateURL();
168 refreshNow();170 refreshNow();
169 }171 }
170}172}
@@ -194,6 +196,8 @@
194 for (var i = 0; i < table.rows.length; i++) {196 for (var i = 0; i < table.rows.length; i++) {
195 table.rows[i].cells[colIndex].style.display = display;197 table.rows[i].cells[colIndex].style.display = display;
196 }198 }
199
200 updateURL();
197}201}
198202
199function tiqitColMoveUp(event) {203function tiqitColMoveUp(event) {
200204
=== modified file 'static/scripts/results.js' (properties changed: -x to +x)
--- static/scripts/results.js 2014-06-05 14:38:53 +0000
+++ static/scripts/results.js 2015-11-30 09:54:32 +0000
@@ -176,3 +176,34 @@
176176
177 document.location.href = 'search?buglist=' + bugs.join(',');177 document.location.href = 'search?buglist=' + bugs.join(',');
178}178}
179
180//
181// Update URL (if columns on display are changed by the user for instance)
182//
183
184function updateURL() {
185 var local_table = getTables('results')[0];
186 var sel_count = 1;
187 var selections = tiqitSearchQuery.match(/(selection\d+=[\w\s-]+)/g);
188 var split_url = tiqitSearchQuery.split("&");
189
190 for (var k in selections) {
191 var temp_idx = split_url.indexOf(selections[k]);
192 split_url.splice(temp_idx, 1);
193 }
194
195 for (var k = 0; k < local_table.tHead.rows[0].cells.length; k++) {
196 var head = local_table.tHead.rows[0].cells[k];
197 var field = head.getAttribute('field');
198
199 // Don't want to display hidden fields.
200 if (field && (head.style.display != "none")) {
201 var temp_selection = "selection" + sel_count + "=" + field;
202 split_url.push(temp_selection);
203 sel_count++;
204 }
205 }
206 var new_url = document.baseURI + "results?" + split_url.join("&");
207
208 history.replaceState(null, null, new_url);
209}

Subscribers

People subscribed via source and target branches