Modify

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#12462 closed enhancement (fixed)

[patch] Extend Save Layers dialog for more layer types

Reported by: holgermappt Owned by: bastiK
Priority: normal Milestone: 16.02
Component: Core Version:
Keywords: Cc: Klumbumbus

Description

This patch adds functionality that allows all layers to indicate that they contain modified data. That is done by inheriting from AbstractModifiableLayer. So far only OsmDataLayer and NoteLayer have that capability. A layer with modified data is added to the Unsaved Changes dialog. Layers indicate how they can save data by implementing one or all of the interfaces UploadToServer and SaveToFile. Layers that cannot save or upload their data are added to the dialog, but the only way to save their data is to save a session file. The dialog reflects the layer capabilities accordingly.

Changes

  • Added interfaces UploadToServer and SaveToFile. They are intended to be used by layers that are able to upload data to a server or to save changes to a file. I added them to document what a layer intends to do, not to enforce the implementation of the interfaces. It is a little bit redundant since AbstractModifiableLayer implements the interfaces and the layers that implement one of the interfaces inherit from AbstractModifiableLayer.
  • AbstractModifiableLayer:
    • Added method isUploadable(). The related isSavable() is defined in Layer. Not sure if it is better to define both in Layer because of similarities. But isUploadable() is not needed in Layer, so I defined it here.
    • Implement interfaces UploadToServer and SaveToFile.
  • NoteLayer, OsmDataLayer: Implement interfaces UploadToServer and SaveToFile. Declare that the layer can upload.
  • GeoImageLayer: Inherit from AbstractModifiableLayer, added method isModified(). None of the interfaces is implemented.
  • Main: Add layer to dialog if it requires save or upload or if it has no save or upload capability.
  • ActionFlagsTableCell: Update the panel according to layer save/upload capabilities.
  • LayerNameAndFilePathTableCell: Add file chooser only if the layer can save to a file.
  • SaveLayerInfo: Added methods isSavable() and isUploadable().
  • SaveLayersModel: Set doSaveToFile only if the layer can be saved to a file.
  • SaveLayersTableColumnModel: Make addition of empty line that aligns the Recommended Actions cell content with the Actions To Take cell dependent on actions.

Open Issues

  • If a layer with geotagged images is part of the dialog, then some table cells turn white if they receive the focus:
    • Edit the GPS data of a photo (either correlate with a GPX track or move it with the photoadjust plugin), then delete the layer. In the Unsaved Changes dialog click in the Actions To Take cell of the Geotagged Images layer, then click in the Layer Name and File Path cell. The Layer Name and File Path cell becomes white. What could cause this?

References

Attachments (5)

ExtendedSaveLayersDialog.patch (19.7 KB) - added by holgermappt 3 years ago.
12462_initial.png (31.2 KB) - added by holgermappt 3 years ago.
Initial save lavers dialog.
12462_geoimage_layer_white.png (28.5 KB) - added by holgermappt 3 years ago.
Geoimage layer name white.
12462_data_layer_white.png (28.0 KB) - added by holgermappt 3 years ago.
OSM data layer and path white.
12462_2_data_layers_white.png (31.1 KB) - added by holgermappt 3 years ago.
JOSM 9742 without patch, two data layers, layer name cell white.

Download all attachments as: .zip

Change History (20)

Changed 3 years ago by holgermappt

comment:1 Changed 3 years ago by Don-vip

Milestone: 16.02

comment:2 in reply to:  description ; Changed 3 years ago by Don-vip

Replying to holgermappt:

  • If a layer with geotagged images is part of the dialog, then some table cells turn white if they receive the focus:
    • Edit the GPS data of a photo (either correlate with a GPX track or move it with the photoadjust plugin), then delete the layer. In the Unsaved Changes dialog click in the Actions To Take cell of the Geotagged Images layer, then click in the Layer Name and File Path cell. The Layer Name and File Path cell becomes white. What could cause this?

Can you please post a screenshot for this issue?

Changed 3 years ago by holgermappt

Attachment: 12462_initial.png added

Initial save lavers dialog.

Changed 3 years ago by holgermappt

Geoimage layer name white.

Changed 3 years ago by holgermappt

Attachment: 12462_data_layer_white.png added

OSM data layer and path white.

comment:3 in reply to:  2 Changed 3 years ago by holgermappt

Replying to Don-vip:

Replying to holgermappt:

  • If a layer with geotagged images is part of the dialog, then some table cells turn white if they receive the focus

Can you please post a screenshot for this issue?

I added three screen shots. 12462_initial.png is the initial dialog. 12462_geoimage_layer_white.png is after clicking into the Actions To Take cell of the Geotagged Images layer, then click in the Layer Name and File Path cell. 12462_data_layer_white.png is after clicking into the Actions To Take cell of the Geotagged Images layer, then click in the Layer Name and File Path cell of the Data Layer.

Changed 3 years ago by holgermappt

JOSM 9742 without patch, two data layers, layer name cell white.

comment:4 Changed 3 years ago by holgermappt

I removed my patch and tried with two data layers. The Unsaved changes dialog shows the same white cell, so this is a general issue and not related to my patch:
JOSM 9742 without patch, two data layers, layer name cell white..
To reproduce: Create two new data layers with changes, Ctrl+q, click into Data Layer 1 cell, then click into Data Layer 2 cell. Is this a known issue?

Last edited 3 years ago by Don-vip (previous) (diff)

comment:5 Changed 3 years ago by Don-vip

I see you're on Linux (Ubuntu ?). Have you checked that GNOME ATK wrapper is disabled? (see #12022 comments). This component breaks Java applications in every possible way.

comment:6 in reply to:  5 Changed 3 years ago by holgermappt

Replying to Don-vip:

I see you're on Linux (Ubuntu ?). Have you checked that GNOME ATK wrapper is disabled? (see #12022 comments). This component breaks Java applications in every possible way.

I'm on openSUSE Leap, KDE, OpenJDK 8. Package java-atk-wrapper is not installed, there is no /etc/java-8-openjdk, accessibility.properties, or a file that contains assistive_technologies in /etc.

comment:7 in reply to:  4 ; Changed 3 years ago by bastiK

Replying to holgermappt:

To reproduce: Create two new data layers with changes, Ctrl+q, click into Data Layer 1 cell, then click into Data Layer 2 cell. Is this a known issue?

Not known to me, but I can reproduce on Ubuntu with ATK wrapper turned off. The problem is that LayerNameAndFilePathTableCell uses the same object as cell renderer and as cell editor. Java will launch the editor on click and afterward continues to render another cell. So during cell rendering JOSM wipes the editor component clean.

comment:8 in reply to:  7 Changed 3 years ago by holgermappt

Replying to bastiK:

The problem is that LayerNameAndFilePathTableCell uses the same object as cell renderer and as cell editor. Java will launch the editor on click and afterward continues to render another cell. So during cell rendering JOSM wipes the editor component clean.

Can that be fixed? My Java knowledge is limited, my GUI programming knowledge is close to zero. I can change my patch if I have an example that does it the right way.

Last edited 3 years ago by holgermappt (previous) (diff)

comment:9 Changed 3 years ago by bastiK

Owner: changed from team to Don-vip

We could apply the patch first, if there are no other issues, Vincent?

comment:10 Changed 3 years ago by Don-vip

Sure. On my way.

comment:11 Changed 3 years ago by Don-vip

In 9751/josm:

see #12462 - Extend Save Layers dialog for more layer types (patch by holgermappt, modified for checkstyle compliance)

comment:12 Changed 3 years ago by Don-vip

Owner: changed from Don-vip to bastiK

If you know how to fix the rendering problem :)

comment:13 Changed 3 years ago by Klumbumbus

Cc: Klumbumbus added

comment:14 Changed 3 years ago by bastiK

Resolution: fixed
Status: newclosed

In 9754/josm:

fixed: white cells in unsaved changed dialog (closes #12462)

comment:15 in reply to:  14 Changed 3 years ago by holgermappt

Replying to bastiK:

In 9754/josm:

fixed: white cells in unsaved changed dialog (closes #12462)

First I thought that it is clever to use the same object for the cell renderer and the cell editor. Turns out it wasn't clever.

The Actions To Take cell has the same issue. For layers with checkboxes in that cell it is very hard to trigger it. But the geo image layer has an empty cell that turns white, e.g. if you click into the cell and then re-size the dialog. This is a very minor issue, though.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain bastiK.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.