GNOME Bugzilla – Bug 730829
Give extension-prefs some GNOME 3 treatment
Last modified: 2014-05-27 16:04:50 UTC
This mostly implements Jakub's mockup at http://jimmac.musichall.cz/stuff/extension-settings.png. I'm still a bit undecided on the last patch - rather than a command line option, it might make sense to just not show the main window when an initial UUID is specified. At least it looks like the right thing to do for both web and tweak tool ...
Created attachment 277299 [details] [review] extension-prefs: Give the UI a bit of GNOME 3 treatment The extension-prefs UI has never been great, but as the GNOME 3 design patterns are evolving, it is starting to look seriously outdated. Modernize the UI a bit to have it fit in a bit better.
Created attachment 277300 [details] [review] extensionPrefs: Add switches to enable/disable extensions Bring the extension-prefs tool in line with the mockup by adding switches to enable/disable extensions, similar to the extension page in gnome-tweak-tool.
Created attachment 277301 [details] [review] extensionPrefs: Add option to skip main window The Tweak Tool spawns the extension-prefs tool for extension preferences. However as it already implements its own extension list, the main window is not useful in that context (to not say it is rather silly). So add an option to skip the main window and just show the specified extension's preference dialog.
Review of attachment 277301 [details] [review]: The website does this as well with its buttons (see LaunchExtensionPrefs in shellDBus.js). Basically, if we specify an extension URI directly, we should probably do the same. Unless you want to just patch shellDBus.js as well.
Review of attachment 277300 [details] [review]: ::: js/extensionPrefs/main.js @@ +311,3 @@ + this._disable(); + })); + this._switch.connect('state-set', function() { return true; }); This seems all too confusing to me. Can't you simply connect to notify::state ?
Review of attachment 277299 [details] [review]: Not overly happy with the new design, but meh. ::: js/extensionPrefs/main.js @@ +268,3 @@ + let hbox = new Gtk.Box({ orientation: Gtk.Orientation.HORIZONTAL, + hexpand: true, margin: 12, spacing: 6 }); + this.add(hbox); It's times like these that I wish we had the icon on the client. Maybe you can play the same trick extensionDownloader.js does with a URI-based FileIcon. @@ +294,3 @@ + hbox.add(button); + + this.prefsButton = button; I wonder if we should add in a "show in web" button that takes you to the proper web page for the extension.
(In reply to comment #4) > Basically, if we specify an extension URI directly, we should probably do the > same. Yeah, see comment #0. I'll drop the option. (In reply to comment #5) > + this._switch.connect('state-set', function() { return true; }); > > This seems all too confusing to me. Can't you simply connect to notify::state ? This is the documented way to achieve delay state changes, e.g. the "active" property changes when the user clicks the switch, the "state" property changes when the underlying GSettings changed. It's not too useful from a UI perspective, but it does help code clarity a bit IMO ("active" modified due to user action, "state" modified due to GSettings changes)
Created attachment 277318 [details] [review] extensionPrefs: Skip main window when launched with a UUID Dropped the option in favor of just skipping the window when launched with a UUID.
Attachment 277299 [details] pushed as e1b30b2 - extension-prefs: Give the UI a bit of GNOME 3 treatment Attachment 277300 [details] pushed as 521f5f2 - extensionPrefs: Add switches to enable/disable extensions Attachment 277318 [details] pushed as 5b3fb02 - extensionPrefs: Skip main window when launched with a UUID