juju-quickstart

Merge lp:~frankban/juju-quickstart/tab-navigation into lp:juju-quickstart

  1. tab-navigation
  2. Merge into trunk
Proposed by Francesco Banconi
Status: Merged
Merged at revision: 42
Proposed branch: lp:~frankban/juju-quickstart/tab-navigation
Merge into: lp:juju-quickstart
Diff against target: 222 lines (+69/-17)
5 files modified
quickstart/cli/ui.py (+11/-0)
quickstart/cli/views.py (+10/-8)
quickstart/tests/cli/helpers.py (+7/-0)
quickstart/tests/cli/test_ui.py (+25/-0)
quickstart/tests/cli/test_views.py (+16/-9)
To merge this branch: bzr merge lp:~frankban/juju-quickstart/tab-navigation
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+200403@code.launchpad.net

Description of the change

Enable tab navigation in forms.

Implemented a TabNavigationListBox
that can be used when tab navigation is
required.

Also fixed views' status messages.

Tests: `make check`.

QA: start the demo app
(`make` and `./cli-app-demo.py).
Use it to edit existing environments and
to create new ones (ec2 and local).
Check that the status messages make sense.
In the creation/edit forms you should
be able to navigate through the form fields
using tab and shift+tab.

https://codereview.appspot.com/47350044/

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

LGTM and QA is good, though I was surprised tab doesn't work between
save/cancel/restore, though you say it is a known limitation.

https://codereview.appspot.com/47350044/

Revision history for this message
Francesco Banconi (frankban) wrote :

*** Submitted:

Enable tab navigation in forms.

Implemented a TabNavigationListBox
that can be used when tab navigation is
required.

Also fixed views' status messages.

Tests: `make check`.

QA: start the demo app
(`make` and `./cli-app-demo.py).
Use it to edit existing environments and
to create new ones (ec2 and local).
Check that the status messages make sense.
In the creation/edit forms you should
be able to navigate through the form fields
using tab and shift+tab.

R=bac
CC=
https://codereview.appspot.com/47350044

https://codereview.appspot.com/47350044/

Revision history for this message
Francesco Banconi (frankban) wrote :

Thank you!

https://codereview.appspot.com/47350044/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'quickstart/cli/ui.py'
2--- quickstart/cli/ui.py 2013-12-20 15:50:29 +0000
3+++ quickstart/cli/ui.py 2014-01-03 14:21:56 +0000
4@@ -170,6 +170,17 @@
5 return dismiss
6
7
8+class TabNavigationListBox(urwid.ListBox):
9+ """A ListBox supporting tab navigation."""
10+
11+ key_conversion_map = {'tab': 'down', 'shift tab': 'up'}
12+
13+ def keypress(self, size, key):
14+ """Override to convert tabs to up/down keys."""
15+ key = self.key_conversion_map.get(key, key)
16+ return super(TabNavigationListBox, self).keypress(size, key)
17+
18+
19 def thunk(function, *args, **kwargs):
20 """Create and return a callable binding the given method and args/kwargs.
21
22
23=== modified file 'quickstart/cli/views.py'
24--- quickstart/cli/views.py 2014-01-03 11:38:43 +0000
25+++ quickstart/cli/views.py 2014-01-03 14:21:56 +0000
26@@ -160,7 +160,6 @@
27 else:
28 title = 'No Juju environments already set up: please create one'
29 app.set_title(title)
30- app.set_status('')
31 # Start creating the page contents: a list of selectable environments.
32 widgets = []
33 focus_position = None
34@@ -197,13 +196,12 @@
35 for env_type in envs.get_supported_env_types(env_type_db)
36 ])
37 # Set up the application status messages.
38- status = []
39+ status = [' \N{UPWARDS ARROW LEFTWARDS OF DOWNWARDS ARROW} navigate ']
40 if default_found:
41 status.append(' \N{CHECK MARK} default ')
42 if errors_found:
43 status.extend([('error status', ' \N{BULLET}'), ' has errors '])
44- if status:
45- app.set_status(status)
46+ app.set_status(status)
47 # Set up the application contents.
48 contents = urwid.ListBox(urwid.SimpleFocusListWalker(widgets))
49 if focus_position is not None:
50@@ -269,7 +267,6 @@
51
52 env_metadata = envs.get_env_metadata(env_type_db, env_data)
53 app.set_title(envs.get_env_short_description(env_data))
54- app.set_status('')
55 # Validate the environment.
56 errors = envs.validate(env_metadata, env_data)
57 widgets = []
58@@ -282,14 +279,16 @@
59 text = [label, ('highlight', field.display(value))]
60 widgets.append(urwid.Text(text))
61 controls = [ui.MenuButton('back', ui.thunk(index_view))]
62+ status = [' \N{RIGHTWARDS ARROW OVER LEFTWARDS ARROW} navigate ']
63 if errors:
64- app.set_status([
65+ status.extend([
66 ('error status', ' \N{LOWER SEVEN EIGHTHS BLOCK}'),
67 ' field error ',
68 ])
69 else:
70 # Without errors, it is possible to use/start this environment.
71 controls.append(ui.MenuButton('use', ui.thunk(use, env_data)))
72+ app.set_status(status)
73 if not env_data['is-default']:
74 controls.append(
75 ui.MenuButton('set default', ui.thunk(set_default, env_data)))
76@@ -344,7 +343,9 @@
77 initial_errors = {}
78 app.set_title(title.format(env_data['type']))
79 app.set_status([
80- ' \N{UPWARDS ARROW LEFTWARDS OF DOWNWARDS ARROW} navigate ',
81+ ' \N{UPWARDS ARROW LEFTWARDS OF DOWNWARDS ARROW}'
82+ ' \N{LEFTWARDS ARROW TO BAR OVER RIGHTWARDS ARROW TO BAR}'
83+ ' navigate ',
84 ('optional status', ' \N{LOWER SEVEN EIGHTHS BLOCK}'),
85 ' optional field ',
86 ('error status', ' \N{BULLET}'),
87@@ -399,7 +400,8 @@
88 ('restore', ui.thunk(render_form, env_data, initial_errors)),
89 )
90 widgets.append(forms.create_actions(actions))
91- contents = urwid.ListBox(urwid.SimpleFocusListWalker(widgets))
92+ contents = ui.TabNavigationListBox(
93+ urwid.SimpleFocusListWalker(widgets))
94 app.set_contents(contents)
95
96 # Render the initial form.
97
98=== modified file 'quickstart/tests/cli/helpers.py'
99--- quickstart/tests/cli/helpers.py 2013-12-20 10:30:37 +0000
100+++ quickstart/tests/cli/helpers.py 2014-01-03 14:21:56 +0000
101@@ -64,3 +64,10 @@
102 columns = controls.contents[1][0].base_widget
103 buttons = [content[0].base_widget for content in columns.contents]
104 return title_widget, message_widget, buttons
105+
106+
107+def keypress(widget, key):
108+ """Simulate a key press on the given widget."""
109+ # Pass to the keypress method an arbitrary size.
110+ size = (42, 42)
111+ widget.keypress(size, key)
112
113=== modified file 'quickstart/tests/cli/test_ui.py'
114--- quickstart/tests/cli/test_ui.py 2013-12-20 10:30:37 +0000
115+++ quickstart/tests/cli/test_ui.py 2014-01-03 14:21:56 +0000
116@@ -165,6 +165,31 @@
117 self.assertEqual('push me', caption)
118
119
120+class TestTabNavigationListBox(unittest.TestCase):
121+
122+ def setUp(self):
123+ # Set up a TabNavigationListBox.
124+ self.widgets = [urwid.Edit(), urwid.Edit()]
125+ self.listbox = ui.TabNavigationListBox(
126+ urwid.SimpleFocusListWalker(self.widgets))
127+
128+ def test_widgets(self):
129+ # The listbox includes the provided widgets.
130+ self.assertEqual(self.widgets, list(self.listbox.body))
131+
132+ def test_tab_navigation(self):
133+ # The next widget is selected when tab is pressed.
134+ self.assertEqual(0, self.listbox.focus_position)
135+ cli_helpers.keypress(self.listbox, 'tab')
136+ self.assertEqual(1, self.listbox.focus_position)
137+
138+ def test_shift_tab_navigation(self):
139+ # The previous widget is selected when shift+tab is pressed.
140+ self.listbox.set_focus(1)
141+ cli_helpers.keypress(self.listbox, 'shift tab')
142+ self.assertEqual(0, self.listbox.focus_position)
143+
144+
145 class TestThunk(unittest.TestCase):
146
147 widget = 'test-widget'
148
149=== modified file 'quickstart/tests/cli/test_views.py'
150--- quickstart/tests/cli/test_views.py 2014-01-03 11:38:43 +0000
151+++ quickstart/tests/cli/test_views.py 2014-01-03 14:21:56 +0000
152@@ -123,6 +123,8 @@
153
154 class TestEnvIndex(EnvViewTestsMixin, unittest.TestCase):
155
156+ base_status = ' \N{UPWARDS ARROW LEFTWARDS OF DOWNWARDS ARROW} navigate '
157+
158 def test_view_default_return_value_on_exit(self):
159 # The view configures the app so that the return value on user exit is
160 # a tuple including a copy of the given env_db and None, the latter
161@@ -229,14 +231,14 @@
162 env_db = helpers.make_env_db()
163 views.env_index(self.app, self.env_type_db, env_db, self.save_callable)
164 status = self.app.get_status()
165- self.assertEqual(' \N{BULLET} has errors ', status)
166+ self.assertEqual(self.base_status + ' \N{BULLET} has errors ', status)
167
168 def test_status_with_default(self):
169 # The status message explains how default environment is represented.
170 env_db = helpers.make_env_db(default='lxc', exclude_invalid=True)
171 views.env_index(self.app, self.env_type_db, env_db, self.save_callable)
172 status = self.app.get_status()
173- self.assertEqual(' \N{CHECK MARK} default ', status)
174+ self.assertEqual(self.base_status + ' \N{CHECK MARK} default ', status)
175
176 def test_status_with_default_and_errors(self):
177 # The status message includes both default and errors explanations.
178@@ -244,18 +246,21 @@
179 views.env_index(self.app, self.env_type_db, env_db, self.save_callable)
180 status = self.app.get_status()
181 self.assertEqual(
182- ' \N{CHECK MARK} default \N{BULLET} has errors ', status)
183+ self.base_status +
184+ ' \N{CHECK MARK} default \N{BULLET} has errors ',
185+ status)
186
187- def test_empty_status(self):
188- # The status message is empty if there are no errors.
189+ def test_status(self):
190+ # The status only includes navigation info if there are no errors.
191 env_db = helpers.make_env_db(exclude_invalid=True)
192 views.env_index(self.app, self.env_type_db, env_db, self.save_callable)
193 status = self.app.get_status()
194- self.assertEqual('', status)
195+ self.assertEqual(self.base_status, status)
196
197
198 class TestEnvDetail(EnvViewTestsMixin, unittest.TestCase):
199
200+ base_status = ' \N{RIGHTWARDS ARROW OVER LEFTWARDS ARROW} navigate '
201 env_db = helpers.make_env_db(default='lxc')
202
203 def call_view(self, env_name='lxc'):
204@@ -434,13 +439,15 @@
205 # The status message explains how field errors are displayed.
206 self.call_view(env_name='local-with-errors')
207 status = self.app.get_status()
208- self.assertEqual(' \N{LOWER SEVEN EIGHTHS BLOCK} field error ', status)
209+ self.assertEqual(
210+ self.base_status + ' \N{LOWER SEVEN EIGHTHS BLOCK} field error ',
211+ status)
212
213 def test_status_without_errors(self):
214- # The status message is empty if there are no errors.
215+ # The status only includes navigation info if there are no errors.
216 self.call_view(env_name='lxc')
217 status = self.app.get_status()
218- self.assertEqual('', status)
219+ self.assertEqual(self.base_status, status)
220
221
222 class TestEnvEdit(EnvViewTestsMixin, unittest.TestCase):

Subscribers

People subscribed via source and target branches