Modify

Opened 8 years ago

Closed 6 years 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 8 years ago.
patch-13467.patch (39.3 KB ) - added by michael2402 7 years ago.

Download all attachments as: .zip

Change History (33)

by michael2402, 8 years ago

comment:1 by Don-vip, 8 years ago

same question regarding travis status ("build failing")

comment:2 by michael2402, 8 years ago

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 by Don-vip, 8 years ago

Milestone: 16.09

comment:4 by simon04, 8 years ago

Milestone: 16.0916.10

comment:5 by simon04, 8 years ago

Milestone: 16.1016.11

Milestone renamed

comment:6 by bastiK, 7 years ago

Shouldn't this patch SelectionEventManager.java?

comment:7 by Don-vip, 7 years ago

Milestone: 16.1116.12

Milestone renamed

comment:8 by Don-vip, 7 years ago

Milestone: 16.1217.01

comment:9 by Don-vip, 7 years ago

Milestone: 17.0117.02

comment:10 by Don-vip, 7 years ago

Milestone: 17.0217.03

comment:11 by Don-vip, 7 years ago

Milestone: 17.0317.04

comment:12 by Don-vip, 7 years ago

Milestone: 17.0417.05

by michael2402, 7 years ago

Attachment: patch-13467.patch added

comment:13 by michael2402, 7 years ago

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 by michael2402, 7 years ago

In 12113/josm:

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

comment:15 by Don-vip, 7 years ago

Milestone: 17.0517.06

comment:16 by Don-vip, 7 years ago

Milestone: 17.0617.07

comment:17 by Don-vip, 7 years ago

Milestone: 17.0717.08

comment:18 by bastiK, 7 years ago

Milestone: 17.0817.09

comment:19 by Don-vip, 7 years ago

Milestone: 17.0917.10

comment:20 by bastiK, 7 years ago

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

comment:21 by michael2402, 7 years ago

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.

in reply to:  21 ; comment:22 by bastiK, 7 years ago

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?

in reply to:  22 comment:23 by michael2402, 7 years ago

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 by Don-vip, 6 years ago

Milestone: 17.1017.11

comment:25 by Don-vip, 6 years ago

Milestone: 17.1117.12

comment:26 by Don-vip, 6 years ago

Milestone: 17.1218.01

comment:27 by Don-vip, 6 years ago

Milestone: 18.0118.02

comment:28 by Don-vip, 6 years ago

Milestone: 18.0218.03

comment:29 by Don-vip, 6 years ago

Milestone: 18.03

comment:30 by Don-vip, 6 years ago

Milestone: 18.06

comment:31 by Don-vip, 6 years ago

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. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.