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

On 2014/01/03 11:42:10, frankban wrote:
> On 2014/01/02 14:24:57, gary.poster wrote:
> > QA comments, pre-review:
> >
> > - Looking very cool.
> > - I like the fact that I can right click on links to launch them.
That may
> well
> > be a terminal feature rather than a quickstart or urwid feature, but
it all
> fits
> > together nicely.

> I agree, that's cool.

> > - Trivial, but I continue to be dissatisfied with the footer
formatting. I
> > don't like the fact that the text in the legend line wraps. For
instance,
> when
> > creating, I want "field errors" to be on the same line as the dot it
> describes.
> > Possible resolutions include the following: (a) we don't show
"juju-quickstart
> > 0.5.0" anywhere; (b) we show it in the right of the header; or (c)
we show it
> in
> > its own line in the footer.

> Removed the quickstart brand and put the notifications widget in its
own line
> (above the footer). This way I see the status in a single line even
when
> the terminal size is 80x24.

Much better, thanks!

> > - It would be nice if you could tab between fields. I assume this
is an Urwid
> > limitation, in which case never mind. Maybe we could clarify in the
footer
> that
> > arrow keys move between fields?

> I'd also like tab navigation to be implemented as part of the UX
improvement
> card(s). I think it should not be too difficult to implement that in
Urwid.
> In the meanwhile, added navigation help to the status as you
suggested.

Yes, looks good.

> > - It was not clear to me that clicking on "automatically generate"
would
> > generate the admin secret. Could we maybe change that text to
"Click here to
> > automatically generate this value"?

> Good idea, done.

> > - For the default series, you say that you may leave the series
empty. It
> would
> > be nice if you explained the semantics of that in the help text. Am
I right
> > that the semantics are that the series of the host computer is used?
> > - Should we default the default series value to precise?

> You are right. While we usually let juju itself handle the default
values
> where not set, for the default series quickstart should force an
explicit
> "precise" when the value is not specified. This means the default
series
> field must overwrite the normalize method, and that should be easy
enough.
> I'll do this in another card, perhaps the same as the one below
(choices).

Cool

> > - Maybe a bad idea: we could make the "precise," "quantal," and
other default
> > series text clickable. When you click on them, they fill in the
field with
> the
> > given value. Same could be done with ec2 region.
> > - Maybe a better idea: Does Urwid support the idea of a combo box,
somehow?
> > Would be nice. :-)

> AFAIK Urwid does not have a combo widget out of the box, but we can
try to
> implement it on top of the other base widgets. The first solution
sounds good
> to me anyway, and as you noticed I already have an XXX for it, and I
will
> create a card as well.

Great. I think the links will be fine (instead of trying to implement a
combobox) as long as our text clearly indicates the functionality (e.g.
"Click one to autofill the field with these standard options: precise,
quantal, raring, ...")

> > - I wish the two "create" options in the main menu looked more
different. Any
> > bright UX ideas? :-) Right now the whitespace separating the
environments
> from
> > the creation options don't clarify their difference sufficiently for
my eye.

> Updated the index view so that the environment creation options are
more
> separated. Now there is a text indicating how to use the creation
links.
> Does it look better?

Yes, much, thanks.

> > - Relatedly, the header instructions in the main menu are currently
"Select
> the
> > Juju environment you want to use." That no longer seems to describe
reality.
> > "Select the Juju environment you want to use, edit, view, or
remove." That's
> > wordy and leaves out the creation options. "Select an existing Juju
> environment
> > or create a new one." Maybe that's good enough? The user can see
what to do
> > with the environment once they select it.

> Done.

Cool.

> > - I know I suggested linking to
https://juju.ubuntu.com/docs/config-aws.html
> for
> > help, but it is confusing since the first half of the page is not
pertinent to
> > quickstart. The good stuff begins with "You can retrieve these
values easily
> > from your AWS Management Console at...." My ideas so far include
the
> following:
> > (a) confer with Nick to see if he has any ideas on whether we can
add an
> anchor
> > in the page to the part we want, or if we can have a separate page
with just
> the
> > part we want; (b) include the information for users actually in the
help text
> > for access key ("You can retrieve these values easily from your AWS
Management
> > Console at http://console.aws.amazon.com. Click on your name in the
top-right
> > and then the "Security Credentials" link from the drop down menu.
Under the
> > "Access Keys" heading click the "Create New Root Key" button. You
will be
> > prompted to "Download Key File" which by default is named
rootkey.csv. Open
> this
> > file to get the access-key and secret-key for the environments.yaml
> > configuration file.") and then have the secret key help point to the
access
> key
> > help.

> Done b).

Cool.

> Thank you for this great QA!

Thank you! Looking great.

Gary

https://codereview.appspot.com/44750044/