GNOME Bugzilla – Bug 687127
Run dialog is missing a close button
Last modified: 2012-12-10 19:17:21 UTC
It is possible that someone could accidentally trigger the run dialog by pressing Alt+F2. This could be rather confusing, since the dialog doesn't have a close button - there's no visible way to make it go away. Let's add a close button. :) https://dl.dropbox.com/u/5031519/gnome-shell/run-dialog-new.png
Note that this also requires that we use a standard text entry field in this dialog. The text entry field should receive keyboard focus by default.
(In reply to comment #1) > Note that this also requires that we use a standard text entry field in this > dialog. The text entry field should receive keyboard focus by default. That is already the case, it is just styled differently. Is that what you are referring to? You opened two bugs attaching two different mockups ...
(In reply to comment #2) > That is already the case, it is just styled differently. Is that what you are > referring to? OK, then it just needs to use the standard styling so you can see when the text entry is focused. > You opened two bugs attaching two different mockups ... I updated the same mockup for both bugs (one advantage of hosting them yourself).
Created attachment 227556 [details] [review] runDialog: Better match style of other modal dialogs Update the run dialog to - use a proper title - spot a close button - use the standard entry style
*** Bug 687125 has been marked as a duplicate of this bug. ***
Review of attachment 227556 [details] [review]: ::: data/theme/gnome-shell.css @@ +338,2 @@ .login-dialog-prompt-entry, .prompt-dialog-password-entry { Can we have a shared style class for these? ::: js/ui/runDialog.js @@ +239,3 @@ + this.setButtons([{ action: Lang.bind(this, this.close), + label: _("Close"), + key: Clutter.Escape }]); As discussed on IRC, it would be nice to have a "Run" button as well.
Created attachment 227567 [details] [review] runDialog: Better match style of other modal dialogs Add a "Run" button as well, use more concise style classes
Review of attachment 227567 [details] [review]: ::: js/ui/runDialog.js @@ +297,3 @@ + this.close(); + else { + if (!this.pushModal()) I know you copy-pasted it from somewhere else, but bad style. Fix this up before pushing, please.
Attachment 227567 [details] pushed as 0c807bd - runDialog: Better match style of other modal dialogs
Created attachment 227621 [details] [review] theme patch I've just tested this, and there are a few tiny issues with the text styles. See the attached patch - I've use the standard text styles for the entry field and tweaked the color and the padding of the heading. Also, I don't know why you've added a run button here. I don't think we should be aiming to make the run dialog friendlier. The only reason we're adding the close button is to provide an escape route in case someone accidentally triggers the dialog. Finally, it would be good to remove the icon from the error warnings we show here. The red symbol seems overkill for what is most likely a typo. I would use the same style that we have in the login screen - which is basically the same as we have now, except that it doesn't use an icon and the text is color: orange; .
(In reply to comment #10) > Also, I don't know why you've added a run button here. I don't think we should > be aiming to make the run dialog friendlier. The only reason we're adding the > close button is to provide an escape route in case someone accidentally > triggers the dialog. As discussed on IRC, it was said that we wanted to provide a way for someone to be sure that they can run a command if they accidentally focused away from the text entry.
(In reply to comment #11) > (In reply to comment #10) > > Also, I don't know why you've added a run button here. I don't think we should > > be aiming to make the run dialog friendlier. The only reason we're adding the > > close button is to provide an escape route in case someone accidentally > > triggers the dialog. > > As discussed on IRC, it was said that we wanted to provide a way for someone to > be sure that they can run a command if they accidentally focused away from the > text entry. Doesn't sound like a valid concern to me. It's a run command: you press return. This is analogous to a terminal or even chat: we don't have a "run" button in the terminal or a "send" button in chat notifications.
Created attachment 227639 [details] [review] runDialog: Add entry to focus chain Currently the entry takes the intial key focus, but is not actually part of the focus chain. Fix that, even though keynav does not work too well for the dialog anyway, due to the entry consuming tab for command completion.
Created attachment 227640 [details] [review] runDialog: Remove "Run" button again While not in the mockups, it was introduced during review of commit 0c807bddaf4da5 after discussion on IRC, but the designers disagree; remove it again.
Review of attachment 227639 [details] [review]: Uh, sure.
Review of attachment 227640 [details] [review]: Meh. It looks really strange to have an explicit close button and no "OK" action. I think I'd be happier if it was an X bubble instead, but whatever. What we *do* know is that lots of people use the run dialog right now simply because the overview is too slow.
(In reply to comment #14) > Created an attachment (id=227640) [details] [review] > runDialog: Remove "Run" button again > > While not in the mockups, it was introduced during review of commit > 0c807bddaf4da5 after discussion on IRC, but the designers disagree; > remove it again. Thanks Florian. Is it OK for me to push my theme patch?
Attachment 227639 [details] pushed as a607174 - runDialog: Add entry to focus chain Attachment 227640 [details] pushed as 4d51056 - runDialog: Remove "Run" button again (In reply to comment #17) > Thanks Florian. Is it OK for me to push my theme patch? The patch looks fine, so I took the freedom to just push it with the other two patches.
Allan has posted on his blog[1] about this change. As described in post comments I'm not the only one to find it weird to have Close button to run a command. My mind will refrain me to press Enter key to avoid to trigger this Close button (comment #16 makes sense to me if you really want a close button). BTW, I find the current dialog (GNOME 3.4) visually 'lighter'. [1] http://afaikblog.wordpress.com/2012/11/07/fantastic-progress-in-every-detail-matters/(In reply to comment #16)
*** Bug 689999 has been marked as a duplicate of this bug. ***