Del-key should delete feature #17739
Comments
|
Author Name: Giovanni Manghi (@gioman) it can be configured in "configure shortcuts" panel, but anyway I guess that it could be configured by default. |
|
Author Name: Noone Noone (Noone Noone) 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. |
|
Author Name: Antonio Locandro (Antonio Locandro) 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 |
|
Author Name: Alvaro Huarte (@ahuarte47) I propose this pull request:
|
|
Author Name: Alvaro Huarte (@ahuarte47)
|
|
Author Name: Giovanni Manghi (@gioman) are we sure that this will not clash with the "del" action used by the node tool to delete selected nodes? :) |
|
Author Name: Alvaro Huarte (@ahuarte47) Giovanni Manghi wrote:
Hi Giovanni, you're right, 'QgsMapToolNodeTool::keyReleaseEvent' catches the "del" key to remove the selected nodes of a feature. I wonder if 'delete a feature' should use "del" key and 'remove a node' user other key (e.g. Ctrl+Del). What do you think? |
|
Author Name: Giovanni Manghi (@gioman)
certainly a thing that would need to be discussed in the users/devs ML. Cheers! |
|
Author Name: Noone Noone (Noone Noone) I'm not that deep into QGIS, but aren't both with an internal state?
But please call me wrong :) |
|
Author Name: Giovanni Manghi (@gioman)
you can have a feature selected, as well as a node, at the same time, even of different features. |
|
Author Name: Alvaro Huarte (@ahuarte47) Giovanni Manghi wrote:
Someone prefers to start the discussion or leave things as they are |
|
Author Name: Alvaro Huarte (@ahuarte47) More opinions: |
|
Author Name: Alvaro Huarte (@ahuarte47) Hi, we can propose to user/dev ML several shortcut changes similar to Arcmap behavior: Arcmap 9.3:
QGIS:
Regards |
|
Author Name: Richard Duivenvoorde (@rduivenvoorde) 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) |
|
Author Name: Matthias Kuhn (@m-kuhn) 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. |
|
Author Name: Nathan Woodrow (@NathanW2) I hate confirmation dialogs so a +1 for me to not have one. The undo is enough if something goes wrong. |
|
Author Name: Alvaro Huarte (@ahuarte47) We can propose a configurable option:
(+) Use a new global setting: !AskForConfirmation.jpg! Regards
|
|
Author Name: Nathan Woodrow (@NathanW2) 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. |
|
Author Name: Alvaro Huarte (@ahuarte47) Nathan Woodrow wrote:
Ok, I agree, then I request for opinions in user/dev mailing list:
Thanks! |
|
Author Name: Nathan Woodrow (@NathanW2) 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. |
|
Author Name: Alvaro Huarte (@ahuarte47) Nathan Woodrow wrote:
Then I propose to ML...
I must check if this key is already used.
|
|
Author Name: Alvaro Huarte (@ahuarte47) Nathan Woodrow wrote:
Hi Nathan, what do you think to use '-' instead 'backspace' ? Can be more coherent for remove a node. |
|
Author Name: Nathan Woodrow (@NathanW2) Backspace already deletes a node when drawing a feature. I think backspace should delete a node too. |
|
Author Name: Alvaro Huarte (@ahuarte47) Nathan Woodrow wrote:
ok, Thanks! |
|
Author Name: Alvaro Huarte (@ahuarte47) Nathan Woodrow wrote:
Hi Nathan, I updated the pull request with our shortcut changes. I am waiting some "serious" objection from ML :-) Best regards! |
|
Author Name: Alvaro Huarte (@ahuarte47) Pull request updated:
|
|
Author Name: Richard Duivenvoorde (@rduivenvoorde) 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. |
|
Author Name: Nathan Woodrow (@NathanW2) 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. |
|
Author Name: Alvaro Huarte (@ahuarte47) 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 ?
Best regards |
|
Author Name: Nathan Woodrow (@NathanW2) Alvaro Huarte wrote:
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: Edit workflow 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. |
|
Author Name: Alvaro Huarte (@ahuarte47) 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):
Regards |
|
Author Name: Alvaro Huarte (@ahuarte47) Hi, there are new proposed improvements in 'node tool' to select and delete nodes of current feature in more agile way. I guess I should create a new feature request with only these improvements. Regards |
|
Author Name: Noone Noone (Noone Noone) 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? |
|
Author Name: Alvaro Huarte (@ahuarte47) noone noone wrote:
The QGIS user manual contains a keymap reference: For editing tools (It includes shortcuts too): I thought I understood that a QGIS member will document the new keys of this issue in the manual. |
|
Author Name: Alvaro Huarte (@ahuarte47) Hi, summarizing, We have implemented several shortcut changes in QGIS desktop application in order to make more agile editing geometries: QgisApp / MapCanvas:
Node tool:
"PR 1010":#1010 contains these changes. Thank your very much to Richard, Nyall, Nathan, Matthias,... for your advices! |
|
Author Name: Alvaro Huarte (@ahuarte47) "pull request 1010":#1010 merged :-)
|
|
Author Name: Noone Noone (Noone Noone) Thanks @ALL :) |
|
Author Name: Alvaro Huarte (@ahuarte47)
|
|
Author Name: Etienne Tourigny (@etiennesky) 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.
|
|
Author Name: Jonathan Moules (Jonathan Moules) Etienne Tourigny wrote:
+1 |
|
Author Name: Alvaro Huarte (@ahuarte47) There was a long discussion about it in (#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 |
|
Author Name: Etienne Tourigny (@etiennesky) 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! |
|
Author Name: Larry Shaffer (Larry Shaffer) Etienne Tourigny wrote:
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. |
|
Author Name: Etienne Tourigny (@etiennesky) Larry Shaffer wrote:
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. |
|
Author Name: Richard Duivenvoorde (@rduivenvoorde) 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? |
|
Author Name: Jürgen Fischer (@jef-n) Fixed in changeset "e9f808ad0fe780f954a89c17397f6d4f2959e56b".
|
|
Author Name: Alvaro Huarte (@ahuarte47) 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 |
|
Author Name: Alvaro Huarte (@ahuarte47) Richard Duivenvoorde wrote:
+1 |
|
Author Name: Borys Jurgiel (@borysiasty) 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. |
|
Author Name: Nathan Woodrow (@NathanW2) 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 :) |
|
Author Name: Etienne Tourigny (@etiennesky) 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. |
|
Author Name: Denis Rouzaud (@3nids) 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. |
|
Author Name: Etienne Tourigny (@etiennesky) see my PR that implements basic support for undo of layer and group removal) |
qgib
added
Feature Request
GUI/UX
Author Name: Noone Noone (Noone Noone)
Original Redmine Issue: 9094
Redmine category:gui
Assignee: Alvaro Huarte
Most users would expect that Del key will erase the current selected feature, if you are in edit mode. Please fullfil this semi-standard.
Related issue(s): #18311 (relates)
Redmine related issue(s): 9769
The text was updated successfully, but these errors were encountered: