Code review comment for lp:~wallyworld/launchpad/request-build-popup

Revision history for this message
Ian Booth (wallyworld) wrote :

> The check and cross button look wrong. I do not know of any other "popups" or
> "overlays" that use them. I only see them used when editing inline content. I
> do not think they mean submit or cancel. I think users believe they mean done
> editing or cancel editing.

They are used on a few other popup forms, mainly in the bugs area. eg Mark a Bug Report as Duplicate etc

> I expect to see a button [Request Build] to submit the form. Overlay forms use
> a [Cancel] button, but I think that contradicts the _Cancel_ rule for page
> forms. Most popups (gosh, maybe all) place the cancel (x) icon in the top
> right corner. There is a rule (maybe in code) that when the (x) cancel icon is
> in the right corner the popup will be dismissed when the user clicks outside
> the popup. Overlays remain on visible when he user clicks outside. I think the
> unfocus rule will help us decide the buttons.
> If the popup is dismissed when it loses focus, place the (x) cancel icon in
> the right corner. If it should remain, we use a [Cancel] button. The submit
> button should be [Request Build].
> /me is very aware that the NE cancel icon contradicts his unity desktop
> conventions

There's 2 lazr widgets at play here - PrettyOverlay which provides the behaviour where a popup has the close button in the top right corner and is dismissed if the user clicks outside. Then there's FormOverlay which extends PrettyOverlay and wires in form submission behaviour and doesn't wire in the "dismiss when click outside" behaviour since it allows for a proper cancel button as such (ie as used by the bug forms mentioned earlier).

I've stuck with using FormOverlay since it really is a modal form that is required here. The user clicks Request Build and they see a spinner and when the submission has been processed, the form disappears. On the other hand, if there's an error processing the form stays and an error is shown. They can then choose to correct the error and try again or click cancel to dismiss the form. What I did do though is replace the check/cross buttons with standard "Request Builds" and "Cancel" buttons.

What do you think?

« Back to merge proposal