Feature request #9094

Del-key should delete feature

Added by Noone Noone over 5 years ago. Updated almost 5 years ago.

Status:Closed
Priority:Normal
Assignee:Alvaro Huarte
Category:GUI
Pull Request or Patch supplied:No Resolution:fixed/implemented
Easy fix?:No

Description

Most users would expect that Del key will erase the current selected feature, if you are in edit mode. Please fullfil this semi-standard.

AskForConfirmation.jpg (38.6 KB) Alvaro Huarte, 2013-11-27 01:27 PM


Related issues

Related to QGIS Application - Feature request #9769: Feature removal doesn't require confirmation Closed 2014-03-12

Associated revisions

Revision e9f808ad
Added by Jürgen Fischer almost 5 years ago

make confirmation on layer deletion optional (fixes #9094)

History

#1 Updated by Giovanni Manghi over 5 years ago

it can be configured in "configure shortcuts" panel, but anyway I guess that it could be configured by default.

#2 Updated by Noone Noone over 5 years ago

IMHO it should be set by default. Think on yourself from an enduser POV, how often do you alter the shortcuts of an app and did you enjoy it? IMHO wise defaults that allow a user just to start working and finishing his tasks, are an essential step in good usability.

#3 Updated by Antonio Locandro about 5 years ago

I agree, I have been using QGIS for some time and didn't know you could just configure the shortcut to do that. By default it should delete selected features, if power users would like to change behavior they can do it from the shortcut menu, but IMHO most users would expect selecting a feature and pressing DEL to actually delete a feature

#4 Updated by Alvaro Huarte about 5 years ago

  • Assignee set to Alvaro Huarte

I propose this pull request:
https://github.com/qgis/QGIS/pull/1010

#5 Updated by Alvaro Huarte about 5 years ago

  • Assignee deleted (Alvaro Huarte)

#6 Updated by Giovanni Manghi about 5 years ago

are we sure that this will not clash with the "del" action used by the node tool to delete selected nodes? :)

#7 Updated by Alvaro Huarte about 5 years ago

Giovanni Manghi wrote:

are we sure that this will not clash with the "del" action used by the node tool to delete selected nodes? :)

Hi Giovanni, you're right, 'QgsMapToolNodeTool::keyReleaseEvent' catches the "del" key to remove the selected nodes of a feature.
It is not defined in a configurable shortcut, it is hard-coded.

I wonder if 'delete a feature' should use "del" key and 'remove a node' user other key (e.g. Ctrl+Del).
But it changes the current behavior.

What do you think?

#8 Updated by Giovanni Manghi about 5 years ago

I wonder if 'delete a feature' should use "del" key and 'remove a node' user other key (e.g. Ctrl+Del).
But it changes the current behavior.

What do you think?

certainly a thing that would need to be discussed in the users/devs ML. Cheers!

#9 Updated by Noone Noone about 5 years ago

I'm not that deep into QGIS, but aren't both with an internal state?
  • If feature is selected, delete it
  • If a node within a polygone is selected, delete it

But please call me wrong :)

#10 Updated by Giovanni Manghi about 5 years ago

But please call me wrong :)

you can have a feature selected, as well as a node, at the same time, even of different features.

#11 Updated by Alvaro Huarte about 5 years ago

Giovanni Manghi wrote:

I wonder if 'delete a feature' should use "del" key and 'remove a node' user other key (e.g. Ctrl+Del).
But it changes the current behavior.

What do you think?

certainly a thing that would need to be discussed in the users/devs ML. Cheers!

Someone prefers to start the discussion or leave things as they are

#13 Updated by Alvaro Huarte about 5 years ago

Hi, we can propose to user/dev ML several shortcut changes similar to Arcmap behavior:

Arcmap 9.3:

  • Delete selected features in map canvas: DEL key without ask for confirmation.
  • Delete selected features in attribute table: DEL key without ask for confirmation.
  • Remove layer from legend: There is not shortcut, it executes from context menu
  • Remove selected node of a feature: There is not shortcut, it executes from context menu

QGIS:

  • Delete selected features in map canvas: DEL key without ask for confirmation (Now QGIS ask for confirmation first).
  • Delete selected features in attribute table: DEL key without ask for confirmation.
  • Remove layer from legend: Ctrl+D with ask for confirmation (Now QGIS does not ask for confirmation).
  • Remove selected node of a feature: Ctrl+DEL without ask for confirmation.

Regards

#14 Updated by Richard Duivenvoorde about 5 years ago

very much agree on the proposal above.

at the moment you are being asked for removal of a feature (while you can easily undo the removal) and the removal of a (huge) layer is instantaneous and can NOT be undone...

(The use of ctrl as a helper key is in line with a context menu on a mac. There the context menu pops up with a ctrl-mouse click)

#15 Updated by Matthias Kuhn about 5 years ago

How about sticking to the confirmation with DEL, and add a shortcut SHIFT + DEL for no-confirmation delete? That's what I'm used to from filebrowsers. On the other hand, an undo option should be as good as a confirmation dialog.

#16 Updated by Nathan Woodrow about 5 years ago

I hate confirmation dialogs so a +1 for me to not have one. The undo is enough if something goes wrong.

#17 Updated by Alvaro Huarte about 5 years ago

We can propose a configurable option:

  • Delete selected features in map canvas: DEL key (+).
  • Delete selected features in attribute table: DEL key (+).
  • Remove layer from legend: Ctrl+D with ask for confirmation (Now QGIS does not ask for confirmation).
  • Remove selected node of a feature: Ctrl+DEL without ask for confirmation.

(+) Use a new global setting:

Regards

#18 Updated by Nathan Woodrow about 5 years ago

No not another option please. There is no need in this case. Like I, and others, have said undo and redo are the reason to have no confirm dialog.

In a perfect world I would like to have undo on everything so you could undo removing a layer, undo a style change, undo a label change.

#19 Updated by Alvaro Huarte about 5 years ago

Nathan Woodrow wrote:

No not another option please. There is no need in this case. Like I, and others, have said undo and redo are the reason to have no confirm dialog.

In a perfect world I would like to have undo on everything so you could undo removing a layer, undo a style change, undo a label change.

Ok, I agree, then I request for opinions in user/dev mailing list:

  • Delete selected features in map canvas: DEL key without ask for confirmation (Now QGIS ask for confirmation first).
  • Delete selected features in attribute table: DEL key without ask for confirmation (Now QGIS ask for confirmation first).
  • Remove layer from legend: Ctrl+D with ask for confirmation (Now QGIS does not ask for confirmation).
  • Remove selected node of a feature: Ctrl+DEL without ask for confirmation.

Thanks!

#20 Updated by Nathan Woodrow about 5 years ago

I think those options work fine. If we have undo for a action 1) 2) 4) then there is no need to confirm, and in the case of 3) we don't so confirming there is good.

We also backspace for deleting a node which I like. I'm not sure I'm a fan of having Ctrl+D for deleting a node because it needs two hands, or at least a stretch. I think just having backspace for this would be fine.

If you make a pull request I will merge it.

#21 Updated by Alvaro Huarte about 5 years ago

  • Assignee set to Alvaro Huarte

Nathan Woodrow wrote:

I think those options work fine. If we have undo for a action 1) 2) 4) then there is no need to confirm, and in the case of 3) we don't so confirming there is good.

We also backspace for deleting a node which I like. I'm not sure I'm a fan of having Ctrl+D for deleting a node because it needs two hands, or at least a stretch. I think just having backspace for this would be fine.

If you make a pull request I will merge it.

Then I propose to ML...

  • Delete selected features in map canvas: DEL key without ask for confirmation (Now QGIS ask for confirmation first).
  • Delete selected features in attribute table: DEL key without ask for confirmation (Now QGIS ask for confirmation first).
  • Remove layer from legend: Ctrl+D with ask for confirmation (Now QGIS does not ask for confirmation).
  • Remove selected node of a feature: 'backspace' without ask for confirmation.

I must check if this key is already used.

#22 Updated by Alvaro Huarte about 5 years ago

Nathan Woodrow wrote:

I think those options work fine. If we have undo for a action 1) 2) 4) then there is no need to confirm, and in the case of 3) we don't so confirming there is good.

We also backspace for deleting a node which I like. I'm not sure I'm a fan of having Ctrl+D for deleting a node because it needs two hands, or at least a stretch. I think just having backspace for this would be fine.

If you make a pull request I will merge it.

Hi Nathan, what do you think to use '-' instead 'backspace' ? Can be more coherent for remove a node.

#23 Updated by Nathan Woodrow about 5 years ago

Backspace already deletes a node when drawing a feature. I think backspace should delete a node too.

#24 Updated by Alvaro Huarte about 5 years ago

Nathan Woodrow wrote:

Backspace already deletes a node when drawing a feature. I think backspace should delete a node too.

ok, Thanks!

#25 Updated by Alvaro Huarte about 5 years ago

Nathan Woodrow wrote:

I think those options work fine. If we have undo for a action 1) 2) 4) then there is no need to confirm, and in the case of 3) we don't so confirming there is good.

We also backspace for deleting a node which I like. I'm not sure I'm a fan of having Ctrl+D for deleting a node because it needs two hands, or at least a stretch. I think just having backspace for this would be fine.

If you make a pull request I will merge it.

Hi Nathan, I updated the pull request with our shortcut changes. I am waiting some "serious" objection from ML :-)
https://github.com/qgis/QGIS/pull/1010

Best regards!

#26 Updated by Alvaro Huarte about 5 years ago

Pull request updated:

  • Delete selected features in map canvas: DEL key without ask for confirmation.
  • Delete selected features in attribute table: DEL key without ask for confirmation.
  • Remove layer from legend: D key with ask for confirmation, Ctrl+D key without ask for confirmation.
  • Remove selected node of a feature: 'backspace' key without ask for confirmation.

#27 Updated by Richard Duivenvoorde about 5 years ago

I do not agree with the backspace as use for removing a node. Is there a (technical) reason to make it different? What about the mac users, did someone not say they do not have one of the keys?

Plz let us keep the different keys to use as low as possible. Delete (or even better: both Delete AND backspace) are ok for me to do the same things.

I think the conclusion from the ML (which is my choice too ;-) ) is to not use single keys. So I would prefer to change ctrl-D to action with confirmation.

#28 Updated by Nathan Woodrow about 5 years ago

I have no issue using Delete for deleting a node too. When you have the node tool active and a node selected then it makes sense.

#29 Updated by Alvaro Huarte about 5 years ago

Hi, We can not use Delete for deleting a node if we use Delete for delete selected features too, as Giovanni said above, you can have a feature selected, as well as a node, at the same time, even of different features.

Using the advice of Richard and other opiniones of ML, what you think of this proposal ?

  • Delete selected features in map canvas: DEL key without ask for confirmation (It is an undoable action).
  • Delete selected features in attribute table: DEL key without ask for confirmation (Now QGIS uses Ctrl+D with confirmation).
  • Remove layer from legend: D key with ask for confirmation (To avoid single keys as ML say), Ctrl+D key with ask for confirmation (Because of it is not an undoable action).
  • Remove selected node of a feature (adding or editing a feature): Ctrl+D key without ask for confirmation.

Best regards

#30 Updated by Nathan Woodrow about 5 years ago

Alvaro Huarte wrote:

Hi, We can not use Delete for deleting a node if we use Delete for delete selected features too, as Giovanni said above, you can have a feature selected, as well as a node, at the same time, even of different features.

Best regards

While it looks like an issue I don't really think it is. If you have the node tool selected I really don't think you are in current workflow of deleting a feature you are in a edit workflow.

Select workflow:
Click Select Tool -> Select Features -> Delete

Edit workflow
Click node tool -> add/remove nodes

I just don't see someone doing:

Click Select Tool -> Select Features -> Click Node tool -> Add/Remove nodes -> Delete Features

For me having a Ctrl+D just to delete a node seems like to much work. To be honest I remap all my major keys, delete feature, edit mode, node tool, etc, all around ASDEWQ because then I don't have to move my hand and it's all single keys but I can understand how this doesn't make sense for everyone.

#31 Updated by Alvaro Huarte about 5 years ago

Nathan Woodrow wrote:

Alvaro Huarte wrote:

Hi, We can not use Delete for deleting a node if we use Delete for delete selected features too, as Giovanni said above, you can have a feature selected, as well as a node, at the same time, even of different features.

Best regards

While it looks like an issue I don't really think it is. If you have the node tool selected I really don't think you are in current workflow of deleting a feature you are in a edit workflow.

Select workflow:
Click Select Tool -> Select Features -> Delete

Edit workflow
Click node tool -> add/remove nodes

I just don't see someone doing:

Click Select Tool -> Select Features -> Click Node tool -> Add/Remove nodes -> Delete Features

For me having a Ctrl+D just to delete a node seems like to much work. To be honest I remap all my major keys, delete feature, edit mode, node tool, etc, all around ASDEWQ because then I don't have to move my hand and it's all single keys but I can understand how this doesn't make sense for everyone.

Hi Nathan, I agree with the workflow what you say, but now 'QgisApp::deleteSelected' ignores the active tool. Right seems to only run when a selection tool is active. You think I should implement it this way?

-> Del key only delete selected features in map canvas when one selection tool is active.

#32 Updated by Alvaro Huarte about 5 years ago

Alvaro Huarte wrote:

Nathan Woodrow wrote:

Alvaro Huarte wrote:

Hi, We can not use Delete for deleting a node if we use Delete for delete selected features too, as Giovanni said above, you can have a feature selected, as well as a node, at the same time, even of different features.

Best regards

While it looks like an issue I don't really think it is. If you have the node tool selected I really don't think you are in current workflow of deleting a feature you are in a edit workflow.

Select workflow:
Click Select Tool -> Select Features -> Delete

Edit workflow
Click node tool -> add/remove nodes

I just don't see someone doing:

Click Select Tool -> Select Features -> Click Node tool -> Add/Remove nodes -> Delete Features

For me having a Ctrl+D just to delete a node seems like to much work. To be honest I remap all my major keys, delete feature, edit mode, node tool, etc, all around ASDEWQ because then I don't have to move my hand and it's all single keys but I can understand how this doesn't make sense for everyone.

Hi Nathan, I agree with the workflow what you say, but now 'QgisApp::deleteSelected' ignores the active tool. Right seems to only run when a selection tool is active. You think I should implement it this way?

-> Del key only delete selected features in map canvas when one selection tool is active.

If you agree with this behavior, I must catch the 'Del' key in all 'QgsMapToolSelectXXX' tools of qgis. There is not problem.

#33 Updated by Nathan Woodrow about 5 years ago

Well I think you only need to catch it in node tool, everything else just ignores it.

#34 Updated by Alvaro Huarte about 5 years ago

Nathan Woodrow wrote:

Well I think you only need to catch it in node tool, everything else just ignores it.

Hi Nathan, I think more coherent catch Del key in selection tools, other tools (qgsmaptoolcapture, qgsmaptoolannotation, qgsgrassmapcalc) use this key in this way, when each tool is active, the Del key executes its own action.

#35 Updated by Alvaro Huarte about 5 years ago

Hi, I have tried to bring together all the comments and I have updated the pull request (https://github.com/ahuarte47/QGIS/compare/Issue_9094):

  • Selection tools (With point, rectangle, circle...) delete selected features with 'backspace' or 'delete' key without confirmation (It is an undoable action).
    Also, a new status message notify the number of features deleted.
  • DEL key in attribute table delete the seleted features similarly.
  • Node tool, or capture tool, remove a node with 'backspace' or 'delete' key.
  • TOC remove selected layer[s] with Ctrl+D with ask for confirmation (It is not an undoable action).
    Also, a new status message notify the number of layers removed.

Regards

#36 Updated by Alvaro Huarte about 5 years ago

Hi, there are new proposed improvements in 'node tool' to select and delete nodes of current feature in more agile way.

https://github.com/qgis/QGIS/pull/1010#issuecomment-29672086

I guess I should create a new feature request with only these improvements.
You think that might be useful?

Regards

#37 Updated by Noone Noone about 5 years ago

Thanks for discussing and fixing this issue :) But IMHO this shows that QGIS currently lacks a keymap reference to avoid such conflicts/inconsistencies. What do you think about that or is there already such kind of overview?

#38 Updated by Alvaro Huarte about 5 years ago

noone noone wrote:

Thanks for discussing and fixing this issue :) But IMHO this shows that QGIS currently lacks a keymap reference to avoid such conflicts/inconsistencies. What do you think about that or is there already such kind of overview?

The QGIS user manual contains a keymap reference:
http://www.qgis.org/en/docs/user_manual/introduction/qgis_gui.html

For editing tools (It includes shortcuts too):
http://docs.qgis.org/testing/en/docs/user_manual/working_with_vector/editing_geometry_attributes.html

I thought I understood that a QGIS member will document the new keys of this issue in the manual.
Regards

#39 Updated by Alvaro Huarte about 5 years ago

Hi, summarizing, We have implemented several shortcut changes in QGIS desktop application in order to make more agile editing geometries:

QgisApp / MapCanvas:

  • By default, 'Delete' and 'Backspace' keys are used to delete the current selected features. Now, no message for confirmation is showed (It is an undoable action), but a new status message shows the number of deleted features. 'Delete' key does the same in attribute table dialog, as heretofore.

To avoid key conflicts, we have implemented that active MapTool can override the default behavior of shortcut keys.

  • 'Ctrl+D' removes seleted layer[s] in legend. Now, it ask for confirmation because of it is not an undoable action.

Node tool:

  • 'Delete' and 'Backspace' keys delete the selected node[s]. This tool, as 'capture tool', ... override the default behavior of these keys. Also, it is automatically selected the adjacent node to last removed node.
  • '>' key activates the next node to current selected node.
  • '<' key activates the previous node to current selected node.

PR 1010 contains these changes.

Thank your very much to Richard, Nyall, Nathan, Matthias,... for your advices!
Best regards

#40 Updated by Alvaro Huarte about 5 years ago

  • % Done changed from 0 to 100
  • Status changed from Open to Resolved
  • Resolution set to fixed/implemented

#41 Updated by Noone Noone about 5 years ago

Thanks @all :)

#42 Updated by Alvaro Huarte about 5 years ago

  • Status changed from Resolved to Closed

#43 Updated by Etienne Tourigny almost 5 years ago

  • Status changed from Closed to Open
  • Target version set to Version 2.4

I find it rather annoying that I have to confirm to remove a layer from the canvas. If I explicitly click the context menu "Remove" or use the hot-key ctrl-d to remove the layer, I know what I am doing. It is also easy to add it back if I removed it by mistake.

There should be an option to "ask confirmation before removing layer" in the Canvas & Legend options page.

#44 Updated by Jonathan Moules almost 5 years ago

Etienne Tourigny wrote:

I find it rather annoying that I have to confirm to remove a layer from the canvas. If I explicitly click the context menu "Remove" or use the hot-key ctrl-d to remove the layer, I know what I am doing. It is also easy to add it back if I removed it by mistake.

There should be an option to "ask confirmation before removing layer" in the Canvas & Legend options page.

+1

#45 Updated by Alvaro Huarte almost 5 years ago

There was a long discussion about it in (https://github.com/qgis/QGIS/pull/1010).

I can understand that action from the context menu does not need confirmation, I can skip this confirmation if it is decided, but about ctrl-d key, think about the case that the user has defined a complex symbology or other settings of the layer.

Alvaro

#46 Updated by Etienne Tourigny almost 5 years ago

Agreed - simple solution is to always ask confirmation with shortcut, but never with context menu. It that ok?

sorry I stepped into this discussion a little late!

#47 Updated by Larry Shaffer almost 5 years ago

Etienne Tourigny wrote:

Agreed - simple solution is to always ask confirmation with shortcut, but never with context menu. It that ok?

This is also the easiest to implement, since the CTRL-D key sequence is already bound in the qgisapp.ui file to the Remove Layer action.

However, realize that the Duplicate Layer action is right next to the Remove Layer action in the contextual menu. A simple missed click on Duplicate Layer might cause a non-undo-able removal of the layer instead. This is against the HIG guidelines (i.e. putting an irreversible destructive command right next to a constructive command).

Remove layer could be further sequestered by adding another separator below it, but that may not look very cohesive.

A general rule of thumb for key sequences that skip confirmation dialogs is to add another modifier key to the sequence, i.e. SHIFT-CTRL-D. On Mac, this is generally the option key (which even dynamically changes the menus when pressed, e.g. Remove Layer... to Remove Layer). However, the option key is not a good cross-platform second modifier, since it often has unwanted side effects on Linux, e.g. moves window instead.

#48 Updated by Etienne Tourigny almost 5 years ago

Larry Shaffer wrote:

However, realize that the Duplicate Layer action is right next to the Remove Layer action in the contextual menu. A simple missed click on Duplicate Layer might cause a non-undo-able removal of the layer instead. This is against the HIG guidelines (i.e. putting an irreversible destructive command right next to a constructive command).

Remove layer could be further sequestered by adding another separator below it, but that may not look very cohesive.

I agree with you on the principle, but this has never happened to me before. Has any user complained that they lost information because "Remove Layer" did not ask for confirmation?

I believe this was implemented as an afterthought, and the confirmation dialog added for the shortcut.

I guess if the remove layer was separated by 2 separators (after set layer srs) it would avoid any accidental removal.

#49 Updated by Richard Duivenvoorde almost 5 years ago

I myself have (irreversably) removed wrong layers myself (especially complex styled layers, but also slow loading database layers), please see the discussion here, on the mailinglist and on the pull request.

We have made the OK button of the dialog as active, so hitting enter will do.

I agree that maybe the confirmation is a little redundant in the context menu, but still feel that a confirmation dialog is on it's place here. You can select seperate layers and delete them at once, is the confirmation/hitting enter then really a problem?

#50 Updated by Jürgen Fischer almost 5 years ago

  • Status changed from Open to Closed

#51 Updated by Alvaro Huarte almost 5 years ago

Hi Jürgen, the method 'QgisApp::removeLayer()' is called to delete layers and groups (it removes all selected nodes in legend), is why the labels have the text 'objects' no 'layers'.

Alvaro

#52 Updated by Alvaro Huarte almost 5 years ago

Richard Duivenvoorde wrote:

I myself have (irreversably) removed wrong layers myself (especially complex styled layers, but also slow loading database layers), please see the discussion here, on the mailinglist and on the pull request.

We have made the OK button of the dialog as active, so hitting enter will do.

I agree that maybe the confirmation is a little redundant in the context menu, but still feel that a confirmation dialog is on it's place here. You can select seperate layers and delete them at once, is the confirmation/hitting enter then really a problem?

+1

#53 Updated by Borys Jurgiel almost 5 years ago

It was a long discussion, and I'm happy we've finally found the solution :) There are clearly two kinds of QGIS users. Some of us use complex projects, and an incidental layer remove is a big loss. Some others, including me, use QGIS to browse dozens of layers every day, so the possibility to remove layer as easy as possible is absolutely essential. Thus, the checkbox JEF just implemented seems to me to be the only reasonable solution.

#54 Updated by Nathan Woodrow almost 5 years ago

I think the decision we made was the correct one. Removing a layer is a unrecoverable action in the sense there is no confirm or undo and nukes a lot of the work a user might have done. I have done this in the past and I can tell you it annoyed the hell out of me. Having a setting for this is fine in this case in order to support the power users but it must be enabled by default everyone else, so what Jurgen has done is fine.

Having said that, I also don't like confirm dialogs so the correct solution for this is to implement a undo action for the remove of a layer, the same could be done for undoing style changes. Having that means you can do away with the confirm dialog and just provide the user with the option to undo the action. Removing a large layer is now less of a issue with mulithreading as we can just add it back in and not have to wait or block the UI.

I'm not sure how much work there would be in creating the undo action but I know that is the correct solution. If anyone is game feel free to give it a go :)

#55 Updated by Etienne Tourigny almost 5 years ago

Nathan - I agree with everything you just said.

Undo action would be best (I was just thinking about it), but I had a glance at the code and it seems that the undo/layer stack can change with active layer (in the legend).

So how would you activate the "removed layers" undo stack, from a usability point of view? When you click somewhere on the legend? Or just have it immediately available, if user does other stuff then that undo action would be forgotten?

Also managing legend groups would be a headache, so it would be easier to just keep deleted layers and restore them to root or a new group.

#56 Updated by Denis Rouzaud almost 5 years ago

Just for the record, Martin is supposed to refactor the legend within the 2.3 dev cycle.

Anyhow, there might be a way of having an undo/redo stack for the legend.
Besides accidentally removing layers, there is also accidentally moving layers which is really annoying. It happens often to me, and I am never really sure which has been moved.

#57 Updated by Etienne Tourigny almost 5 years ago

see my PR that implements basic support for undo of layer and group removal)

https://github.com/qgis/QGIS/pull/1218

Also available in: Atom PDF