Merge lp:~elachuni/ubuntu-webcatalog/improve-pagination-widget into lp:ubuntu-webcatalog

Proposed by Anthony Lenton
Status: Merged
Approved by: Anthony Lenton
Approved revision: 43
Merged at revision: 42
Proposed branch: lp:~elachuni/ubuntu-webcatalog/improve-pagination-widget
Merge into: lp:ubuntu-webcatalog
Diff against target: 177 lines (+81/-16)
5 files modified
src/webcatalog/static/css/webcatalog.css (+11/-0)
src/webcatalog/templates/webcatalog/batch_navigation.html (+30/-7)
src/webcatalog/templatetags/webcatalog.py (+6/-0)
src/webcatalog/tests/test_templatetags.py (+26/-3)
src/webcatalog/tests/test_views.py (+8/-6)
To merge this branch: bzr merge lp:~elachuni/ubuntu-webcatalog/improve-pagination-widget
Reviewer Review Type Date Requested Status
Ricardo Kirkner (community) Approve
Review via email: mp+68439@code.launchpad.net

Commit message

Improvements for the pagination widget.

Description of the change

Overview
========
This branch improves the pagination widget by implementing the recommendations on
http://developer.yahoo.com/ypatterns/navigation/pagination/search.html

Details
=======
You can see samples of the rendered resulting widget on
http://people.canonical.com/~anthony/Selection_003.png
http://people.canonical.com/~anthony/Selection_004.png
http://people.canonical.com/~anthony/Selection_005.png

To post a comment you must log in.
43. By Anthony Lenton

Added visble_page_range testcase to tests that are actually run.

Revision history for this message
Ricardo Kirkner (ricardokirkner) wrote :

LGTM.

Don't like how the second screenshot displays '1 to 1 of 1' twice in a very short 'space', but that's work for a different mp probably.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/webcatalog/static/css/webcatalog.css'
2--- src/webcatalog/static/css/webcatalog.css 2011-07-04 14:47:05 +0000
3+++ src/webcatalog/static/css/webcatalog.css 2011-07-19 18:35:13 +0000
4@@ -246,3 +246,14 @@
5 }
6
7 /* End breadcrumbs style */
8+
9+.paginator p {
10+ text-align: center;
11+}
12+.paginator .arrow a {
13+ font-size: 16px;
14+}
15+
16+.paginator p, .paginator a {
17+ font-size: 14px;
18+}
19
20=== added file 'src/webcatalog/static/images/arrow_next.png'
21Binary files src/webcatalog/static/images/arrow_next.png 1970-01-01 00:00:00 +0000 and src/webcatalog/static/images/arrow_next.png 2011-07-19 18:35:13 +0000 differ
22=== added file 'src/webcatalog/static/images/arrow_previous.png'
23Binary files src/webcatalog/static/images/arrow_previous.png 1970-01-01 00:00:00 +0000 and src/webcatalog/static/images/arrow_previous.png 2011-07-19 18:35:13 +0000 differ
24=== modified file 'src/webcatalog/templates/webcatalog/batch_navigation.html'
25--- src/webcatalog/templates/webcatalog/batch_navigation.html 2011-06-27 13:00:56 +0000
26+++ src/webcatalog/templates/webcatalog/batch_navigation.html 2011-07-19 18:35:13 +0000
27@@ -1,11 +1,34 @@
28 {% load i18n %}
29- {% if page.has_previous %}
30- <a href="?{% if query %}q={{ query }}&{% endif %}page={{ page.previous_page_number }}">{% trans "prev" %}</a>
31- {% endif %}
32+{% load webcatalog %}
33+<div class="paginator">
34 {% blocktrans with start_index=page.start_index end_index=page.end_index count=page.paginator.count %}
35- {{ start_index }} to {{ end_index }} of {{ count }}
36+ <p>Displaying items {{ start_index }} to {{ end_index }} of {{ count }}</p>
37 {% endblocktrans %}
38+ {% if page.has_other_pages %}
39+ <p>
40+ {% if page.has_previous %}
41+ <span class="arrow">
42+ <a href="?{% if query %}q={{ query }}&{% endif %}page={{ page.previous_page_number }}">
43+ {% trans "Prev" %}</a>
44+ <a href="?{% if query %}q={{ query }}&{% endif %}page={{ page.previous_page_number }}">
45+ <img src="{{ STATIC_URL }}images/arrow_previous.png"/></a>
46+ </span>
47+ {% endif %}
48+ {% for i in page|visible_page_range %}
49+ {% ifequal i page.number %}
50+ {{i}}
51+ {% else %}
52+ <a href="?{% if query %}q={{ query }}&{% endif %}page={{ i }}">{{ i }}</a>
53+ {% endifequal %}
54+ {% endfor %}
55 {% if page.has_next %}
56- <a href="?{% if query %}q={{ query }}&{% endif %}page={{ page.next_page_number }}">{% trans "next" %}</a>
57- {% endif %}
58-
59+ <span class="arrow">
60+ <a href="?{% if query %}q={{ query }}&{% endif %}page={{ page.next_page_number }}">
61+ <img src="{{ STATIC_URL }}images/arrow_next.png"/></a>
62+ <a href="?{% if query %}q={{ query }}&{% endif %}page={{ page.next_page_number }}">
63+ {% trans "Next" %}</a>
64+ </span>
65+ {% endif %}
66+ </p>
67+ {% endif %}
68+</div>
69
70=== modified file 'src/webcatalog/templatetags/webcatalog.py'
71--- src/webcatalog/templatetags/webcatalog.py 2011-07-04 14:47:05 +0000
72+++ src/webcatalog/templatetags/webcatalog.py 2011-07-19 18:35:13 +0000
73@@ -171,3 +171,9 @@
74 if inside_li:
75 html += "</ul>"
76 return mark_safe(html)
77+
78+@register.filter
79+def visible_page_range(page):
80+ first = max(page.number - 5, 1)
81+ last = min(first + 10, page.paginator.num_pages + 1)
82+ return range(first, last)
83\ No newline at end of file
84
85=== modified file 'src/webcatalog/tests/test_templatetags.py'
86--- src/webcatalog/tests/test_templatetags.py 2011-07-04 20:17:45 +0000
87+++ src/webcatalog/tests/test_templatetags.py 2011-07-19 18:35:13 +0000
88@@ -26,17 +26,20 @@
89
90 from django.core.urlresolvers import reverse
91 from django.template import Context
92+from mock import Mock
93
94 from webcatalog.templatetags.webcatalog import (
95 htmlize_package_description,
96 install_options,
97+ visible_page_range,
98 )
99 from webcatalog.tests.factory import TestCaseWithFactory
100
101 __metaclass__ = type
102 __all__ = [
103 'InstallOptionsTestCase',
104- 'TestHtmlize',
105+ 'HtmlizePackageDescriptionTestCase',
106+ 'VisiblePageRangeTestCase',
107 ]
108
109
110@@ -234,7 +237,7 @@
111 * Extensible with plugins
112 """
113
114-class TestHtmlize(unittest.TestCase):
115+class HtmlizePackageDescriptionTestCase(unittest.TestCase):
116 def test_htmlize(self):
117 for descr in [d1, d2, d3]:
118 html_descr = htmlize_package_description(descr)
119@@ -263,4 +266,24 @@
120 html_descr = htmlize_package_description(descr)
121 li_count = html_descr.count("<li>")
122 self.assertEqual(3, li_count)
123-
124\ No newline at end of file
125+
126+class VisiblePageRangeTestCase(unittest.TestCase):
127+ def test_visible_page_range(self):
128+ cases = [
129+ ( 2, 1, [1, 2]),
130+ (20, 1, range(1, 11)),
131+ ( 3, 2, [1, 2, 3]),
132+ (20, 2, range(1, 11)),
133+ ( 4, 4, [1, 2, 3, 4]),
134+ (20, 8, range(3, 13)),
135+ ( 8, 8, range(3, 9)),
136+ (20, 18, range(13, 21)),
137+ (400, 400, range(395, 401)),
138+ ]
139+ for num_pages, current_page, expected in cases:
140+ page = Mock()
141+ page.number = current_page
142+ page.paginator.num_pages = num_pages
143+ result = visible_page_range(page)
144+ self.assertTrue(len(result) <= 10)
145+ self.assertEqual(expected, result)
146
147=== modified file 'src/webcatalog/tests/test_views.py'
148--- src/webcatalog/tests/test_views.py 2011-07-07 20:54:45 +0000
149+++ src/webcatalog/tests/test_views.py 2011-07-19 18:35:13 +0000
150@@ -313,9 +313,10 @@
151 self.assertEqual(3, page.paginator.count)
152 self.assertEqual(2, page.next_page_number())
153
154- # And we have two references to the 'next' link, one at the top
155- # and one at the bottom.
156- self.assertContains(response, '<a href="?q=fo&page=2"', 2)
157+ # And we have six references to the next page, three at the top
158+ # and three at the bottom, as you have the linked "2" number, plus the
159+ # arrow and text "Next" link.
160+ self.assertContains(response, '<a href="?q=fo&page=2"', 6)
161
162 def test_invalid_page_doesnt_error(self):
163 distro = self.factory.make_distroseries(code_name='lucid')
164@@ -396,9 +397,10 @@
165 self.assertEqual(3, page.paginator.count)
166 self.assertEqual(2, page.next_page_number())
167
168- # And we have two references to the 'next' link, one at the top
169- # and one at the bottom.
170- self.assertContains(response, '<a href="?page=2"', 2)
171+ # And we have six references to the next page, three at the top
172+ # and three at the bottom, as you have the linked "2" number, plus the
173+ # arrow and text "Next" link.
174+ self.assertContains(response, '<a href="?page=2"', 6)
175
176 def test_invalid_page_doesnt_error(self):
177 dept = self.factory.make_department('bar')

Subscribers

People subscribed via source and target branches