mapbox / deprecated-osm-comments Goto Github PK

Current code handles API being down, but not 500 errors, as they are not returned as an error in the xhr callback.

i.e. https://www.openstreetmap.org/user/Das%20Fantom

http://mapbox.github.io/osm-comments-frontend/#/changesets/?q=users:Das%20Fantom

Changeset: https://www.openstreetmap.org/changeset/29915022

Curently the thumbnail images to the right don't really add visual clues.
I'd suggest to display them at zoom level around 15:
and to make it clickable (starts which osm.org at this place).

Need to figure out an ideal deployment work-flow with webpack.

Right now, when running the dev-server, it serves bundle.js from memory, but to deploy to gh-pages / mb-pages, we need to write to disk. Both these locations should be the same.

Also, in deploy mode, it should not poll local websocket server for changes.

Edit the Loading component + css to make loading look nicer, with a spinner or so.

/cc @geohacker

@tristen can you help? :)

This is straightforward to do with, for eg: linkify

We will however then need to inject HTML into the DOM using React's dangerouslySetInnerHTML or so. The incoming string will need to be properly sanitized before being passed to the linkify function.

Some libs to look at for sanitizing:

• https://github.com/cure53/DOMPurify
• https://github.com/mapbox/sanitize-caja

Perhaps something like users:* should omit the users filter request to the API and return results for all users.

Default sorting is based on date of last comment but is not very obvious on the UI.

Something like Last comment: 12 days ago would help

As per, #13 we will default to searching only through changeset discussions. Add a syntax to allow user to search only through comments - like comments:foo as the search or so.

This is until we resolve https://github.com/mapbox/osm-comments-api/issues/30

Will add a way to only search changeset comments if user wishes to.

When a user arrives at search form from a URL, ensure the fields are filled in correctly.

Just to make it easier to serve using gh-pages / mb-pages, and also since it is a bit awkward inside dist/ since it is not really built - will make things more clear.

Figure out tools / libraries and boilerplate the front-end.

It seems like it would make sense to use ReactJS for this, even though my limited experience with it would mean I'd probably need a little bit of hand-holding / guidance.

@aaronlidman @tristen I was going to use basic boilerplate ideas from https://github.com/osmlab/to-fix - does this seem reasonable or are there other resources you think I should look at?

Would also love any links to reading that maybe useful for starting off doing stuff with React. Thanks!

/cc @rclark @geohacker

Add "About..." page and a footer inlcuding weblinks like this:
"© OpenStreetMap contributors | Improve this map"

I came across this issue, when I wanted to check my OSM note which I added recently on 24-Aug-2016 and when I query for that it was not being shown up. Instead it was showing other notes which were added on March,2016 but not mine which is more recent.

Query link: https://www.mapbox.com/osm-comments/#/notes/?q=bbox:13.12,52.35,13.79,52.65%20from:2015-08-01

• bbox coordinates taken from here . In this case it was Berlin, where my OSM notes falls within this bbox.

It would be great if it resolves which will help us in collecting stats for our next mapping task in Germany, where we are giving OSM notes for the missing turn restrictions.

cc @geohacker @batpad

@batpad @geohacker - again, great work here. I think we can structure the content in listing better with some small changes.

Goals:

• See quickly what's been said last (a little like your messages app on ios or Android)
• Show clearly number of comments, time of first note / changeset, time of last post
• Simplify where possible

Wires (intent here is to show functionality, actual layout can differ):

Comments page

Notes page

corresponding changes on notes page:

cc @mikelmaron

https://www.openstreetmap.org/user/malenki/diary/38480

Some notes that are closed are displaying as open. Need to investigate.

I was just having a play, trying out viewing notes in London using this tool with this URL https://www.mapbox.com/osm-comments/#/notes/?q=bbox%3A0.3021%2C51.7253%2C-0.5576%2C51.2550 but is seems as though there's an additional filter going on, maybe it's filtering just mapbox data team users? Any way of using the tool for browsing notes from all users?

Right now, you need to run webpack to build. This should just be npm build to be more standard.

individual instances / setups should be able to over-ride values in config - to set a different API URL, for eg. or a different Mapbox Access Token.

Would create an optional local_config.js where users can over-ride settings defined in config.js

... but another user created that note.

If I am not wrong, this service only displays OSM noted that a specific user has created.

So you can see your own OSM notes, and filtering about open or closed status.

Is it possible in general to display all OSM notes where a specific user (the one you can query for) left a comment, but the OSM note was created by another user?

Figure out adding the Map component. Component will need to listen to certain events:

• when a new non-bbox search takes place, zoom to extent of results
• when user requests a bbox search, take current map extents as bbox
• Ideally, highlight point on map when user hovers over item in result list

Make it a bit more clear in the README that it defaults to searching for users defined in config file, and this can be over-ridden by searching for users:*

/cc @harry-wood

https://www.mapbox.com/osm-comments/#/changesets/?q=users%3A%2A+involves%3Asaikabhi&show=all

vs

https://www.mapbox.com/osm-comments/#/changesets/?q=users%3A%2A+involves%3Asaikabhi

Does not look right.

cc @abhisheksaikia

Right now, most filters apply to the changeset, and not the comment. This is very confusing. Users expect to find changesets that they have commented on when searching for users:<username>.

This is being tracked:

• Search for users: https://github.com/mapbox/osm-comments-api/issues/44
• Search for from: https://github.com/mapbox/osm-comments-api/issues/45

Implementing these probably does require a bit of a refactor of the SQL, and perhaps some schema changes. For now, we should clarify the documentation clearly to mention this.

@batpad I think it will be a good idea to add tooltips the icons.

Should show something to the user instead of just showing Loading spinner when an API request fails.

@batpad there should be a minimal about page (about modal) describing what this app is. Think of someone landing on the app without any context.

• brief description
• link to mapping wiki

Stuff like this: https://github.com/mapbox/osm-comments-frontend/blob/dbccd49451457169e9b5db100ad79fcb2a83a5c3/app/components/ChangesetsSearch.js#L18

We should be able to set and receive URLs with query params as objects without doing string manipulation and concatenation.

During validation we have been leaving changeset comments on OSM user changesets. The osm-comments-API allows us to download geojson of all changesets with discussion for the past 1 year. It would be great to visualize unresolved changeset discussions continuously similar to this.

The complete geojson can be filtered by

• discussioncount=1
• lastcommentusername=*(from Mapbox data team)

cc @batpad @planemad @geohacker @ajithranka @maning @manoharuss

parse search query for user:{comma_seperated_list_of_osm_user_names}, and override default list of users sent to api query

Hi @batpad, congrats on launching the osm-comments page!

I have a few design feedback suggestions to make the page look a little bit more polished:

Header part

1. The Mapbox logo

By adding the dark class to this container element of the logo, it will be white instead of cyan, which will look a bit cleaner on that background.

2. The navigation/tool bar

Instead of using row1 to set the height of this element, you can use pad1y which will give all the elements a little bit more vertical space.

3. Search field

It would be nice if the search field is right aligned with the rest of the content on the page. You can use pin-right on the parent element, and you'll have to add a contain class to the limiter which holds all of the elements in this bar. Using the contain class will set position: relative so you can absolute position the search field with pin-right.

Like this:

Comment boxes

In general my main feedback would be to give all the different elements in these boxes a little bit more breathing space. There's a lot going on right now, which makes it hard to quickly scan the content.

One thing that always helps is adding prose to text elements, the prose class will automatically make the line-height bigger and add margin-bottoms where needed. I would also recommend to not use the row class too much, but just let the elements itself decide how much vertical space the should take up, and add padding or margin if needed.

Please ping me if you have any questions, or if you want to talk through any of these suggestions!

Right now:

Suggestions:

In this snippet you can see some of the classes I added/removed (mostly prose pad0y small quiet)

<div class="clearfix box round pad2 blurb" data-reactid=".0.1.0.$500264"><div class="" data-reactid=".0.1.0.$500264.0"><div class="col8 row2" data-reactid=".0.1.0.$500264.0.0"><div class="clearfix" data-reactid=".0.1.0.$500264.0.0.0"><div class="col12" data-reactid=".0.1.0.$500264.0.0.0.0"><h3 class="fancy inline-heading middle inline fl" data-reactid=".0.1.0.$500264.0.0.0.0.0"><a href="https://www.openstreetmap.org/note/500264" target="_blank" data-reactid=".0.1.0.$500264.0.0.0.0.0.0">500264</a></h3><span class="button short fill-green button-status button-inline pad1x fl space-top0 space-left1" data-reactid=".0.1.0.$500264.0.0.0.0.1">Open</span></div></div><div class="quiet pad0y small" data-reactid=".0.1.0.$500264.0.0.1"><a class="icon account" title="User" href="https://www.openstreetmap.org/user/andygol" target="_blank" data-reactid=".0.1.0.$500264.0.0.1.0">andygol</a><span data-reactid=".0.1.0.$500264.0.0.1.1"> |</span><span class="icon time" href="#" title="Date" data-reactid=".0.1.0.$500264.0.0.1.2">a day ago</span><span data-reactid=".0.1.0.$500264.0.0.1.3"> |</span><span class="icon contact" title="Number of comments" data-reactid=".0.1.0.$500264.0.0.1.4">9</span><span data-reactid=".0.1.0.$500264.0.0.1.5"> |</span><a class="icon crosshair" href="#" data-reactid=".0.1.0.$500264.0.0.1.6">JOSM</a></div><div class="prose pad1y quiet" data-reactid=".0.1.0.$500264.0.0.2"><span data-reactid=".0.1.0.$500264.0.0.2.0">Can anyone investigate on the ground an  amount of mini-roundabouts here? For now there are mapped 6 instead of 5, as I can see at https://youtu.be/DmvHZ4omB2A. And tracing is not accurate :(</span><div class="space-top1" data-reactid=".0.1.0.$500264.0.0.2.1"><span class="icon contact comment-user quiet" title="Comment" data-reactid=".0.1.0.$500264.0.0.2.1.0"><a class="" href="https://www.openstreetmap.org/user/SK53" target="_blank" data-reactid=".0.1.0.$500264.0.0.2.1.0.0">SK53</a><span data-reactid=".0.1.0.$500264.0.0.2.1.0.1"> </span></span><span class="quiet" data-reactid=".0.1.0.$500264.0.0.2.1.1">commented a day ago</span><div class="comment-last pad1rt" data-reactid=".0.1.0.$500264.0.0.2.1.2">@andygol I'd tend to agree with your interpretation that the '6th' roundabout is an artefact of the imagery. I think the whole principle of 'magic' roundabouts requires as many mini-roundabouts as exit roads. There is Mapillary sequence, but from a cyclist so not completely useful: http://www.mapillary.com/map/im/xIBy8JnGDU8IxPd7MhxSGA/photo.

I think your first comment stands: it needs someone to check it out on the ground.</div></div></div></div><div class="col4 clip" data-reactid=".0.1.0.$500264.0.1"><img src="https://api.mapbox.com/v4/mapbox.streets/-1.7712155,51.5628654,13/500x300.png?access_token=pk.eyJ1Ijoic2FuamF5YiIsImEiOiI3NjVvMFY0In0.byn_eCZGAwR1yaPeC-SVKw" data-reactid=".0.1.0.$500264.0.1.0"></div></div></div>

Responsive

Also make sure to always check what the page will look like on smaller screen, sometimes some elements can behave in a weird way:

• **I'm submitting a ... **

• bug report
• feature request
• support / question

Brief Description

I looked for OSM note filtering tool and found this one. I followed links in the readme to test whatever tool is dead/broken or working.

What is the motivation / use case for this feature?

Filtering OSM notes.

What is the current behaviour, (attach relevant screenshots) ?

Following "bbox:0.3021,51.7253,-0.5576,51.2550 to find conversations happening with the London bounding box (try it)" leads to https://www.mapbox.com/osm-comments/#/notes/?q=bbox:0.3021,51.7253,-0.5576,51.2550 with "No results found."

What is the expected behaviour ?

At least https://www.openstreetmap.org/note/1121181 is displayed

When does this occur ?

Every time when I tried.

How do we replicate the issue ?

1. Follow readme link

Please tell us about your environment:

Firefox 56.0 (64-bit)

Other Information / context:

We need some way to page through results.

I would prefer explicit Next / Previous buttons to also be able to have permalinks for pages, but another option would be an infinite scroll where we just fetch more results as the user scrolls down.

@samanpwbb @tristen any examples of pagination that you think I should follow? Any strong opinions on explicit prev / next vs infinite scroll?

Currently only a link to JOSM is supported, which needs separate installation.
Add support for built-in iD editor which needs no installation.

No bbox == No map
See: https://www.openstreetmap.org/api/0.6/changeset/36082110/download

The search syntax as it is is pretty powerful, but also pretty confusing.

We should have an explicit ? next to the search field that explains the syntax in details with examples.

Using ffox: 43.0.03

Search for PlaneMad > 1 result. Search for users:PlaneMad > 3 results

Both should give 3 hits?

Write tests for the front-end.

@tristen any pointers / examples of tests for React code I could look at to get started?

@tristen can you help?

@batpad @mikelmaron @geohacker -

Should we just call this osmcomments?

https://www.mapbox.com/osmcomments