Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow DropFilterAction between same layers #292

Merged
merged 1 commit into from
Sep 24, 2019

Conversation

nprigour
Copy link
Contributor

DropFilterAction is currently allowed within the same layer. This should be avoided since it may cause some weird result

Signed-off-by: Nikolaos Pringouris nprigour@gmail.com

Signed-off-by: Nikolaos Pringouris <nprigour@gmail.com>
@fgdrf fgdrf added this to the 2.1.0 milestone Jun 26, 2018
@fgdrf
Copy link
Contributor

fgdrf commented Jun 26, 2018

@nprigour I guess your change looks reasonable. I'm wondering a bit how to test this change in running application. I'm not aware of this functionality .

Can you help here or point to a help page?

@nprigour
Copy link
Contributor Author

nprigour commented Jun 26, 2018

You can check this as follows supposing you have loaded 2 shapefiles to a map. While selection tool is active select a feature i Map editor. Then you can do one of the following actions:

  1. hold the left mouse button like doing a drag & drop and release it on the same map editor.
  2. hold the left mouse button and do a drag & drop to a layer in Layer view.
    in both cases when holding the mouse key and d&d the mouse pointer icon should transform to an icon similar to that of a shortcut.

Action 1 is the main reason for providing this PR. With the suggested fix it does nothing, while previously it caused a copy of the selected feature on the same layer resulting in duplicate features. This may seemed weird to a potential user since if you were clicking very quickly and selecting features you may accidentally copy features on the same layer.
Action 2 now results allows copying of features only between different layers and not if the drop is performed on the currently active layer which seems rational.

@fgdrf
Copy link
Contributor

fgdrf commented Jul 1, 2018

I tried the following:

  • select an object of a shapefile layer
  • selected this layer in Layers View
  • draged it onto Map View and droped it there

Finally a new layer is listed in Layers View with the same resource

Then I tried to drag the layer on the same layer within Layers View: Nothing happens.

When I use copy and paste ( layer is selected) on Map, then selected feature is copied within the same ressource (as expected)

I'm wondering how to drag and drop a layer to reproduce behavior you mentioned. I guess I miss something

@nprigour
Copy link
Contributor Author

nprigour commented Jul 1, 2018

I did not say to select a layer in layer view or d&d a layer!
What I meant is to select one (or more) feature in the Map and then while in single selection mode (make sure that 'Feature Selection' option is the active tool in the palette) do a d&d it on the same layer (in the Layer view) or on the Map itself.

@nprigour
Copy link
Contributor Author

nprigour commented Aug 9, 2018

What about this @fgdrf?
Shall we merge and close it?

@fgdrf
Copy link
Contributor

fgdrf commented Apr 25, 2019

What about this @fgdrf?
Shall we merge and close it?

I'll try to test it tomorrow again.

@fgdrf
Copy link
Contributor

fgdrf commented Apr 26, 2019

@nprigour Just tested uDig build from current state of master (without your fix) and it works quite well. I was able to copy a feature on the map-panel using Drag&Drop.

So I'm not sure if your fix is required. Can you test the scenario again? Many Thanks

@nprigour
Copy link
Contributor Author

Yes indeed feature copy is possible I do not deny it. However the reasons for the fix as explained in a previous comment (see my comment on 27 June 2018) are the following (based on my personal experience from an application we have developed):

  1. When working especially with database layers that may have unique constraints such a D&D will cause DB exceptions and rollbacks.
  2. More often than not a confused user that works on the map in single selection mode (with a feature already selected) attempts a drag action, assuming he is in multi feature selection mode where drag is perfectly valid. The consequence is that when he realizes his mistake and release the mouse these action results in a drop which causes a copy of the feature which it was not his intention. In the case there is a unique constraint on some layer field this will result in an exce;tion and rollback (possibly invalidating may other actions that may have performed. Even worst if no constraints exist it may result in an undesired feature copy which may not come to his attention. Believe me this has happened numerous times in a cadastral application we have developed based on udig.
  3. The accidental copy of feature described above may also occur if you are clicking and selecting features very quickly while in single selection mode

Based on all the above my humble opinion is that allowing such a D&D within the same layer causes more problems than the ones it solves and I strongly suggest to disable it. If copy between the same layer is desired it can still be achieved by the copy-paste combination of actions which is more or less fully controlled by the user.

@fgdrf
Copy link
Contributor

fgdrf commented Sep 24, 2019

@nprigour Should we merge it or abandon? My read of your last comment is: Let's deactivale D&D of features if source and destination layer are the same. If so, lets prepare another change (for Milestone 2.1.0 ??)

@nprigour
Copy link
Contributor Author

nprigour commented Sep 24, 2019

For me its definetely a must merge PR. Maybe we disallow D&D within the same layer but:

  1. we avoid accidental creation of features during selection mode during fast mouse clicks. Try i.e. the following: If you have a polygon layer load it on map, enable 'Feature Selection' tool from the palette and continuously left click the mouse trying to select various polygon features. Verify how easy it is to create a new feature without even noticing it!
  2. Copy of features within the same layer is still possible via copy/paste actions so the user is not banned from duplicating a feature. Rather it can do it with full awareness.

@fgdrf fgdrf merged commit 62d47fe into locationtech:master Sep 24, 2019
fgdrf pushed a commit to fgdrf/udig-platform that referenced this pull request Dec 23, 2019
Signed-off-by: Nikolaos Pringouris <nprigour@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants