Merge lp:~abentley/launchpad/display-cleanup into lp:launchpad
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Deryck Hodge | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 14216 | ||||
Proposed branch: | lp:~abentley/launchpad/display-cleanup | ||||
Merge into: | lp:launchpad | ||||
Prerequisite: | lp:~abentley/launchpad/navigate-batches | ||||
Diff against target: |
353 lines (+186/-26) 4 files modified
lib/lp/bugs/browser/bugtask.py (+1/-0) lib/lp/bugs/browser/tests/test_bugtask.py (+1/-0) lib/lp/bugs/javascript/buglisting.js (+55/-7) lib/lp/bugs/javascript/tests/test_buglisting.js (+129/-19) |
||||
To merge this branch: | bzr merge lp:~abentley/launchpad/display-cleanup | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Abel Deuring (community) | Approve | ||
Deryck Hodge | Pending | ||
Review via email: mp+80621@code.launchpad.net |
Commit message
[r=adeuring][bug=882795] Polish ajax bug listing navigation.
Description of the change
= Summary =
Fix bug #882795: ajax bug listing navigation needs polish
== Proposed fix ==
Replace all navigation nodes with hyperlink nodes.
Mark first/previous disabled on first batch.
Mark last/next disabled on last batch.
Update batch info (1 -> 5 of 10) on navigation.
== Pre-implementation notes ==
None
== Implementation details ==
None
== Tests ==
bin/test -t test_buglisting
== Demo and Q/A ==
- Go to a listing page. first & previous should be disabled.
- Click last. next & last should be disabled, and first & previous should be green. The info about the batch should be updated.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
(16:40:10) adeuring: abentley: I think "this.current_ batch.mustache_ model.bugtasks. length + 1" in line 113 is wrong. Shouldn't that be "... -1"?
(16:40:35) adeuring: (line 113 of the diff)
(16:41:09) adeuring: abentley: no, that should be "+0"...
(16:41:13) abentley: adeuring: So, the +1 is trying to compensate for start being 0-indexed.
(16:41:47) adeuring: abentley: right. But for 5 results and start = 0, we want to show "results 1-> 5"
(16:41:56) adeuring: while we get "1 to 6"
(16:42:08) adeuring: abentley: just click on any of the links ;)
(16:42:15) abentley: Hmm, so why does it look right in use?
(16:42:36) adeuring: and compare the number in "n->m of k" with the real number of results ;)
(16:42:49) abentley: adeuring: that's an estimate, though.
(16:43:26) adeuring: abentley: well, k is an estimate (at least for StormRangeFactory), but we known the number of results in the batch precisely
(16:44:07) abentley: adeuring: You're right, I'll fix it.
(16:44:15) adeuring: abentley: cool, thanks
(16:46:07) adeuring: abentley: another quirk: For the first batch, the "first" and "previous" links are gray, but still active. "unlinking" them is something for antother branch, I assume?
(16:46:30) adeuring: (same for the last batch and the last/next links)
(16:47:39) abentley: adeuring: I would prefer if the CSS was fixed so that inactive links don't show as links.
(16:48:33) adeuring: abentley: right, makes sense. So, r=me, and file a bug about the links?
(16:49:02) abentley: adeuring: Thanks. I'll file a bug.