Code review comment for lp:~rockstar/launchpad/codereview-js

On Mon, 03 Aug 2009 18:41:09 -0000, Martin Albisetti <email address hidden>
wrote:
> - The escape key doesn't seem to work for me on this overlay. Any idea why?

So, this overlay doesn't seem to act like our other overlays, which leads me to
the next point...

> - The optional type field feels odd. I think it is because it's not clear
> what to click to accept, and since the person is above that field, then
> it's even weirder to have to click on top to save something on the bottom.
> My feeling is that this should have an accept button come up once you've
> clicked on a person, to confirm it, and give you time to fill-in the review
> type. What do you think?

So, I've also thought about this. I'd like to land it as-is, but I've spent
all weekend thinking about how it needs to change. The biggest thing, I think,
is that in all other overlays, there's an ok and a cancel, but in this one, on
click of a person, it automatically submits. This means we can't use the
person picker in other situations that also require context (like subscribing
a person to a branch). This branch's work still won't go in vain though,
because we've discovered this issue, and have been putting thought into how we
fix this.

The ultimate plan is to make the person picker itself it's own widget, and then
make the person picker overlay just use that new widget. Then I can have other
overlays that use that widget as well.