Code review comment for lp:~frankban/juju-quickstart/env-manage-selection-view

*** Submitted:

Environment list/selection view.

This branch implements the environment
selection Urwid view. It is possible to
start it running `make` and then
`./cli-app-demo.py`. This is still not
integrated in quickstart, but the demo
file demonstrates how to show the view
passing the required data structures
already implemented in the
quickstart.models.envs module.

To test the branch, run `make check`.

To QA the new functionality, run the
demo application as described above,
and play with the interface: any UX
suggestion is welcome, and will be part
of a card we already have
("envs management: ux improvements").

Other changes:

The Urwid application is no longer an
ObjectDict (removed from the code).
I decided to use a Python namedtuple
which seemed to me a better choice,
considering that views are not supposed
to modify the app data structure.
This way we have both immutability and
attribute access to the object.

Implemented a TimeoutText wrapper used to
wrap the message notification widget.
This wayt he timer is restarted when a
subsequent message is set and the previous
has not yet been removed.

R=rharding, gary.poster
CC=
https://codereview.appspot.com/43860044

https://codereview.appspot.com/43860044/diff/1/quickstart/cli/base.py
File quickstart/cli/base.py (right):

https://codereview.appspot.com/43860044/diff/1/quickstart/cli/base.py#newcode37
quickstart/cli/base.py:37: _Application = namedtuple(
On 2013/12/18 16:44:58, rharding wrote:
> why the _? Why call it 'App' but also call it _Application? In my
experience I'd
> just have App = namedtuple('App'...

Done.

https://codereview.appspot.com/43860044/diff/1/quickstart/cli/base.py#newcode152
quickstart/cli/base.py:152: # place where to add new API functions,
after changing the application
On 2013/12/18 16:44:58, rharding wrote:
> Create the App named tuple. If, in the future, we have a view that
requires
> additional capabilities or API access, this is the place to add those.

Done.

https://codereview.appspot.com/43860044/diff/1/quickstart/cli/ui.py
File quickstart/cli/ui.py (right):

https://codereview.appspot.com/43860044/diff/1/quickstart/cli/ui.py#newcode49
quickstart/cli/ui.py:49: None: 'selected',
On 2013/12/18 16:44:58, rharding wrote:
> I don't follow this. When None is in focus it's selected?

When a widget without a class is in focus, we apply the selected class.

https://codereview.appspot.com/43860044/diff/1/quickstart/cli/views.py
File quickstart/cli/views.py (right):

https://codereview.appspot.com/43860044/diff/1/quickstart/cli/views.py#newcode32
quickstart/cli/views.py:32: A view is a callable receiving an app object
(a named tuple of functions) and
On 2013/12/18 16:44:58, rharding wrote:
> Can we capitalize the App here and in cases referencing the specific
object?

Done.

https://codereview.appspot.com/43860044/diff/1/quickstart/cli/views.py#newcode180
quickstart/cli/views.py:180: return env_db, None
On 2013/12/18 16:57:15, gary.poster wrote:
> this is a decoy, right? No one can use these?

> I worry a bit that the functional style here is a mismatch with Urwid.
  These
> functions are all about side effects. However, on reflection, I feel
like it's
> easy to read what you have, and that increasing the functional feel is
> increasing the simplicity. We'll see how time and other people
instruct us, I
> guess. :-)

As discussed, I'll make this more explicit.

https://codereview.appspot.com/43860044/