GNOME Bugzilla – Bug 709604
Click the map should grab (steal) focus away from the search text entry widget
Last modified: 2014-03-07 19:20:07 UTC
...otherwise, it is impossible to "dismiss" the on-screen keyboard if you're using a touch/tablet device.
Created attachment 270954 [details] [review] Bug patch Attached a patch that fix the bug.
Review of attachment 270954 [details] [review]: You want to add a more descriptive commit message. First line is the subject and should be ≤40 chars, just being a short description of what the commit does. Second line should be blank Third line and beyound should be ≤ 72 chars (I think?, I try to stop at 70 at least) and add any extra information. If you use git-bz a reference to the bug is automatically included. The code looks fine to me otherwise. Zeeshan, Jonas?
(In reply to comment #2) > Review of attachment 270954 [details] [review]: > > You want to add a more descriptive commit message. > > First line is the subject and should be ≤40 chars, just being a short > description of what the commit does. > Second line should be blank > Third line and beyound should be ≤ 72 chars (I think?, I try to stop at 70 at > least) and add any extra information. In future, just point to: https://wiki.gnome.org/Git/CommitMessages . One of the potential SoC students also added a checklist there, which should be helpful for new comers. > If you use git-bz a reference to the bug is automatically included. > > The code looks fine to me otherwise. Zeeshan, Jonas? I'll do a review..
Review of attachment 270954 [details] [review]: The changes look good to me too so if it works, we just need a nice commit log. :)
Review of attachment 270954 [details] [review]: It does not seem to apply on master for me, what revision is it generated against?
Review of attachment 270954 [details] [review]: Seems fine. ::: src/mainWindow.js @@ +121,1 @@ this.mapView.view.connect('button-press-event', What is this for? Why do we want to grab focus when we have selected an item to go to? Not saying it is wrong, just curious on the thinking.
My idea is that after you search something, the focus should go on the map. In the current release you can search, then move on the maps, and then continue typing... I didn't believe this was a right behaviour. Is it applying on your code now? Just yesterday I learned how to rebase and branch, now I could manage better this patch. If it's ok I'm going just to change the commit log, otherwise I'll make the changes. :-) (In reply to comment #6) > Review of attachment 270954 [details] [review]: > > Seems fine. > > ::: src/mainWindow.js > @@ +121,1 @@ > this.mapView.view.connect('button-press-event', > > What is this for? Why do we want to grab focus when we have selected an item to > go to? > Not saying it is wrong, just curious on the thinking.
(In reply to comment #7) > My idea is that after you search something, the focus should go on the map. > In the current release you can search, then move on the maps, and then continue > typing... I didn't believe this was a right behaviour. Yeah, I agree with you. > > Is it applying on your code now? Just yesterday I learned how to rebase and > branch, now I could manage better this patch. > > If it's ok I'm going just to change the commit log, otherwise I'll make the > changes. :-) > It's ok, change the commit log and try to make sure that the new patch applies cleanly on the master branch. Thanks! > > > (In reply to comment #6) > > Review of attachment 270954 [details] [review] [details]: > > > > Seems fine. > > > > ::: src/mainWindow.js > > @@ +121,1 @@ > > this.mapView.view.connect('button-press-event', > > > > What is this for? Why do we want to grab focus when we have selected an item to > > go to? > > Not saying it is wrong, just curious on the thinking.
Created attachment 270996 [details] [review] The map now grabs the focus Added the property "can-focus" to true for the GtkOverlay where the map is contained. The map take the focus, after clicking on it or after searching a place.
Created attachment 270998 [details] [review] The map now grabs the focus Added the property "can-focus" to true for the GtkOverlay where the map is contained. The map take the focus, after clicking on it or after searching a place.
Created attachment 271001 [details] [review] The map now grabs the focus Added the property "can-focus" to true for the GtkOverlay where the map is contained. The map take the focus, after clicking on it or after searching a place.
Review of attachment 271001 [details] [review]: Thanks, good work! I will commit this but I will also change the commit message a bit. To make it more in line with how they look in Maps. Take a log at the commit log to see how messages are formated.