Notes #5107
Notes #5107
Conversation
modules/services/notes.js
Outdated
| utilIdleWorker | ||
| } from '../util'; | ||
|
|
||
| var urlroot = 'https://api.openstreetmap.org', |
There was a problem hiding this comment.
This shouldn't really be a hardcoded URL endpoint, right?
There was a problem hiding this comment.
thanks @mmd-osm !
It is hard coded in the other services including osm.js. The remainder of the url includes the notes portion and then a bounding box once it is determined. What would you suggest?
There was a problem hiding this comment.
I was just wondering if you could run the Notes feature against a local database only (which is convenient for testing). As I have the osm.js part running in such a way, I'm assuming that there's some override mechanism in place. Looking very briefly at your implementation I couldn't find something similar for notes. Could be I simply missed it.
There was a problem hiding this comment.
Hmm yeah @mmd-osm and @thomas-hervey - it might make sense to refactor this notes service into services/osm.js, which already is setup to switch between dev and live servers.
For now, let's just test it how it is..
|
This is awesome 👏
|
This makes the osmNote work a bit more like other osm objects in iD.
- When working with the osm objects, we'll treat them as immutable.
So all modifications will be through the update method:
e.g. can do this in a repl, like chrome devtools console:
> n = iD.osmNote()
osmNote { id: -1 }
> n = n.update({ foo: 'bar' });
osmNote { foo: "bar", id: -1, v: 1 }
- none of the other osm objects have getters, and in JavaScript all the
properties are public anyway
|
I bit of feedback from local testing: sometimes, a note has quite a number of comments and you want to scroll through the list on the left hand side to see all details. Currently, I don't see a way to focus a specific note and easily browse through that list. Image (click to open)
|
This seems like a lot but the main things here are:
- remove the _loadingTiles "global" variable. It was causing problems because
it was being checked from the callbacks, which are async.
- cleanup the caches
- use DOM API getElementsByTagName('id') to get note id
- set a lower tilezoom z13 for notes (meaning: fetch larger bounding boxes)
Yep, this isn't implemented yet - coming soon! |
`postNoteUpdate` can hangle status changes and comment additions (I named it that to be like `putChangeset`) Also renamed `user` to `loadUser` to be consistent with other calls
This call really does need for a result to be passed to the callback
This eliminates a lot of the copy-paste code in every callback.
Before: We drew 2 fontawesome comment icons, on on top of the other After: Moved icon into iD spritesheet and gave it an actual `stroke` property that can be styled
|
Thanks for this feature! Where would you like to collect feedback – Ideas, issues, …? Separate issues in iD? Or here? Or somewhere external like a reply on Twitter? |
Separate issues would be perfect, and we already have a few. I read the replies on Twitter too because I know not everyone wants to use GitHub. |
Added initial display functionality of osm notes. There are several small bugs to fix including repeatedly appending svg tags within notes. Next, I will focus on hover triggers that will populate the sidebar with details related to a hovered note.