This kinda harkens back to some discussions here and there with @muratmaga and others here regarding placing control points and the right-click pop-up menu. I rename and delete control points placed in the 3D view a lot, and on several occasions I’ve accidentally hit “Delete Fiducial” instead of “Delete control point”, and wind up blowing away a bunch of points. I fully realize it’s my own lack of dexterity that leads to this, but would it make sense to have a confirmation dialog for “Delete Fiducial”? Or possibly moving it so it’s not right next to “Delete control point”? If I’m the only one with this issue, it’s no big deal and I can live with it, just wondering if others have run into this and if it’s worth thinking about changing.
I don’t believe an additional confirmation box is the right solution to this issue. Right-click context menu with selection and clicking is a fairly deliberate action, so it would be a bit annoying to deliberately choose to “Delete Fiducial” and then have to click another pop-up to confirm the deliberate action to delete.
Moving the location of this action to the top of the list however, could be a nice solution to move the two options away from being directly next to each other. It would also make consistent the ordering of the control point actions versus the fiducial object actions where the “Delete” action is at the top followed by various editing of that object below it. Having it at the top is probably also consistent with it seemingly being the more commonly used action as well. See the image below of this change.
It is true, but we don’t have an undo. So a mistake of accidentally deleting a whole markup node vs a control point is very costly (that you might loose dozens or hundreds of points). While I don’t see a need for confirmation for deleting an individual point, I think the cost of error overweighs the minor annoyance, for confirming to delete a whole node.
If a confirmation dialog is added, then it should be added consistently across the application for all delete object actions. This would including right-clicking and pressing “Delete” when selecting an object to delete in a node table such as the Data module or Markups module. This just doesn’t strike me as the correct solution. It is going to be annoying to users who know what they are doing and are saying to delete and then get asked are you sure you want to delete. All this to avoid the more infrequent case of accidentally deleting.
Why? It is not like we have lots of consistency across actions in all modules?
THe screenshot you share is not reflective of the issue in hand. In markups, in the menu there are two delete operations and that the difference between control points and fiducials has been murky for users (perhaps with the exception of people who has been using Slicer since version 2) at best. In your scenario above, there is no accidental deleting (i.e., mistaking an action).
I would also second that confirmation should be added to any costly unrecoverable operations, with the option to disable the confirmation (e.g., like we do closing the slicer scene without asking to save the files, if it is a deliberate action)
This should be something attempted. Blatantly going against consistency is bad design.
If your users have some confusion between fiducials and control points have you considered creating a Slicer custom application so that you can customize the right-click context menu as you please for your users?
I would argue risking to loose data is even worse design.
We do not have a set of ‘fixed users’. A lot of ‘our users’ are biologists, who are likely to use the base SLicer application, not a customized version of slicer. Nor we have the bandwidth or resources to maintain such application. We are biologists, not software developers. Plus, this mistake is likely to affect any new Slicer user, just ‘our users’.
@hherhold Would the proposed design of moving the delete control point action to the top of the list be helpful for you? If so, I would be happy to submit that change as a new PR.
I think @jamesobutler 's suggestion of moving Delete Fiducial Node away from Delete Control Point is the “low hanging fruit” for this. Regarding the last suggestion from @muratmaga - It’s not that I don’t know what each one does, it’s just in the heat of the moment (putting down markups is pretty exciting) I will inadvertently click the wrong one and delete a bunch of work.
I would leave Rename as the first option as it’s probably done more than any other, and delete control point right under it.
That’s exactly my point. You as an experienced user can potentially confuse this. İ thought keeping them right next to each other oppose to far from, likely to reduce this error. Because you are more likely to see both at the same time and give it a thought before you click the whichever one you saw first.
Oh, I see. Yeah, that’s a valid point, and having the extra verbiage is probably enough to make sure I don’t click the wrong one. And organizationally it does make sense to have them in the same general area. Sure, I’ll vote for that option.
Actions that apply to the entire node actions are kept away from actions that only apply to the current control point. I wouldn’t break this grouping. There are reasons to have “Delete…” at the top of the action group and bottom of the action group, too.
The proper solution would be to enable scene undo (at least for markups), but we can also add a confirmation popup (with a “Don’t show again” checkbox) - it is easy to implement, and we already have such popups at a number of places.
If we can not show the confirmation Dialog by default, but support customization to show it, that would be great. It would help reduce customization of code and maintenance burden on our group’s end to maintain current behavior. Adding code to maintain current behavior is not desired.
Renaming “Fiducial” to “Fiducial List” would resolve issues of terminology confusion between “Fiducial” vs “Control Point”. Use of the “node” terminology is too technical for regular users, so would have to be something like “Fiducial List” rather than “Fiducial Node”. It is unexpected that “Fiducial” is a group of points. I understand some of the mistakes from the past about this terminology, but surely some improvement is possible even if it may be annoying to fix on the backend code side.
That’s a valid point too, as to casual users “Fiducial” sounds singular, as in a single point. Renaming it to “Delete Fiducial List” seems like a good compromise?
We’ll do this. This need to separate the machine readable markup type (Fiducial, ClosedCurve, etc.) from what is displayed on the GUI (with better formatting, more clear meaning, translatable to different languages; such as fiducial list, closed curve) has come up in another discussion already. I’ve added a ticket to track this:
Going further, the term “Fiducial” is pretty odd (it comes from Slicer’s neurosurgery roots where it refers to a marker stuck to the skin before a scan, and those are even used any more). If we are changing things why not drop that term completely and use a term like “Point Set” or “Point List” or “Landmarks”?
As an aside, for proper names of data types I think Title Case is correct, so Closed Curve and Point Set make sense, even if they are used in a UI element where the text is in Sentence case (that is the menu item would read “Delete highlighted Point Set” or “Delete highlighted Closed Curve”).
