Code review comment for lp:~frankban/juju-quickstart/improve-field-choices

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

On 2014/01/07 13:50:48, gary.poster wrote:
> - It looks like Urwid can support an "underline" setting. I think
that would
> make it easier to see the clickable help text (for both autogeneration
and these
> choices). WDYT?

Sounds good, done.

> - In my default terminal width (80x24) I see "possible values are:
precise,
> quantal, raring, saucy". If I use the arrow keys then I can see there
is a
> final option (", but this field can also be _left empty_"). With the
mouse I am
> out of luck. I'd like to improve that if we could. Options include
letting the
> line wrap, if Urwid supports that; showing a clickable "MORE->" or
"..." or
> something; or simply trying to reduce the length of the clickable
text.

> In line with the last, simplest idea, what about replacing the four
lines with
> something like the following? I'm assuming we can make the first text
wrappable
> easily, but if not maybe this idea needs tweaking.

> To select a value, type above, _clear the field_ to get the default
value
> (precise), or click one of the following choices:
> _precise_, _quantal_, _raring_, _saucy_, _trusty_

> Mm. Darn. I see in the region field that trying to reduce the length
of the
> clickable text is insufficient. Those options go on for days. That
will need
> line wrap or something else similar

Fixed using a GridFlow in place of the Columns widget (which does not
support automatic line wrapping). This way the choices adapt better to
the screen size.

> - I see that we actually explicitly set precise in the file if it is
empty (as
> you describe in the cover letter). However, IIUC, Juju will work fine
if you do
> not specify a series. The current *Juju* default behavior is to
default to
> precise, but probably will change to defaulting to trusty eventually.
That
> makes me think for our opinionated usage that we should begin
rendering the
> field with "precise" but if someone explicitly deletes the field then
it is
> simply empty (and the "empty" semantics are described in the fields
help text).
> WDYT?

I think the current behavior is wrong and what you suggested is better.
I was wrong thinking the default series (if unset) is the host series.
Fixed, now we pre-fill precise when creating a new environment.

Thank you!

https://codereview.appspot.com/34630044/

« Back to merge proposal