Location Events #596

Half-Shot · 2016-12-20T22:59:08Z

Render and post m.location events within riot. Uses Leaflet.

I went for OSM for the mapping service, but if anyone has any better ideas it should be easy enough to swap over to that. The other issue is that I'm using the upload icon as a placeholder, and using stock Leaflet icons for the map marker. That will probably need to be changed before this can be merged.

Other than that, this should be fairly functional.

matrixbot · 2016-12-20T22:59:09Z

Can one of the admins verify this patch?

ara4n · 2016-12-21T01:12:15Z

src/components/structures/UserSettings.js

@@ -337,9 +337,6 @@ module.exports = React.createClass({
            },
        */
        ];
-
-        var syncedSettings = UserSettingsStore.getSyncedSettings();


hm, why are you killing the generic user settings UI?

i'd expect that supporting location sharing would be precisely the sort of thing that the generic settings code would be good for. (or possibly hiding it behind a labs option for now)

Hmm, I might leave it in labs for now.

ara4n · 2016-12-21T01:12:56Z

src/components/views/dialogs/LocationInputDialog.js

+    },
+
+    componentDidMount: function() {
+      const leafletMap = new Leaflet.Map(this.refs.map);


please can haz consistent indentation? O:-)

ara4n · 2016-12-21T01:14:36Z

src/components/views/messages/MLocationBody.js

+    },
+
+    renderMap: function () {
+      const zoom = ZOOM_STREET;


again, 4 spaces for indentation please

ara4n · 2016-12-21T01:15:04Z

src/components/views/messages/MLocationBody.js

+          const uri = `https://www.openstreetmap.org/#map=${ZOOM_STREET}/${parts[0]}/${parts[1]}`;
+          this.setState({uri, coords, body: content.body});
+        } else {
+          this.setState({error: "Mising required fields."});


ara4n · 2016-12-21T01:15:58Z

src/components/views/rooms/MessageComposer.js

+        Modal.createDialog(
+          ErrorDialog, {
+          title: "Post Location",
+          description: "You either have disabled location in the browser, or it isnt supported.",


ara4n · 2016-12-21T01:17:07Z

src/components/views/rooms/MessageComposer.js

@@ -274,6 +316,15 @@ export default class MessageComposer extends React.Component {
                </div>
            );

+            const locationButton = (
+                // Add option for either current or specifed location.
+                // TODO: Add icon


Hm, in practice we probably are going to want a generic + button or something which provides a popup for the more exotic stuff like push-to-talk, locations, etc. But we can handle that!

ara4n · 2016-12-21T01:24:06Z

from quick inspection this is looking insanely cool :) there are few minor cosmeticish things in review comments, one wtf concerning generic settings, and i haven't actually tried using it yet.

One important thing: what is your proposed format for m.location events? I'm assuming:

content.geo_uri = "geo:...";
content.body = "some kind of label";
...

...but do we support formatted bodies here?

The Right solution to this would be to use the long-awaited "human representation of events" stuff to associate formatted bodies with the metadata, i suspect, but happy to ignore this for now; just want it clarified so we can spec it!

Half-Shot · 2016-12-21T03:26:07Z

Okay, that should be all the minor issues taken care of.

The spec is kinda lacking with what I had in mind.

"content": {
        "body": "Big Ben, London, UK",
        "geo_uri": "geo:51.5008,0.1247",
        "msgtype": "m.location",
        "thumbnail_info": {
            ....
        },
        "thumbnail_url": "mxc://localhost/FHyPlCeYUSFFxlgbQYZmoEoe"
    },

Has a few problems where I'd assume you'd want body and formatted_body to give some sort of representation to clients that do not support m.location events. Something along the lines of:

"body":"Big Ben @ geo:51.5008,0.1247",
"formatted_body":"Big Ben @ <a href='osm.org/blah'>geo:51.5008,0.1247</a>",

I think having a separate label field for clients that do support it would be more beneficial rather than trying to parse body. As far as thumbnail goes, I'm not sure how I'm going to handle that yet. The argument kind of goes that if you are aware of m.location events, then you can probably render them clientside with more up to date representation that something stored in a image.

Up to you if you want to merge this first or change the spec first :)

ara4n · 2016-12-21T23:49:08Z

happy to add it to spec later. the format looks good, although we may wish to finally implement the "human representation of events" stuff from https://docs.google.com/document/d/1cLSlMZ0tulRJM4ki-ImQ_z6DSySQgTRafO2m13RtgZo/edit#heading=h.6azo8zlrfebi to handle thumbnails & body representation etc of the m.location events. Let's hold off on speccing m.location until then.

Going to test this locally and then merge so folks can experiment in labs!

ara4n · 2016-12-22T00:10:18Z

Looks like there are some uncommitted resources here... e.g. img/marker-icon.png (or from the other PR, perhaps).

Otherwise, the only other weirdness is the width of the caption entry form on the map preview (but that's just cosmetic).

Are you sure that the thumbnails in the timeline should be interactively zoomable? They trap the mouse as you scrollwheel the page and start zooming the image rather than the page. I wonder if opening OSM (or opening a lightbox) would be better when you click on them, to let the user properly navigate...

Half-Shot · 2016-12-22T00:15:04Z

Looks like there are some uncommitted resources here... e.g. img/marker-icon.png (or from the other PR, perhaps).

Should be in riot-web, but apparently I didn't add them. Will do so. done

Otherwise, the only other weirdness is the width of the caption entry form on the map preview (but that's just cosmetic).

Yeah, that and a margin would be nice. I'll see if I can adjust it a bit.

Are you sure that the thumbnails in the timeline should be interactively zoomable? They trap the mouse as you scrollwheel the page and start zooming the image rather than the page. I wonder if opening OSM (or opening a lightbox) would be better when you click on them, to let the user properly navigate...

What I might end up doing here is thumbnailing the map at the sender end, and then opening a lightbox for an interactive version.

ara4n · 2016-12-22T00:17:26Z

right. for compatibility with other matrix clients I'd definitely suggest including a link to OSM in the caption (both HTML & plaintext for compatibility), and ideally thumbnailing it at the sender as you suggest.

(this is a really good example of why the 'human representation' event stuff is needed, imo, although given the final soln hasn't been picked there yet we can fudge it with body/formatted_body/thumbnail etc for now)

Half-Shot · 2016-12-22T01:15:42Z

Thumbnailing is proving more challenging than anticipated. I originally assumed you could draw any element onto a canvas (and then upload that as a thumb), but I'm limited to images and videos 🤦‍♂️ .

ara4n · 2016-12-22T02:03:40Z

does OSM not provide a "just give me an image" API?

Half-Shot · 2016-12-23T10:35:25Z

does OSM not provide a "just give me an image" API?

Only provides tiles :/. It's up to the client to assemble them which is a bit of a pain and why we're using leaflet in the first place :(. Would it be cool if we could conveniently leave off thumbnails for the time being (since they aren't strictly required) until I can figure out what the best solution it?

Make the events conform more to the spec.

Half-Shot · 2017-02-09T02:58:29Z

(New commits because my branch got manged after two months 😢 )

Added support for thumbnails for users that don't wish to display maps. We can render them in the browser :)

Half-Shot · 2017-02-09T03:22:43Z

It was brought up that this might throw a lot of traffic at OSM's servers, particularly in busy rooms. While I can somewhat limit this with thumbnails until hover/click, it might be a better idea for riot to host it's own tile server?

Ralith · 2017-02-09T04:26:30Z

A few random thoughts:

When sharing a point, the level of precision the user intends may vary wildly. Are they referring to a specific county in south-east Kansas, or the entire USA? Which should be displayed? It might be good to encode a hint regarding this in the event--perhaps a radius around the point that should be enclosed by the view area of a map tool?

Alternatively, the event could contain arbitrary geometry, for example using GeoJSON (which can be easily fed to most standard GIS or web mapping tools). That way people can send sets of points (favorite restaurants!), polygons (geocache zones!), line strings (travel routes!), or whatever else strikes their fancy.

Half-Shot · 2017-02-09T09:43:01Z

Alternatively, the event could contain arbitrary geometry, for example using GeoJSON (which can be easily fed to most standard GIS or web mapping tools). That way people can send sets of points (favorite restaurants!), polygons (geocache zones!), line strings (travel routes!), or whatever else strikes their fancy.

I like this one more. Seems like a good idea to be included into the spec as an alternative key.

Half-Shot · 2017-05-18T23:52:37Z

matrix-org/matrix-spec-proposals#919 will hopefully widen our scope with regards to what we can do with location events :)

t3chguy · 2017-05-19T18:15:03Z

Post Location button has same icon as upload file
Post location button is shown whether or not location picker is enabled in settings, any reason?
bunch of 404s
Post Location button in Modal has 404 /img/marker-icon-2x.png?cache=1495217303216:1 GET https://riot.ovh/img/marker-icon-2x.png?cache=1495217303216 404 ()

some of these are probably due to it needing merging to current develop

t3chguy · 2017-05-19T18:47:45Z

expects the marker-icon and marker-shadow to be in /img/$1 even if riot is in a subdir

Update branch to latest develop

Button visibility + marker image paths

t3chguy · 2017-05-20T10:47:31Z

src/components/views/rooms/MessageComposer.js

+            if (!UserSettingsStore.isFeatureEnabled('inline_maps')) {
+                locationButton = null;
+            }
+


typically the pattern for these is

let locationButton; if (UserSettingsStore.isFeatureEnabled('inline_maps')) { locationButton = (); }

just for the sake of consistency pls

pvagner · 2017-11-22T09:17:07Z

I am not sure this is a proper place for this comment however I would also like to have current location in profile or something very similar. So for example I can request the precise location of my direct chat partner if they allow me to do that. However I don't want all these location requests in the room timeline if possible. I am thinking that it might somehow complement users presence i.e. last seen %(relativeTime)s ago, near %(approxPlace)s or at %(exactSpot)s if allowed.
Some use cases from the wild:

Show a map of trusted group members if they wish to publish their location
Being able to check your relatives or friends

Of course this should all be optional with proper privacy controls and warnings. And of course it would be even more usefull if mobile apps were also sending location aware presence events if allowed.

almereyda · 2017-11-28T12:25:46Z

@pvagner indeed this may very well be the wrong place to discuss this. Yet since what you say is most interesting, I guess together with #3545 a geoindex on top of matrix messages, even allowing for NEAR searches would be something additional.

http://owntracks.org/ comes to mind here. We may in the end need a separate issue for a geo-aware indexing of Matrix messages et al. and would like to wait for suggestions of the maintainers to where and how this could be scoped.

jfrederickson · 2017-11-29T21:10:31Z

Got another thing that might be worth keeping in mind for future flexibility: location streaming. Not sure if that deserves another event type or if could be done with m.location events.

jryans · 2019-07-31T11:02:19Z

Thanks for making this contribution a while back. Since the code base has changed since this was opened, it no longer applies cleanly, and I don't think there's a need to keep it open in this state, as we can always find the code here again if needed. If you are still interested in pursuing this feature, please discuss with us in #riot-dev:matrix.org to find a good approach forward.

ara4n reviewed Dec 21, 2016

View reviewed changes

ara4n assigned Half-Shot Dec 21, 2016

Half-Shot added 7 commits February 9, 2017 02:30

Cherrypicking location patches

2229544

Cherrypick post m.location events from riot.

6151d87

Rename classNames for location maps.

fdf5c6e

Cherrypick move maps setting to labs.

0b0dcb3

Fix indent

2414582

Stuff

c083577

Add static map generation for m.location

3a80cd6

Make the events conform more to the spec.

Half-Shot force-pushed the location-events branch from c418e14 to 9a2c449 Compare February 9, 2017 02:57

Added thumbnail support for m.location events.

f0e1c13

Half-Shot force-pushed the location-events branch from 9a2c449 to f0e1c13 Compare February 9, 2017 02:59

turt2live and others added 5 commits May 19, 2017 13:02

Merge branch 'develop' into location-events

1d33bf4

Merge pull request #2 from turt2live/travis/half-shot/location-events

d8120b6

Update branch to latest develop

Don't show location button unless location support is enabled

3944016

Make marker images relative paths

b1b9056

Merge pull request #3 from turt2live/travis/half-shot/location-events

53dee4e

Button visibility + marker image paths

t3chguy reviewed May 20, 2017

View reviewed changes

turt2live mentioned this pull request Sep 23, 2017

Some required features element-hq/riot-meta#105

Open

t3chguy mentioned this pull request Nov 26, 2017

Location based rooms element-hq/element-meta#1254

Closed

prodrigestivill mentioned this pull request Mar 5, 2018

Added only to display location messages #1786

Closed

RotBolt mentioned this pull request Mar 10, 2019

[Feature] Share location element-hq/riot-android#1199

Open

jryans closed this Jul 31, 2019

vranki mentioned this pull request Oct 30, 2019

Use Matrix as transport for position updates bilde2910/Hauk#50

Open

turt2live mentioned this pull request Nov 25, 2019

CSS for Leaflet and location events. element-hq/element-web#2774

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Location Events #596

Location Events #596

Half-Shot commented Dec 20, 2016 •

edited

Loading

matrixbot commented Dec 20, 2016

ara4n Dec 21, 2016

ara4n Dec 21, 2016

Half-Shot Dec 21, 2016

ara4n Dec 21, 2016

ara4n Dec 21, 2016

ara4n Dec 21, 2016

ara4n Dec 21, 2016

ara4n Dec 21, 2016

ara4n commented Dec 21, 2016

Half-Shot commented Dec 21, 2016

ara4n commented Dec 21, 2016

ara4n commented Dec 22, 2016

Half-Shot commented Dec 22, 2016 •

edited

Loading

ara4n commented Dec 22, 2016

Half-Shot commented Dec 22, 2016 •

edited

Loading

ara4n commented Dec 22, 2016

Half-Shot commented Dec 23, 2016

Half-Shot commented Feb 9, 2017

Half-Shot commented Feb 9, 2017 •

edited

Loading

Ralith commented Feb 9, 2017 •

edited

Loading

Half-Shot commented Feb 9, 2017

Half-Shot commented May 18, 2017

t3chguy commented May 19, 2017 •

edited

Loading

t3chguy commented May 19, 2017

t3chguy May 20, 2017

pvagner commented Nov 22, 2017

almereyda commented Nov 28, 2017

jfrederickson commented Nov 29, 2017

jryans commented Jul 31, 2019

Location Events #596

Location Events #596

Conversation

Half-Shot commented Dec 20, 2016 • edited Loading

matrixbot commented Dec 20, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ara4n commented Dec 21, 2016

Half-Shot commented Dec 21, 2016

ara4n commented Dec 21, 2016

ara4n commented Dec 22, 2016

Half-Shot commented Dec 22, 2016 • edited Loading

ara4n commented Dec 22, 2016

Half-Shot commented Dec 22, 2016 • edited Loading

ara4n commented Dec 22, 2016

Half-Shot commented Dec 23, 2016

Half-Shot commented Feb 9, 2017

Half-Shot commented Feb 9, 2017 • edited Loading

Ralith commented Feb 9, 2017 • edited Loading

Half-Shot commented Feb 9, 2017

Half-Shot commented May 18, 2017

t3chguy commented May 19, 2017 • edited Loading

t3chguy commented May 19, 2017

Choose a reason for hiding this comment

pvagner commented Nov 22, 2017

almereyda commented Nov 28, 2017

jfrederickson commented Nov 29, 2017

jryans commented Jul 31, 2019

Half-Shot commented Dec 20, 2016 •

edited

Loading

Half-Shot commented Dec 22, 2016 •

edited

Loading

Half-Shot commented Dec 22, 2016 •

edited

Loading

Half-Shot commented Feb 9, 2017 •

edited

Loading

Ralith commented Feb 9, 2017 •

edited

Loading

t3chguy commented May 19, 2017 •

edited

Loading