Modify

Opened 2 years ago

Closed 5 months ago

#13467 closed enhancement (fixed)

Add a new DataSelectionListener and use it to manage the selection.

Reported by: michael2402 Owned by: team
Priority: normal Milestone: 18.06
Component: Core Version:
Keywords: gsoc-core Cc: Don-vip, bastiK, stoecker

Description (last modified by michael2402)

diff, patch

Commits:

  1. 626c26f: Add a new DataSelectionListener and use it to manage the selection.

Stats:

 .../josm/data/osm/DataSelectionListener.java       | 291 ++++++++++++++++++++
 src/org/openstreetmap/josm/data/osm/DataSet.java   | 302 +++++++++------------
 2 files changed, 422 insertions(+), 171 deletions(-)

Attachments (2)

patch-dataset-selection-listener.patch (28.9 KB) - added by michael2402 2 years ago.
patch-13467.patch (39.3 KB) - added by michael2402 19 months ago.

Download all attachments as: .zip

Change History (33)

Changed 2 years ago by michael2402

comment:1 Changed 2 years ago by Don-vip

same question regarding travis status ("build failing")

comment:2 Changed 2 years ago by michael2402

Description: modified (diff)

Same answer.

I'll have to get travis fixed first, problem is not with the patch. I'm currently working on getting that fixed (finding out why tests are failing, fixing warnings, ...)

comment:3 Changed 2 years ago by Don-vip

Milestone: 16.09

comment:4 Changed 2 years ago by simon04

Milestone: 16.0916.10

comment:5 Changed 2 years ago by simon04

Milestone: 16.1016.11

Milestone renamed

comment:6 Changed 2 years ago by bastiK

Shouldn't this patch SelectionEventManager.java?

comment:7 Changed 2 years ago by Don-vip

Milestone: 16.1116.12

Milestone renamed

comment:8 Changed 23 months ago by Don-vip

Milestone: 16.1217.01

comment:9 Changed 22 months ago by Don-vip

Milestone: 17.0117.02

comment:10 Changed 21 months ago by Don-vip

Milestone: 17.0217.03

comment:11 Changed 20 months ago by Don-vip

Milestone: 17.0317.04

comment:12 Changed 19 months ago by Don-vip

Milestone: 17.0417.05

Changed 19 months ago by michael2402

Attachment: patch-13467.patch added

comment:13 Changed 19 months ago by michael2402

Updated the patch to the current version.

There are still some calls to fireSelectionChanged() of which I did not figure out why they are there. The selection is completely handled by the DataSet, so there should be no need for this.

I'd also suggest to replace all uses of the static DataSet.addSelectionListener() using SelectionEventManager.getInstance().addSelectionListener(). This now tracks edit layer changes.

comment:14 Changed 18 months ago by michael2402

In 12113/josm:

See #13467: Preserve order of removed / added fields for selection events.

comment:15 Changed 18 months ago by Don-vip

Milestone: 17.0517.06

comment:16 Changed 17 months ago by Don-vip

Milestone: 17.0617.07

comment:17 Changed 16 months ago by Don-vip

Milestone: 17.0717.08

comment:18 Changed 15 months ago by bastiK

Milestone: 17.0817.09

comment:19 Changed 14 months ago by Don-vip

Milestone: 17.0917.10

comment:20 Changed 13 months ago by bastiK

It appears, most (if not all) of the patch has been committed in [12048]. Is this ticked fixed?

comment:21 Changed 13 months ago by michael2402

Summary: [Patch] Add a new DataSelectionListener and use it to manage the selection.Add a new DataSelectionListener and use it to manage the selection.

I applied the patch.

So the new interface is already in trunk.

There are still many places using the old SelectionChangedListener. We should decide to drop it completely (which would be what I prefer) or to keep it.

comment:22 in reply to:  21 ; Changed 13 months ago by bastiK

Replying to michael2402:

I applied the patch.

So the new interface is already in trunk.

Okay, good to know. :)

There are still many places using the old SelectionChangedListener. We should decide to drop it completely (which would be what I prefer) or to keep it.

DatSet.addSelectionListener(SelectionChangedListener) does the same as SelectionEventManager.getInstance().addSelectionListener(DataSelectionListener) so it is redundant, right?

comment:23 in reply to:  22 Changed 13 months ago by michael2402

Replying to bastiK:

There are still many places using the old SelectionChangedListener. We should decide to drop it completely (which would be what I prefer) or to keep it.

DatSet.addSelectionListener(SelectionChangedListener) does the same as SelectionEventManager.getInstance().addSelectionListener(DataSelectionListener) so it is redundant, right?

Sort of.
DataSelectionListener is more advanced and will tell you more about the actual selection changes.
And it will only fire on changes of the edit layer.
Aditionally, it will fire when the edit layer was changed and the selection was changed due to the edit layer change (it fires a unselect old - select new). So components to not need to track the edit layer themselves (solved some bugs in the UI where it did not refresh correctly).

comment:24 Changed 13 months ago by Don-vip

Milestone: 17.1017.11

comment:25 Changed 12 months ago by Don-vip

Milestone: 17.1117.12

comment:26 Changed 11 months ago by Don-vip

Milestone: 17.1218.01

comment:27 Changed 10 months ago by Don-vip

Milestone: 18.0118.02

comment:28 Changed 9 months ago by Don-vip

Milestone: 18.0218.03

comment:29 Changed 8 months ago by Don-vip

Milestone: 18.03

comment:30 Changed 5 months ago by Don-vip

Milestone: 18.06

comment:31 Changed 5 months ago by Don-vip

Resolution: fixed
Status: newclosed

In 13925/josm:

fix #13467 - use DataSelectionListener everywhere. Deprecate SelectionChangedListener

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
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.