GNOME Bugzilla – Bug 670227
make modal dialog button less vertically packed to content
Last modified: 2012-03-08 19:04:09 UTC
Created attachment 207786 [details] [review] space buttons in modal dialogs All the modal dialogs have minimal whitespace separating the text & controls with the action buttons at the bottom.
I'm not sure we have consistent padding between all our dialogs now... what dialog or dialogs were you testing with?
I came up with the above gap when testing with pkexec. I think having a minimum shitespace like this is a better approach than mandating the dialog author to space up the content area.
ModalDialog uses in the shell: js/gdm/loginDialog.js: Extends: ModalDialog.ModalDialog, js/ui/endSessionDialog.js: Extends: ModalDialog.ModalDialog, js/ui/extensionSystem.js: Extends: ModalDialog.ModalDialog, js/ui/networkAgent.js: Extends: ModalDialog.ModalDialog, js/ui/polkitAuthenticationAgent.js: Extends: ModalDialog.ModalDialog, js/ui/runDialog.js: Extends: ModalDialog.ModalDialog, js/ui/shellMountOperation.js: Extends: ModalDialog.ModalDialog, js/ui/shellMountOperation.js: Extends: ModalDialog.ModalDialog, So basically the plan of attack for this review would be to apply the patch, take screenshots of all the dialogs (loginDialog is the annoying one), see if they have the right space, if not adjust, iterate. (There's a bug open about the extension installation dialog missing space, unless that's been fixed)
Created attachment 208625 [details] [review] theme patch I've got a patch here that was reviewed as a part of 668209. It fixes the issue here as well as making a few other tweaks to bring the theme closer the mockups. The only dialog it hasn't been tested against is the gdm login dialog - I'm not sure how to test that from jhbuild.
Created attachment 208670 [details] [review] theme patch My previous patch contained a bunch of cleanup changes. I've moved those out - the new version is just the styling updates. I've looked into testing this against the gdm dialog but haven't managed to find a way to do it. It seems that it is non-trivial to test this using jhbuild.
Allan, you can test these changes with a Fedora nightly CD. Make your changes in /usr/share/gnome-shell directly from the liveuser session and then log out to see them in action.
(In reply to comment #6) > Allan, you can test these changes with a Fedora nightly CD. Make your changes > in /usr/share/gnome-shell directly from the liveuser session and then log out > to see them in action. Good idea, Evandro. It took a little bit of doing, but I've managed to test the patch against Fedora nightly. The gdm dialogs look fine with the patch.
Patch got OK'ed on IRC, Allan asked me to push the patch for him ...
This broke the UI Freeze.
<aday> owen, is it ok to push the patch on bug 670227? <bebot> Bug http://bugzilla.gnome.org/show_bug.cgi?id=670227 trivial, Normal, ---, tiagomatos, UNCONFIRMED, make modal dialog button less vertically packed to content <owen> aday: your patch replaces jimmac's? <aday> owen, yes <owen> if you've tested it against all dialogs, I think it's good to push. A mail to the docs list telling them that dialog spacings have been adjusted a bit might be appreciated <aday> owen, sure thing
My reasoning was that of the two options: A) Dialogs in both doc screenshots and reality look crappy in spacing B) Dialogs in doc screenshots look crappy in spacing, reality looks better B) clearly is better, so notification was sufficient. Its not like any user is going to be confused because the dialog has different spacing in doc screenshots than in reality.