*** Bug 757838 has been marked as a duplicate of this bug. ***

I'm interested in solving this bug, do you want the help overlay in the space where the about dialog box is?

(In reply to Karanbir Chahal from comment #2)
> I'm interested in solving this bug, do you want the help overlay in the
> space where the about dialog box is?

Hi,

No I want it implemented the way it is described in the links above. It should appear on the given keyboard shortcut that is already defined. Read all the links above and implement it in the same way as in the second link!

This is something that is added to GTK+ we need to implement a specific UI file in our resources and it should work. Read the links above.

thanks!

For more information about UI files, please check the data/ui directory of Maps and read here: https://developer.gnome.org/gtk3/stable/GtkBuilder.html

Alright thanks.I'm on it

Created attachment 317544 [details] [review]
Add a help overlay

GNOME has a new Goal: Adding shortcut windows.

GTK+ 3.19.x includes a new widget called GtkShortcutsWindow.
This is a dialog window that shows an overview of shortcuts
(keyboard shortcuts and touch gestures) for an application.

To add a help overlay we need to create an UI file in our
resources at $(gresource prefix)/gtk/.

And the rest should be magical.

Created attachment 317545 [details]
Screenshot of overlay window

Maybe we could add some more shortcuts as well?

Karan: How about adding keyboard short cut for goto user location?

Yeah thanks!! 
Great patch, I had no idea so many changes were required

Created attachment 317548 [details] [review]
Updated About Dialog Box with information about technologies used

You didn't commit the above patch hence I need to copy what you did and add the accel for user location. I hope you don't mind

Created attachment 317549 [details]
Screenshot

I would generally place quit last in its group - it makes sense logically.

(In reply to Karanbir Chahal from comment #11)
> You didn't commit the above patch hence I need to copy what you did and add
> the accel for user location. I hope you don't mind

I do mind a bit actually :)

I do not want these in the same patch. Please create a clean patch that _only_ adds the accel to goto-user-location.

Review of attachment 317548 [details] [review]:

To much going on here and you destroyed my pretty commit message.

(In reply to Jonas Danielsson from comment #14)
> (In reply to Karanbir Chahal from comment #11)
> > You didn't commit the above patch hence I need to copy what you did and add
> > the accel for user location. I hope you don't mind
> 
> I do mind a bit actually :)
> 
> I do not want these in the same patch. Please create a clean patch that
> _only_ adds the accel to goto-user-location.

You can apply my patch then do 1 commit that adds the accel and adds it to the short-cut. Treat my patch as already applied.

Created attachment 317600 [details] [review]
Updated patch for shortcut to go to user's location

Created attachment 317602 [details]
Screenshot of updated shortcut's window

I'm sorry about the mess,Is this alright?

(In reply to Karanbir Chahal from comment #19)
> I'm sorry about the mess,Is this alright?

No problem! Thank you for working on this!
Will comment some on the patch.

Review of attachment 317600 [details] [review]:

Thanks!


About the commit message, please see the guide here:
https://wiki.gnome.org/Git/CommitMessages

In this case I would think a head line like:

"Add keyboard shortcut for going to current location"

There is no need to mention what files have been altered in the commit message we will see that from the git patch itself and from the commit log.
And just paste the bugzilla link at the end, no need for a label.

::: data/org.gnome.Maps.desktop.in
@@ +1,1 @@
+

Unrelated change, please remove in next version.

::: data/ui/help-overlay.ui
@@ +34,3 @@
               <object class="GtkShortcutsShortcut">
                 <property name="visible">1</property>
+                <property name="title" translatable="yes">Go to User's Location</property>

I think "Go to current location" describes this better. Andreas / Allan, comments?

@@ +35,3 @@
                 <property name="visible">1</property>
+                <property name="title" translatable="yes">Go to User's Location</property>
+                <property name="accelerator">&lt;Primary&gt;L</property>

What is the reasoning behind using >primary>L here? L should be for location?

Andreas / Allan / Someone else: Comments?

(In reply to Allan Day from comment #13)
> I would generally place quit last in its group - it makes sense logically.

Thanks Allan! Other than that does it look ok? The groups atm is General and Map View. And the only stuff in Map View right now is zoom in/zoom out? What more could be in that group? Go to current location? Switch from street / aerial? Switch between upcomming custom layers?

Yeah L should mean location,we could also use something like U for user
It's up to you guys.
I'll update the patch with the changes you mentioned

Created attachment 317626 [details] [review]
Made required corrections to patch

Review of attachment 317626 [details] [review]:

Thanks!

Karanbir: Maybe you could move this to a new bug, so that this bug can be just about the help overlay?
And add this bug as a dependency for that bug?

About commit message: The body looks to long, maybe breakt it up into two lines? No need to talk about the help overlay, all shortcuts we add from now will be added to it, it is implicit.
Talk about the shortcut itself.

::: data/ui/help-overlay.ui
@@ +35,3 @@
                 <property name="visible">1</property>
+                <property name="title" translatable="yes">Go to current location</property>
+                <property name="accelerator">&lt;Primary&gt;L</property>

Maybe this should be added to the Map View section and not the General one? Since it affects the map view in the same sense that a zoom does.

::: src/mainWindow.js
@@ +186,3 @@
             },
             'goto-user-location': {
+                  accels: ['<Primary>L'],

Is this aligned correctly?

Allan: Do keyboard navigation keys belong in the overlay? like using the arrow keys for panning, it only applies of the map view has focus tho.

Comment on attachment 317544 [details] [review]
Add a help overlay

Attachment 317544 [details] pushed as 6c07319 - Add a help overlay

Sorry for the late response,I had gone on holiday ,No internet for 10 days :/
I made a new bug and filed the patch.Please check it out and let me know

Created attachment 318211 [details] [review]
Patch for keyboard shortcut

Did you add this patch to the wrong bug?
Did you mean: https://bugzilla.gnome.org/show_bug.cgi?id=760066

Oh sorry,I'll fix it.