Skip to content

Commit

Permalink
Node tool: don't select another vertex after deleting a vertex (not u…
Browse files Browse the repository at this point in the history
…ser-friendly in click-click mode)
  • Loading branch information
mhugent committed Sep 8, 2015
1 parent fe9461b commit a2f0763
Showing 1 changed file with 0 additions and 1 deletion.
1 change: 0 additions & 1 deletion src/app/nodetool/qgsmaptoolnodetool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,6 @@ void QgsMapToolNodeTool::keyPressEvent( QKeyEvent* e )
return;

mSelectedFeature->deleteSelectedVertexes();
safeSelectVertex( firstSelectedIndex );
mCanvas->refresh();

// Override default shortcut management in MapCanvas
Expand Down

10 comments on commit a2f0763

@3nids
Copy link
Member

@3nids 3nids commented on a2f0763 Sep 8, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still have to test but :)

@nyalldawson
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mhugent @3nids unfortunately this is a big step backwards. Check out http://youtu.be/iiAKI0W8mgI for a demonstration of node deletion in 2.8 vs master. In 2.8 you could select a node, and then hit the delete key multiple times to progressively delete nodes in a sequential order. Now, you can only delete a single node, then you have to manually select the next node using the mouse, hit delete, select using mouse, etc... It's a big regression and will make editing quite painful.

I think a intermediate "selected but not moving" state needs to be added. So deleting a vertex makes the next vertex selected and ready for keyboard interaction (deletion or navigation using < > ), but NOT currently being moved. If you want to move the vertex you could then manually click on it to begin the move.

@3nids
Copy link
Member

@3nids 3nids commented on a2f0763 Sep 8, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactly, I was thinking also to the "selected but not moving" and it should be synchronised with the selection model of the node table views.
Therefore deleting and browsing nodes would be quite fluent!

@nyalldawson
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@3nids been thinking some more about this. Now I'm wondering if click click mode for node tool isnt the right approach. Would a better solution be:

  • click selects a point, ready for deletion via delete key or navigation via < >. Corresponding row is selected in the node table, so coordinates can be manually entered.
  • click and drag moves a node

The current ux is quite weird. You have to click to select a point, and then it automatically starts moving. To type in the node table you then have to move the mouse to the table, and all this time you have the rubberband following the cursor and going all the way across to the node table.

What do you think?

@NathanW2
Copy link
Member

@NathanW2 NathanW2 commented on a2f0763 Sep 9, 2015 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@3nids
Copy link
Member

@3nids 3nids commented on a2f0763 Sep 9, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drag and drop prevents from using advanced digitizing. I am not in favor of having the two modes as it adds more configuration (softwares should behave the same everywhere) and it would make the code much more complex.
I am quite sure that the selection model on the table view would ease deletion and selection of nodes.

@nyalldawson
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@3nids I can see how that would work for geometries with a small number of vertices, but what about polygons with thousands of nodes? (That's what I usually work with). In this case you can't use the table to select as its impossible to identify which a node from thousands of coordinate pairs, so the only way to select is by clicking with the mouse. At the moment is impossible to do this without triggering the move...

@NathanW2
Copy link
Member

@NathanW2 NathanW2 commented on a2f0763 Sep 9, 2015 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vmora
Copy link
Contributor

@vmora vmora commented on a2f0763 Sep 9, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about deselecting tools by pressing a key (e.g. escape): you press delete to delete node/nodes, then "esc" to avoid moving the current node around.

@3nids
Copy link
Member

@3nids 3nids commented on a2f0763 Sep 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's the proposed approach:

  • select a node makes it moving
  1. if you delete it before clicking, it ends digitizing and "pre-select" next node (this pre-selection is also synchro witth the table view selection model)
  2. if you move it, the "pre-selection" is kept on this one, so you can still delete it later one without any additional click.

Hence:

  • you can delete node one by one very quickly as it was before (no regression)
  • behavior of node selection is same as right now
  • advanced digitizing is available when needed
  • editing the node with coordinates is possible in the table view both in selection and pre-selection mode

Please sign in to comment.