GNOME Bugzilla – Bug 658553
Show maps for locations
Last modified: 2017-08-20 18:12:09 UTC
Is there any plans to support location awareness and/or integration with libchamplain in Contacts? If so we'd have to implement (something like) this in libfolks: https://bugzilla.gnome.org/show_bug.cgi?id=627400
We definitely have ambitions to display data about contacts, such as recent emails or shared documents. Location information could certainly be one part of that data. There hasn't been any design work done on location information with respect to contacts, however, so we need to think about the scope of this feature and potential use cases. It's also worth thinking about how contact specific location information would link in with a potential maps app.
I'm kinda considering getting rid of Empathy's contact info dialog (bug #661497) as gnome-contacts provides a better view of these infos, but having location support is a must have.
*** Bug 661212 has been marked as a duplicate of this bug. ***
As per bug 661212, a good first step would be to show a map for addresses.
This would still be nice to see. As I mentioned in the previous bug, we could start by showing a map beneath addresses. Clicking the map would open the location in Maps.
Yeah sounds cool. What would you need from Maps? A command line option or DBus thingie?
Created attachment 287090 [details] mockup Not sure what is needed here from a design point of view. Here's a basic mockup to indicate layout.
(In reply to comment #7) > Created an attachment (id=287090) [details] > mockup > > Not sure what is needed here from a design point of view. Here's a basic mockup > to indicate layout. Exactly that!
Ok, I have started working on this (see wip/map-widget). Questions! 1) So for contacts that have an address, we geocode for that address. This is an async operation. a) What should we show while we are doing this? b) And the operation might fail. Because we need to goecode from user inputed data, and we might not find anything. So should we indicate that and in that case how? 2) A contact may have more that one addres, right? (work/home/etc). How do we handle that in regard to map? Only for home? 3) Is it possible to save the lat/lon in the backend and re-use it? I thought I saw a location field in Folks. How does that work with for instance google-contacts? 4) So we want to re-use the maps-pin.svg that we ship in Maps? Should we add that to some theme or something instead? Thanks!
(In reply to comment #9) > Ok, I have started working on this (see wip/map-widget). > > Questions! > > 1) So for contacts that have an address, we geocode for that address. This is > an async operation. Ok > a) What should we show while we are doing this? A spinner loading? > b) And the operation might fail. Because we need to goecode from user inputed > data, and we might not find anything. So should we indicate that and in that > case how? No idea, ask Allan. > 2) A contact may have more that one addres, right? (work/home/etc). How do we > handle that in regard to map? Only for home? Can we you show all of them on same map? tagged for instance? > 3) Is it possible to save the lat/lon in the backend and re-use it? I thought I > saw a location field in Folks. How does that work with for instance > google-contacts? Dunno, let me do some research. > 4) So we want to re-use the maps-pin.svg that we ship in Maps? Should we add > that to some theme or something instead? For now, I would copy them to your branch, then we'll see. > Thanks! Thanks for working on this.
Thanks for working on this, Jonas! (In reply to comment #9) ... > 1) So for contacts that have an address, we geocode for that address. This is > an async operation. > > a) What should we show while we are doing this? I would show a placeholder - the rectangle that the map will appear in, but blank, and perhaps with a grey symbolic map icon in the middle. > b) And the operation might fail. Because we need to goecode from user inputed > data, and we might not find anything. So should we indicate that and in that > case how? If the location can be found, display it. Otherwise, leave the placeholder as it is. There's no need to give feedback that it has failed. > 2) A contact may have more that one addres, right? (work/home/etc). How do we > handle that in regard to map? Only for home? Only for home feels a bit arbitrary - what if you want to know where someone works? The best thing would be to show both locations on a single map. Otherwise, you could show a map for each address. ... > 4) So we want to re-use the maps-pin.svg that we ship in Maps? Should we add > that to some theme or something instead? You could try getting it added to adwaita-icon-theme - ask andreasn, jimmac or lapo.
(In reply to comment #11) > Thanks for working on this, Jonas! > > > > > a) What should we show while we are doing this? > > I would show a placeholder - the rectangle that the map will appear in, but > blank, and perhaps with a grey symbolic map icon in the middle. > Sure, a symbolic version of the Maps app icon, or would that make you assume that it is Maps embedded? And while on that subject: I have taken for granted that we just want a static map here, so I have disabled events. Do we want the user to be able to pan/zoom this map? > > b) And the operation might fail. Because we need to goecode from user inputed > > data, and we might not find anything. So should we indicate that and in that > > case how? > > If the location can be found, display it. Otherwise, leave the placeholder as > it is. There's no need to give feedback that it has failed. > Sure, thanks. > > 2) A contact may have more that one addres, right? (work/home/etc). How do we > > handle that in regard to map? Only for home? > > Only for home feels a bit arbitrary - what if you want to know where someone > works? The best thing would be to show both locations on a single map. > Otherwise, you could show a map for each address. > I can show both and use the Champlain ensure_visible method to get a zoom-level where both are visible. I can start with using Champlains Label-marker to display the name of the address type, if that does not look pretty enough we can find a designer willing to mock something up and create something with Champlain custom marker. And if there is only one address we can use the pin. > ... > > 4) So we want to re-use the maps-pin.svg that we ship in Maps? Should we add > > that to some theme or something instead? > > You could try getting it added to adwaita-icon-theme - ask andreasn, jimmac or > lapo. Yes, will talk to them. Thanks!
(In reply to comment #10) > > > 3) Is it possible to save the lat/lon in the backend and re-use it? I thought I > > saw a location field in Folks. How does that work with for instance > > google-contacts? > Dunno, let me do some research. > Thanks! It would be nice if we could store the location there. Then Maps and Contacts can help each other out with filling in the location values in the backend. And we can avoid having to Geocode all the time.
(In reply to comment #12) > (In reply to comment #11) > > Thanks for working on this, Jonas! > > > > > > > > > a) What should we show while we are doing this? > > > > I would show a placeholder - the rectangle that the map will appear in, but > > blank, and perhaps with a grey symbolic map icon in the middle. > > > > Sure, a symbolic version of the Maps app icon, or would that make you assume > that it is Maps embedded? I doubt that the symbolic version of the Maps icon will be that familiar... > And while on that subject: I have taken for granted > that we just want a static map here, so I have disabled events. Do we want the > user to be able to pan/zoom this map? Disabling events sounds like the right thing to do - the widget will be too small to be able to pan/zoom comfortably. Instead, clicking on the map can open that location in Maps proper, and you can zoom/pan from there. ,,, > I can show both and use the Champlain ensure_visible method to get a zoom-level > where both are visible. I can start with using Champlains Label-marker to > display the name of the address type, Or maybe the first line of the address? (To avoid multiple "Home" or "Work" markers.) > if that does not look pretty enough we > can find a designer willing to mock something up and create something with > Champlain custom marker. > > And if there is only one address we can use the pin. Yep, let's see how it goes.
Created attachment 292433 [details] Cast of contacts with map Hi, So I wanted to check in with what I got atm. The video shows two test contacts, one with 1 address one with 2. It is possible to click the map, and that will launch Maps with the ActivateAction method from org.freedesktop.Application, and call the "show-contact" action on Maps. But that is not complete yet. The issue I am facing here is, among other things, that I need to geocode the addresses once more when I get to the Maps side. I could send the locationslong I guess, but that would require some nifty variant serializing/deserializing. Atm I just send the uuid of the contact and look that up on the Maps side. Using the wip/contacts branch on Maps. It could take a while to sort that stuff out. I wonder: Is the map something we want event if it is not clickable? So it could be commited first and then we try to get the connection to Maps later on? In the video you will also see that the Map loads at a sort of random location first and the goes to the correct location. I needed to make sure the Map was available before I started calculating the correct bounding box. This is probably quite fixable.
(In reply to comment #15) > Created an attachment (id=292433) [details] > Cast of contacts with map > > Hi, > > So I wanted to check in with what I got atm. The video shows two test contacts, > one with 1 address one with 2. Looks fantastic! Some small things that could be improved: * You don't need to show a spinner. * The icon should be much larger, and grey. * When the map isn't shown, and there is the icon, it would be good to give the square a different colour background - a slightly darker shade of the background colour. * To me, the icon doesn't really say "maps". Maybe the icon itself needs to be improved, or maybe a different one could be used (mark-location-symbolic, possibly). * It would be good to minimise the amount of visual noise when loading the map: removing the initial appearance of the blue tiles before the map itself is loaded, and introducing the map in a single transition rather than as a series of tiles in sequence, > It is possible to click the map, and that will launch Maps with the > ActivateAction method from org.freedesktop.Application, and call the > "show-contact" action on Maps. But that is not complete yet. What does that look like to the user?
(In reply to comment #16) > (In reply to comment #15) > > Created an attachment (id=292433) [details] [details] > > Cast of contacts with map > > > > Hi, > > > > So I wanted to check in with what I got atm. The video shows two test contacts, > > one with 1 address one with 2. > > Looks fantastic! Some small things that could be improved: > > * You don't need to show a spinner. Ok! > * The icon should be much larger, and grey. Sure. > * When the map isn't shown, and there is the icon, it would be good to give > the square a different colour background - a slightly darker shade of the > background colour. Ok. > * To me, the icon doesn't really say "maps". Maybe the icon itself needs to be > improved, or maybe a different one could be used (mark-location-symbolic, > possibly). Can try that one, and maybe you could point Andreas to this bug/discussion. > * It would be good to minimise the amount of visual noise when loading the > map: removing the initial appearance of the blue tiles before the map itself is > loaded, and introducing the map in a single transition rather than as a series > of tiles in sequence, > Yeah, I want to do this as well. Maybe I need to render the map offscreen first and show when it is loaded. > > It is possible to click the map, and that will launch Maps with the > > ActivateAction method from org.freedesktop.Application, and call the > > "show-contact" action on Maps. But that is not complete yet. > > What does that look like to the user? Since I use this dbus method and pass in a special timestamp it should look like launching an app should look. You get the spinner at the top panel, if Maps is open the window will be presented, if it is not it will start and be presented. Can do a video of it later.
(In reply to comment #13) > > Thanks! It would be nice if we could store the location there. Then Maps and > Contacts can help each other out with filling in the location values in the > backend. And we can avoid having to Geocode all the time. Yes, you can, use this [1]. Grab the location, build a location object [2] and set it on the persona from which you took the address. This last bit is important, it must be set on the [1]: https://git.gnome.org/browse/folks/tree/backends/eds/lib/edsf-persona.vala#n256 [2]: https://git.gnome.org/browse/folks/tree/folks/location-details.vala#n31 Great work. Thxs for working on this.
(In reply to comment #18) > (In reply to comment #13) > > > > Thanks! It would be nice if we could store the location there. Then Maps and > > Contacts can help each other out with filling in the location values in the > > backend. And we can avoid having to Geocode all the time. > > Yes, you can, use this [1]. Grab the location, build a location object [2] and > set it on the persona from which you took the address. This last bit is > important, it must be set on the > > [1]: > https://git.gnome.org/browse/folks/tree/backends/eds/lib/edsf-persona.vala#n256 > [2]: https://git.gnome.org/browse/folks/tree/folks/location-details.vala#n31 > > Great work. Thxs for working on this. Hi! Thanks. I looked at that but can it really be used? For once, it states: "Location of a contact. folks tries to keep track of the current location and thus favors live data (say, as advertised by a chat service) over static data (from an address book). Static addresses, such as a contact's home or work address, should be presented using the PostalAddressDetails interface. LocationDetails is purely for exposing the contact's current or recent location." And secondly, it only seem possible to store one location per individual? And we can have multiple addresses, so it seems like it would not work?
Created attachment 292444 [details] Cast of contacts with map So another version. * Have switched icons * Should be less visual noise * No spinner * Click to go to Maps
(In reply to comment #20) Looks great to me!
(In reply to comment #19) > > And secondly, it only seem possible to store one location per individual? And > we can have multiple addresses, so it seems like it would not work? I've pointed you to Persona::location property, so each persona will have a property. Also, we want to respect that, and don't set the location details on the individual as a whole, just per persona, so, live data from a chat service won't clash with other static data from eds or other sources. Again, we should use this only on persona basis, not individual. Now, certainly, we should be able to keep more than one location per persona.
(In reply to comment #22) > (In reply to comment #19) > > > > And secondly, it only seem possible to store one location per individual? And > > we can have multiple addresses, so it seems like it would not work? > > I've pointed you to Persona::location property, so each persona will have a > property. Also, we want to respect that, and don't set the location details on > the individual as a whole, just per persona, so, live data from a chat service > won't clash with other static data from eds or other sources. > > Again, we should use this only on persona basis, not individual. > > Now, certainly, we should be able to keep more than one location per persona. I am not sure. I talked to Philip on irc: "9:47 AM <pwithnall> jonasdn: Back. What was your question? :) 9:47 AM <jonasdn> So I get the addresses from a contact (PostalAddresses) 9:47 AM <jonasdn> and I geocode them and I place markers on the map 9:47 AM <jonasdn> When you click the map you will go to Maps 9:47 AM <jonasdn> I would like to avoid geocoding in Maps as well. And I would like Maps and Contacts to help each other out with geocoding. 9:47 AM <jonasdn> But. 9:48 AM <jonasdn> I can't see a way to store a location for an address with Folks. 9:48 AM <jonasdn> Erick, contacts developer, said I could add locations to a Persona, but I don't see how, I am not experienced with Folks 9:49 AM <pwithnall> What do you mean by a ‘location’? 9:49 AM <pwithnall> A postal address? 9:50 AM <jonasdn> no 9:50 AM <jonasdn> a latitude/longitude location 9:51 AM <pwithnall> oh, right 9:51 AM <pwithnall> folks supports that with the LocationDetails interface 9:52 AM <jonasdn> yeah, for Individual it states it should not be used with static addresses, only for "current location" data gotten from im clients and such 9:52 AM <jonasdn> Erick said I should add it for Persona. 9:52 AM <jonasdn> (I am hazy on Folks architecture) 9:52 AM <jonasdn> but there can be multiple addresses 9:52 AM <pwithnall> Erick is wrong there 9:53 AM <jonasdn> And I need to know for what address a location is valid 9:53 AM <pwithnall> In folks’ architecture, Individual represents a group of Personas and supports all interfaces 9:53 AM <pwithnall> Individual Personas should only implement the interfaces supported by the backends they originate from 9:53 AM <pwithnall> So only the EDS Persona supports LocationDetails 9:53 AM <jonasdn> ok 9:53 AM <jonasdn> But LocationDetails interface only gives me get/set location? 9:53 AM <pwithnall> This sounds like something which folks isn’t designed to do 9:53 AM <jonasdn> a single location is not really useful for me 9:54 AM <pwithnall> Yes, and it is only for the person’s current location 9:54 AM <jonasdn> yeah, agreed 9:54 AM <pwithnall> I think you need to maintain your own location/address cache somewhere 9:54 AM <jonasdn> So either Maps and contacts need a separate backendstorage for location, or both need to geocode everytime 9:54 AM <pwithnall> It is just a caching problem for geocoding 9:54 AM <pwithnall> Which I think definitely belongs in the geocoding library 9:54 AM <jonasdn> well, the cache needs to be kept in sync when address changes 9:54 AM <pwithnall> since the location for an address is never going to change :) 9:55 AM <pwithnall> No, the cache would need a new entry if the address changes 9:55 AM <pwithnall> The entry for the old address would still be valid 9:55 AM <jonasdn> So maybe gnome wants a cache for address=>location 9:55 AM <jonasdn> and if maps / contacts gets a cache miss, they perform a geocode 9:55 AM <pwithnall> As I understand things, that sounds like the best solution to me :) 9:55 AM <jonasdn> and update the cache 9:55 AM <pwithnall> yup 9:55 AM <jonasdn> cumbersome but probably correct 9:56 AM <pwithnall> Maybe look into making the cache desktop-wide (e.g. a freedesktop.org project), rather than GNOME- or Maps-specific 9:56 AM <pwithnall> since KDE could make use of that too"
geocode-glib has support for caching requests and their results, which probably needs more testing to ensure that requests are the same when the same arguments are passed.
New cast, I know geocode in Maps as well and present the locations: https://www.youtube.com/watch?v=DjHTHv2VO64 Bastien: Will look into the cache to see if it behaves.
Created attachment 292814 [details] [review] contacts-utils: Add activate_action function Add a function that activates an action on a given appid.
Created attachment 292815 [details] [review] Add a map pin icon
Created attachment 292816 [details] [review] contacts-app: Add our search path to icon theme
Created attachment 292817 [details] [review] contacts-contact: Add method to geocode a contact
Created attachment 292818 [details] [review] Add map widget
Created attachment 292819 [details] [review] Add a map to the contacts sheet
Created attachment 292821 [details] Cast of contacts with map
So here are some patches. I have worries. The street number resolution in OpenStreetMap is not great at all. We are very lucky if we can geocode to a specified address. Most often this will place contacts on the correct street, but not at the correct house. So it may end up as misinformation. Could we indicate in some way? Please try this out with some of your own contacts to see the effect! You need to have Maps with the patches in the blocking bug 741591 installed on your system proper I think, not just jhbuild env. Thanks
Created attachment 292824 [details] [review] Add map widget
Created attachment 292825 [details] [review] Add a map to the contacts sheet
Review of attachment 292814 [details] [review]: Good to go
Review of attachment 292824 [details] [review]: Others that this minor tweaks, looks great to me. ::: data/ui/style.css @@ +13,3 @@ +.contacts-map { + background-color: @theme_bg_color; + color: gray; Could you not hardcode a color, here. Contacts has had trouble in the past when using the dark variant of Adwaita, my guess is that hard-coding colors won't help. ::: src/contacts-address-map.vala @@ +1,3 @@ +/* -*- Mode: vala; indent-tabs-mode: t; c-basic-offset: 2; tab-width: 8 -*- */ +/* + * Add your copyright line. Your work should be known.
Review of attachment 292825 [details] [review]: Good to go
Review of attachment 292824 [details] [review]: Thanks for review! Will respin this when we get confirmation on the icon. ::: configure.ac @@ +51,3 @@ goa-1.0 gee-0.8 + champlain-gtk-0.12 Should probably add champlain-0.12 as well, for correctnessm and maybe only if we choose not to use GtkChamplainEmbed. ::: data/ui/style.css @@ +13,3 @@ +.contacts-map { + background-color: @theme_bg_color; + color: gray; You mean finding a @-type value instead of gray? Is there anyone that would work? ::: src/contacts-address-map.vala @@ +1,3 @@ +/* -*- Mode: vala; indent-tabs-mode: t; c-basic-offset: 2; tab-width: 8 -*- */ +/* + * Ok. @@ +60,3 @@ + map.get_child ().get_window ().set_cursor (null); + return false; + }); If we wanted to avoid hacks like this we could also create our own ClutterEmbed with the champlain map inside, then we would have more control over what cursors are shown. And wouldn't have to go through loops using implementation details like above, should be simple enough. @@ +74,3 @@ + + addresses = postal_addresses; + found_places = new GLib.List<Place>(); Note to self: space before ()
Another question: Should we react to Maps not being installed and if so, how? Not show map? Not making map clickable?
(In reply to comment #41) > > ::: data/ui/style.css > @@ +13,3 @@ > +.contacts-map { > + background-color: @theme_bg_color; > + color: gray; > > You mean finding a @-type value instead of gray? Is there anyone that would > work? > Yes, something to be complaint with Adwaita theming, maybe something like @theme_fg_color.
(In reply to comment #42) > Another question: Should we react to Maps not being installed and if so, how? > Not show map? Not making map clickable? Making map not reactive to clicks would be fine, but I'd like to show some message like: "Hey Google Maps is not installed. Go and install it so you can see the location", Obviously better worded.
Created attachment 293250 [details] [review] Add map widget
Created attachment 293251 [details] [review] Add map widget
Review of attachment 292817 [details] [review]: This one is fine by me. We just talked about you using the ZIP code or not. You can commit this, and improve it afterward.
Created attachment 294058 [details] [review] Add map widget
Created attachment 294406 [details] [review] Add map widget
Review of attachment 294406 [details] [review]: Looks good for me
Created attachment 294491 [details] [review] contacts-address-map: Go to Maps on button press
Maps have landed in gnome-contacts 3.16, I think this is fixed.