GNOME Bugzilla – Bug 747983
Start-up AppChooser dailog improvements
Last modified: 2015-04-29 18:11:28 UTC
The AppChooser dialog has a number of rough edges: - It allows you to add the same application more than once - When filtering the list running applications are always hidden - The ‘Add Application’ button is active when there's no app selected - There's no mnemonic for ‘Add Application’ - There's no default response set - Pressing ‘Escape’ re-focuses the search-bar rather than closing it - It ignores ‘gtk-dialogs-use-header’ setting
Created attachment 301710 [details] [review] StartUpTweak: Use g_application_mark_busy() It can take a few seconds to create the AppChooser dialog so mark the application as busy to provide some visual feedback when the ‘+’ button is clicked.
Created attachment 301711 [details] [review] AppChooser: Include matching running apps when filtering Running apps have two labels so match against both of them rather than failing if the first doesn't match. This ensures that any apps that match and are running get included in the filtered list.
Created attachment 301712 [details] [review] AppChooser: Don't show existing start-up apps Pass a list of existing start-up applications to the AppChooser dialog so it can avoid showing them.
Created attachment 301713 [details] [review] AppChooser: Desensitize ‘Add’ button when no app is selected The ‘Add Application’ button shouldn't be sensitive if there is no application currently selected. This is complicated be the fact that when Gtk.Listbox filters the list it does not clear the selection if the selected row is filtered out so we need to watch for the selected row being umapped.
Created attachment 301714 [details] [review] AppChooser: Add a default response Adding a default response improves the keyboard navigation as the user can select an application by pressing ‘Enter’. It also changes the style of the ‘Add Application’ button.
Created attachment 301715 [details] [review] AppChooser: Select app when row is activated This allows the user to select a startup application by double clicking on it rather than having to select it and then click on the ‘Add Application’ button.
Created attachment 301716 [details] [review] AppChooser: Rationalize ‘Escape’ handling Pressing ‘Escape’ when the search-bar is visible should close it, if the search-bar is not visible it should close the dialog.
Created attachment 301717 [details] [review] AppChooser: Respect setting of gtk-dialogs-use-header Use a header-bar for the start-up application chooser dialog if ‘gtk-dialogs-use-header’ is True. Rename the ‘Add Application’ button to ‘Add’ and add a mnemonic for it as well as changing the response of the ‘Close’ button so that it is placed on the opposite side of the header-bar to the ‘Add’ button.
Review of attachment 301710 [details] [review]: ok
Review of attachment 301711 [details] [review]: fine
Review of attachment 301712 [details] [review]: ::: gtweak/tweaks/tweak_group_startup.py @@ +55,3 @@ apps = Gio.app_info_get_all() for a in apps: + for b in startup_apps: ugh, if we have a large number of apps here this might take a while @@ +252,3 @@ def _on_add_clicked(self, btn): Gio.Application.get_default().mark_busy() + startup_apps = [] can we make this a set instead? would avoid the issue above
Review of attachment 301715 [details] [review]: ++
Review of attachment 301713 [details] [review]: Don't like the complexity here. Isn't it enough to set the ListBox selection mode to GTK_SELECTION_BROWSE ? ::: gtweak/tweaks/tweak_group_startup.py @@ +56,3 @@ self.entry.connect("search-changed", lambda e: lb.invalidate_filter()) + lb.connect("row-activated", lambda b, r: self.response(Gtk.ResponseType.OK)) this is duplicated on a patch further ahead, should be removed here
Review of attachment 301714 [details] [review]: ++
Review of attachment 301716 [details] [review]: I agree that this feels better
Review of attachment 301717 [details] [review]: anyway, fine ::: gtweak/tweaks/tweak_group_startup.py @@ +34,3 @@ def __init__(self, main_window, running_exes, startup_apps): + uhb = Gtk.Settings.get_default().props.gtk_dialogs_use_header + Gtk.Dialog.__init__(self, title=_("Applications"), use_header_bar=uhb) GtkDialog doesn't do this by itself?
(In reply to Rui Matos from comment #16) > Review of attachment 301717 [details] [review] [review]: > > anyway, fine > > ::: gtweak/tweaks/tweak_group_startup.py > @@ +34,3 @@ > def __init__(self, main_window, running_exes, startup_apps): > + uhb = Gtk.Settings.get_default().props.gtk_dialogs_use_header > + Gtk.Dialog.__init__(self, title=_("Applications"), > use_header_bar=uhb) > > GtkDialog doesn't do this by itself? Unfortunately it doesn't. Internally it uses gtk_dialog_set_use_header_bar_from_setting() for the derived dialogs (colorchooser etc) provided by Gtk but it does not export it.
Created attachment 302431 [details] [review] AppChooser: Don't show existing start-up apps I've updated this to use a set as you suggested. This probably causes conflicts with some later patches so I'll push a branch wip/pwood/app-chooser with the current versions rather than obsoleting the patches you've already approved. Pass a set containing the existing start-up applications to the AppChooser dialog so it can avoid showing them.
Created attachment 302433 [details] [review] AppChooser: Desensitize ‘Add’ button when no app is selected In reply to comment 13 > Review of attachment 301713 [details] [review] [review]: > > Don't like the complexity here. Isn't it enough to set the ListBox > selection mode to GTK_SELECTION_BROWSE ? No, for some reason Gtk refuses to deselect a row when it's hidden by the filter even though it looks to the user like there are no rows selected (I wondered if it was due to the semantics of GTK_SELECTION_BROWSE preventing the selection from being cleared, but it is the same with GTK_SELECTION_SINGLE). I don't like it (I thought this would be a quick change but it took quite a while to work out what was going on) but this seems to be the only way to get it to work. I've added a couple of comments to the code about this. This patch now increases the required Gtk version as it needs 3.14 for GtkListBox::row-selected. >::: gtweak/tweaks/tweak_group_startup.py >@@ +56,3 @@ > self.entry.connect("search-changed", lambda e: lb.invalidate_filter()) > >+ lb.connect("row-activated", lambda b, r: self.response(Gtk.ResponseType.OK)) > >this is duplicated on a patch further ahead, should be removed here Well spotted, it was from a bad rebase, I've fixed it. The ‘Add Application’ button shouldn't be sensitive if there is no application currently selected. This is complicated be the fact that when Gtk.Listbox filters the list it does not clear the selection if the selected row is filtered out so we need to watch for the selected row being umapped.
Review of attachment 302431 [details] [review]: fine
Review of attachment 302433 [details] [review]: I'm pretty sure we can achieve the same result without all the signal juggling by 1) changing the search-changed handler to set_response_sensitive() if we have a selected row and it's mapped 2) set_response_sensitive() on the row-selected handler
Created attachment 302509 [details] [review] AppChooser: Desensitize ‘Add’ button when no app is selected I've updated and simplified this as you suggested. The ‘Add Application’ button shouldn't be sensitive if there is no application currently selected. This is complicated be the fact that when Gtk.Listbox filters the list it does not clear the selection so if there is a selected row we need to check if it is mapped before changing the state of the ‘Add Application’ button.
Created attachment 302510 [details] [review] AppChooser: Select app when row is activated I've updated this to make sure it doesn't add an application when all the applications are hidden by the filter. This allows the user to select a startup application by double clicking on it rather than having to select it and then click on the ‘Add Application’ button. If there are now rows visible then GtkListBox will activate the (hidden) selected row so we need to check that the activated row is mapped.
Created attachment 302511 [details] [review] AppChooser: Add a search button to header-bar This gives a visual prompt that it's possible to search the list of applications.
Created attachment 302513 [details] [review] AppChooser: Add an accelerator to toggle searching Use Control-f to toggle the state of the search-bar
Review of attachment 302511 [details] [review]: looks good ::: gtweak/tweaks/tweak_group_startup.py @@ +21,3 @@ import logging +from gi.repository import Gtk, Gdk, GLib, Gio, GObject The aditional import shouldn't be needed
Review of attachment 302513 [details] [review]: ++
Review of attachment 302510 [details] [review]: ++
Review of attachment 302509 [details] [review]: nice
(In reply to Rui Matos from comment #26) > Review of attachment 302511 [details] [review] [review]: > > looks good > > ::: gtweak/tweaks/tweak_group_startup.py > @@ +21,3 @@ > import logging > > +from gi.repository import Gtk, Gdk, GLib, Gio, GObject > > The aditional import shouldn't be needed I get this error if I take it out self._binding = searchbtn.bind_property("active", self.searchbar, "search-mode-enabled", GObject.BindingFlags.BIDIRECTIONAL) NameError: global name 'GObject' is not defined
Attachment 301710 [details] pushed as f78155a - StartUpTweak: Use g_application_mark_busy() Attachment 301711 [details] pushed as 7664aba - AppChooser: Include matching running apps when filtering Attachment 301714 [details] pushed as 82924af - AppChooser: Add a default response Attachment 301716 [details] pushed as 019eda9 - AppChooser: Rationalize ‘Escape’ handling Attachment 301717 [details] pushed as 96014af - AppChooser: Respect setting of gtk-dialogs-use-header Attachment 302431 [details] pushed as a4f5db2 - AppChooser: Don't show existing start-up apps Attachment 302509 [details] pushed as 6874ba8 - AppChooser: Desensitize ‘Add’ button when no app is selected Attachment 302510 [details] pushed as 1372b5f - AppChooser: Select app when row is activated Attachment 302511 [details] pushed as ebc6b70 - AppChooser: Add a search button to header-bar Attachment 302513 [details] pushed as c60d424 - AppChooser: Add an accelerator to toggle searching Are you happy for me to backport the following patches to the 3.16 branch? It would be nice to have the fixes that don't require a Gtk version bump or string changes in the stable branch. I can just cherry-pick and push or open a new bug if you want to check them first. f78155a StartUpTweak: Use g_application_mark_busy() 7664aba AppChooser: Include matching running apps when filtering a4f5db2 AppChooser: Don't show existing start-up apps 82924af AppChooser: Add a default response 019eda9 AppChooser: Rationalize ‘Escape’ handling
(In reply to Phillip Wood from comment #30) > I get this error if I take it out > self._binding = searchbtn.bind_property("active", self.searchbar, > "search-mode-enabled", GObject.BindingFlags.BIDIRECTIONAL) > NameError: global name 'GObject' is not defined Ah, nevermind, I totally overlooked that last argument. (In reply to Phillip Wood from comment #31) > Are you happy for me to backport the following patches to the 3.16 > branch? It would be nice to have the fixes that don't require a Gtk > version bump or string changes in the stable branch. I can just > cherry-pick and push or open a new bug if you want to check them first. > > f78155a StartUpTweak: Use g_application_mark_busy() > 7664aba AppChooser: Include matching running apps when filtering > a4f5db2 AppChooser: Don't show existing start-up apps > 82924af AppChooser: Add a default response > 019eda9 AppChooser: Rationalize ‘Escape’ handling Yes please, just push them, thanks
Review of attachment 302509 [details] [review]: ::: configure.ac @@ +10,2 @@ DESKTOP_SCHEMAS_REQUIRED_VERSION=3.4.0 +GTK_REQUIRED_VERSION=3.14.0 Wait, why are we bumping this? Looks like all the API used here exists in 3.12
(In reply to Phillip Wood from comment #31) > 6874ba8 - AppChooser: Desensitize ‘Add’ button when no app is selected > 1372b5f - AppChooser: Select app when row is activated I think these are ok for 3.16 too
Created attachment 302598 [details] [review] Revert increase in required Gtk version In reply to comment 33 > Review of attachment 302509 [details] [review] [review]: > > ::: configure.ac > @@ +10,2 @@ > DESKTOP_SCHEMAS_REQUIRED_VERSION=3.4.0 > +GTK_REQUIRED_VERSION=3.14.0 > > Wait, why are we bumping this? Looks like all the API used here > exists in 3.12 Sorry, I'd somehow got it into my head that we needed 3.14 for the ‘row-activated’ and ‘row-selected’ signals. 6874ba8 AppChooser: Desensitize ‘Add’ button when no app is selected erroneously increased the required Gtk version number.
Review of attachment 302598 [details] [review]: push it
Comment on attachment 302598 [details] [review] Revert increase in required Gtk version Attachment 302598 [details] pushed as 7e75c2a - Revert increase in required Gtk version
(In reply to Rui Matos from comment #32) > (In reply to Phillip Wood from comment #31) > > Are you happy for me to backport the following patches to the 3.16 > > branch? It would be nice to have the fixes that don't require a Gtk > > version bump or string changes in the stable branch. I can just > > cherry-pick and push or open a new bug if you want to check them first. > > > > f78155a StartUpTweak: Use g_application_mark_busy() > > 7664aba AppChooser: Include matching running apps when filtering > > a4f5db2 AppChooser: Don't show existing start-up apps > > 82924af AppChooser: Add a default response > > 019eda9 AppChooser: Rationalize ‘Escape’ handling > > Yes please, just push them, thanks > (In reply to Phillip Wood from comment #31) > > 6874ba8 - AppChooser: Desensitize ‘Add’ button when no app is selected > > 1372b5f - AppChooser: Select app when row is activated > > I think these are ok for 3.16 too I've removed the version bump and pushed them