[patch] Add notes dialog

[ToeBee]

Time for another big notes patch! Some comments:

Icons

• Copy images/notes/note_open_24x24.png to images/dialogs/notes.png
• Copy images/notes/note_new_24x24.png to images/mapmode/addnote.png
• Add file images/notes/note_comment.png (will attach)
• Add file images/cursor/modifier/create_note.png (will attach)

To make the note_comment.png icon I started off with something from openclipart and modified it.

Note that the create_note.png cursor modifier may look like the 16 pixel new note file but the contents are different. Apparently the cursor doesn't like having more than 3 or 4 colors in it. The dithering around the edges of the marker made the cursor do weird things so I simplified it to only have solid blue, solid white and transparency.

UI

I'm not going to expect any awards for UI design here. I pretty much just duplicated the functionality of the notes plugin for now. Using a JToolTip to render the note comments on the map seems like a hack and I have some ideas for how to change it but I think I will ask for some input on the mailing list before I go too far there. Also, for right now I want to get the basics working so that I can finish the rest of the functionality (uploading and saving still need to be implemented). Then things can be made prettier.

Internals

There are several things in the code that assume there will only ever be a single note layer. I don't really see how things would work if you could make two note layers. Since the note layer isn't an active editable layer but still responds to mouse events, it would be weird to have more than one. Which one would respond to note selection events? Which list of notes would be displayed in the dialog? I also don't see why you would need more than one. It's not like you can perform all that many actions on notes. They are just an aid for editing the map data. Any objections to this?

NoteLayer and NoteDialog need to communicate with each other. I'm not sure the way I have done it is ideal. Whenever NoteLayer modifies its collection of notes it pushes the list to NoteDialog through a public reference in MapFrame. There were already some other dialogs that did something similar. The other way I thought about doing it was to make a change listener interface and then have NoteDialog register itself with NoteLayer to receive updates. Wouldn't be hard to do and would let other things listen for note changes as well. I can't really think of much that would need it off the top of my head but who knows.

The slightly more complicated interaction is note selections. Notes can be selected in both NoteDialog and NoteLayer so they both have to communicate to each other. Layer to dialog interaction is done through the public reference in MapFrame again. For the dialog to talk to the layer, it registers itself to listen for layer changes and if a note layer is created, keeps a pointer to it until the layer is removed. This is also how the dialog interacts with the layer to modify notes. This is one instance of only being able to support a single layer.

[ToeBee]

Comment icon. Place in images/notes/

[ToeBee]

Note creation mode cursor modifier. Place in images/cursor/modifier/

[bastiK]

Replying to ToeBee:

Time for another big notes patch! Some comments:

Icons

• Copy images/notes/note_open_24x24.png to images/dialogs/notes.png
• Copy images/notes/note_new_24x24.png to images/mapmode/addnote.png

We usually don't include images twice, because it is a waste of space.

• Add file images/notes/note_comment.png (will attach)
• Add file images/cursor/modifier/create_note.png (will attach)

To make the note_comment.png icon I started off with something from openclipart and modified it.

Note that the create_note.png cursor modifier may look like the 16 pixel new note file but the contents are different. Apparently the cursor doesn't like having more than 3 or 4 colors in it. The dithering around the edges of the marker made the cursor do weird things so I simplified it to only have solid blue, solid white and transparency.

UI

I'm not going to expect any awards for UI design here. I pretty much just duplicated the functionality of the notes plugin for now. Using a JToolTip to render the note comments on the map seems like a hack and I have some ideas for how to change it but I think I will ask for some input on the mailing list before I go too far there. Also, for right now I want to get the basics working so that I can finish the rest of the functionality (uploading and saving still need to be implemented). Then things can be made prettier.

Yes, please make it functional and pretty, I'm looking forward to your final version. :)

Internals

There are several things in the code that assume there will only ever be a single note layer. I don't really see how things would work if you could make two note layers. Since the note layer isn't an active editable layer but still responds to mouse events, it would be weird to have more than one. Which one would respond to note selection events? Which list of notes would be displayed in the dialog? I also don't see why you would need more than one. It's not like you can perform all that many actions on notes. They are just an aid for editing the map data. Any objections to this?

NoteLayer and NoteDialog need to communicate with each other. I'm not sure the way I have done it is ideal. Whenever NoteLayer modifies its collection of notes it pushes the list to NoteDialog through a public reference in MapFrame. There were already some other dialogs that did something similar. The other way I thought about doing it was to make a change listener interface and then have NoteDialog register itself with NoteLayer to receive updates. Wouldn't be hard to do and would let other things listen for note changes as well. I can't really think of much that would need it off the top of my head but who knows.

I think both is fine. Probably some day a Plugin will want to register for Note changes, but we can add it then.

The slightly more complicated interaction is note selections. Notes can be selected in both NoteDialog and NoteLayer so they both have to communicate to each other. Layer to dialog interaction is done through the public reference in MapFrame again. For the dialog to talk to the layer, it registers itself to listen for layer changes and if a note layer is created, keeps a pointer to it until the layer is removed. This is also how the dialog interacts with the layer to modify notes. This is one instance of only being able to support a single layer.

Maybe a separate class as content manager for both?

[ToeBee]

Replying to bastiK:

Replying to ToeBee:

Time for another big notes patch! Some comments:

Icons

• Copy images/notes/note_open_24x24.png to images/dialogs/notes.png
• Copy images/notes/note_new_24x24.png to images/mapmode/addnote.png

We usually don't include images twice, because it is a waste of space.

Agreed but unless I am missing something, the dialog and map mode constructors are hard-coded to look for their icon images in these subdirectories so I can't point them at the files in /notes/. I guess maybe all of the note icons could live in the dialogs directory and I could reference them there for rendering in the note layer. I think there would still have to be a second copy of the new note icon in the mapmode directory though. I'm not actually using the icon in the mapmode directory right now since I'm not adding the button to any menus or the left side bar. The feature is only accessed through the button in the note dialog where the icon is specified in the button, not the map mode. But JOSM throws an error if there isn't a mapmode icon defined, even if it isn't actually displayed anywhere.

The slightly more complicated interaction is note selections. Notes can be selected in both NoteDialog and NoteLayer so they both have to communicate to each other. Layer to dialog interaction is done through the public reference in MapFrame again. For the dialog to talk to the layer, it registers itself to listen for layer changes and if a note layer is created, keeps a pointer to it until the layer is removed. This is also how the dialog interacts with the layer to modify notes. This is one instance of only being able to support a single layer.

Maybe a separate class as content manager for both?

Hmm perhaps. I guess it would be kind of like the DataSet class for OSM data. It could act as the authoritative holder of the note list instead of NoteLayer and then notify both NoteLayer and NoteDialog when things change. It could also hold the functions to perform operations on notes (new note, comment, close, reopen) instead of those being in NoteLayer like they are now. I will think about this some more when I get home tonight.

[Don-vip]

As there are many strings to translate this one is for the next milestone as we are in stabilization phase.

[bastiK]

Replying to ToeBee:

Replying to bastiK:

Replying to ToeBee:

Time for another big notes patch! Some comments:

Icons

• Copy images/notes/note_open_24x24.png to images/dialogs/notes.png
• Copy images/notes/note_new_24x24.png to images/mapmode/addnote.png

We usually don't include images twice, because it is a waste of space.

Agreed but unless I am missing something, the dialog and map mode constructors are hard-coded to look for their icon images in these subdirectories so I can't point them at the files in /notes/. I guess maybe all of the note icons could live in the dialogs directory and I could reference them there for rendering in the note layer.

Yes, for example. Better change dialog and map mode classes than having duplicate images. You can also try to start the image path with "../".

I think there would still have to be a second copy of the new note icon in the mapmode directory though. I'm not actually using the icon in the mapmode directory right now since I'm not adding the button to any menus or the left side bar. The feature is only accessed through the button in the note dialog where the icon is specified in the button, not the map mode. But JOSM throws an error if there isn't a mapmode icon defined, even if it isn't actually displayed anywhere.

An extra mapmode without a button in the upper left panel sounds a bit strange to me, but I haven't tested it so far.

[ToeBee]

Version 2 of this patch is here. First of all, I implemented runtime scaling of the icons so there is no need for the 16 pixel versions. The default scaling algorithm was terrible but telling it to use the "smooth" algorithm seems to yield acceptable results. And I moved the icons into the dialogs directory to reduce duplication. However I was unable to get MapMode to work right. It doesn't seem to like putting ../dialogs/ in the image name. Is it worth altering MapMode over 887 bytes?

As of right now the new icon situation looks like this:

• Move images/notes/note_closed_24x24.png to images/dialogs/notes/note_closed.png
• Move images/notes/note_new_24x24.png to images/dialogs/notes/note_new.png
• Move images/notes/note_open_24x24.png to images/dialogs/notes/note_open.png
• Delete images/notes/ and all remaining images
• Copy images/dialogs/notes/note_new.png to images/mapmode/addnote.png
• Add attached file images/dialogs/notes/note_comment.png
• Add attached file images/cursor/modifier/create_note.png

You know, some version control systems actually handle binary files in patches... :)

I also pulled the note list and list manipulation code out of the layer and into its own data class. This did indeed simplify some of the event passing between the dialog and the layer. It also simplified the layer class since it is now only focused on rendering and mouse things.

[bastiK]

I think the methods in NoteData should be synchronized as it is accessed from EDT in NoteDialog and from a worker thread in DownloadNotesTask. Otherwise looks good, will be applied after next release.

[ToeBee]

You are probably right. In the current UI I don't think there is a way to cause any threading problems but that may very well change so synchronization is probably a good idea.

[bastiK]

I would appreciate if someone else could apply this patch as I don't have time for JOSM at the moment.

[Don-vip]

In 7608/josm:

fix #10554 - Add notes dialog (patch by ToeBee)