GNOME Bugzilla – Bug 731540
Make it easier to perform new searches without using the mouse
Last modified: 2014-10-19 13:12:21 UTC
It would be nice to not have to use the mouse to perform a new search when you have interacted with the map a bit. One possibility is to do it like in the content apps and just search on type. Another (arguably easier to get working right) is to do like in Web and Firefox and bind Ctrl+L to move to the search field.
Or instead of Ctrl+L you could do Ctrl+F f or find. Not sure there.
Well the reason we made the map grab focus was because it made it impossible for tablets to dismiss the on-screen-keyboard. So search on type might be out. A shortcut might be fine.
Created attachment 287517 [details] [review] Added ctrl+f accelerator.
Review of attachment 287517 [details] [review]: Thanks for the patch! I am not a fully experienced Gtk+ developer (I didn't suffered much of the Gtk+ 2 epoch :) ), so maybe I'm wrong. I think a most modern approach is by creating a new action in the main window that focus on the search entry when it's activated and then bind an accelerator by using gtk_application_add_accelerator.
Review of attachment 287517 [details] [review]: Hi thanks for patch! Yes, I agree with Damián. Almost. This is what the docs say: "gtk_application_add_accelerator has been deprecated since version 3.14 and should not be used in newly-written code. Use gtk_application_set_accels_for_action() instead" So yes, I think we should use gtk_application_set_accels_for_action() Where the detailed_action_name shoud probably be: 'win.find' and the accels: '<Primary>f' ? Or '<Control>f' ? You can check the gedit source for reference. Then you need to add the 'find' action to mainWindow.js with an appropriate signal handler. Check the syntax for the other actions in _initActions. Jonas
Or maybe it should be called win.search and action name search.
Created attachment 287573 [details] [review] Added _initAccelerator() so that we can have the other accelerator in the same method.
Created attachment 287574 [details] [review] Added Ctrl+F accelerator.
Review of attachment 287573 [details] [review]: Thanks this looks great! One small nit below, but that can be fixed up in merge. ::: src/mainWindow.js @@ +164,3 @@ + signalHandlers: { + activate: this.mapView.view.zoom_out.bind(this.mapView.view) + } There is an extra space between activate: and the callack.
Review of attachment 287574 [details] [review]: Thanks, looks great!
Will be commited after we branch off gnome-3-14, we are still collecting pure bugfixes for the 3.14.1 release.
Great work Ankita! I think it would make sense to move the calls to set_accels_for_action into Utils.initActions though. That way you would write code like: { properties: { name: 'find' }, signalHandlers: { activate: this._placeEntry.grab_focus.bind(this._placeEntry) }, accels: ['<Primary>F'] } And have all that code collected in one place. Jonas what do you think?
(In reply to comment #12) > Great work Ankita! > > I think it would make sense to move the calls to set_accels_for_action into > Utils.initActions though. > > That way you would write code like: > > { > properties: { name: 'find' }, > signalHandlers: { > activate: this._placeEntry.grab_focus.bind(this._placeEntry) > }, > accels: ['<Primary>F'] > } > > And have all that code collected in one place. > > Jonas what do you think? Good idea! That works for me! Ankita, mind doing the change?
I pushed these patches. I noticed that we are already using deprecated methods in our Utils actions code. Let's take a closer look at how we are supposed to do actions later, and maybe we can couple the accelerators better then.