Notes #1895

Closed
wants to merge 7 commits into
from

Conversation

Projects
None yet
6 participants
@tmcw
Contributor

tmcw commented Oct 15, 2013

Oh boy. This adds notes functionality to iD, this addressing #1393

Right now it takes the approach that:

  • Notes have their own mode and ui
  • Notes are in the history/graph and have the prefix 'e'
  • In some places notes need to be explicitly excluded so they aren't dragged/selected/etc.

Todos:

  • Re-add the show-notes toggle
  • Tests
  • Should this be able to create a new note? Ugh.
  • The comment-posting UI 'lies' but that's probably all right.
  • The styling could be cleaner
  • We could use a notes icon instead of a blank circle as I'm doing now
  • Refactor note-versus-point CSS
  • Notes should have halos too?
@jfirebaugh

This comment has been minimized.

Show comment
Hide comment
@jfirebaugh

jfirebaugh Oct 15, 2013

Member

Seems like a good approach to me.

Would it be cleaner if note SVG elements were not classed point though?

Member

jfirebaugh commented Oct 15, 2013

Seems like a good approach to me.

Would it be cleaner if note SVG elements were not classed point though?

js/id/renderer/notes_layer.js
@@ -0,0 +1,85 @@
+iD.NotesLayer = function(context, dispatch) {

This comment has been minimized.

@jfirebaugh

jfirebaugh Oct 15, 2013

Member

This class is no longer used, right?

@jfirebaugh

jfirebaugh Oct 15, 2013

Member

This class is no longer used, right?

@tmcw

This comment has been minimized.

Show comment
Hide comment
@tmcw

tmcw Oct 15, 2013

Contributor

Would it be cleaner if note SVG elements were not classed point though?

Yes, this is currently an icky point; needs CSS refactoring

Contributor

tmcw commented Oct 15, 2013

Would it be cleaner if note SVG elements were not classed point though?

Yes, this is currently an icky point; needs CSS refactoring

@tmcw

This comment has been minimized.

Show comment
Hide comment
@tmcw

tmcw Oct 30, 2013

Contributor

@samanpwbb Do you have thoughts on how notes should be shown with the map in iD? On osm they have hacky leaflet-esque markers with green/red colors.

Contributor

tmcw commented Oct 30, 2013

@samanpwbb Do you have thoughts on how notes should be shown with the map in iD? On osm they have hacky leaflet-esque markers with green/red colors.

@samanpwbb

This comment has been minimized.

Show comment
Hide comment
@samanpwbb

samanpwbb Oct 30, 2013

Member

@tmcw as much as I'd like to restyle these to be better looking, I think the best approach right now is to make notes in iD as close as possible to notes in osm.org, for the sake of familiarity, and so the two can evolve together.

Member

samanpwbb commented Oct 30, 2013

@tmcw as much as I'd like to restyle these to be better looking, I think the best approach right now is to make notes in iD as close as possible to notes in osm.org, for the sake of familiarity, and so the two can evolve together.

@mourner

This comment has been minimized.

Show comment
Hide comment
@mourner

mourner Jan 8, 2014

Contributor

Is this on hold now?

Contributor

mourner commented Jan 8, 2014

Is this on hold now?

@jfirebaugh

This comment has been minimized.

Show comment
Hide comment
@jfirebaugh

jfirebaugh Jan 8, 2014

Member

Yes, iD's kind of in low maintenance mode currently.

Member

jfirebaugh commented Jan 8, 2014

Yes, iD's kind of in low maintenance mode currently.

This was referenced Mar 28, 2014

@ToeBee

This comment has been minimized.

Show comment
Hide comment
@ToeBee

ToeBee Nov 4, 2014

Contributor

Just to poke things a little, as of an hour ago, josm-latest now has (very basic) note support built into core :)

On a more serious note, it looks like iD is running into some of the same issues I did when adding note support to JOSM. Notes are part of the OSM API but are different from all other API objects in that they don't involve versions, tags or changesets. It does make things kind of awkward...

Contributor

ToeBee commented Nov 4, 2014

Just to poke things a little, as of an hour ago, josm-latest now has (very basic) note support built into core :)

On a more serious note, it looks like iD is running into some of the same issues I did when adding note support to JOSM. Notes are part of the OSM API but are different from all other API objects in that they don't involve versions, tags or changesets. It does make things kind of awkward...

@bhousel bhousel referenced this pull request May 3, 2015

Open

Map notes #2629

@jameslaneconkling jameslaneconkling referenced this pull request in loggingroads/logging-roads Jul 9, 2015

Open

Drop an OSM note asking for clarification of imagery #37

@bhousel bhousel removed the 1.6 label Jul 29, 2015

@tmcw tmcw closed this Oct 7, 2015

@bhousel bhousel deleted the notes branch Sep 26, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment