@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 would suggest having these actions close together and with at a more description, perhaps something like this:
- Delete Control Point
- Delete FiducialNode (deletes all control points).
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.
That will indeed be the best solution. +1
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 (
ClosedCurve, etc.) from what is displayed on the GUI (with better formatting, more clear meaning, translatable to different languages; such as
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”).
I agree. Point List or Landmark List would be great to refer to fiducials.
Totally agreed. I’ve had to explain “Fiducial” on several occasions - always gets a quizzical look. Either Point List or Landmark List (following @pieper’s Title Case) would be much clearer.
I like “Point List” because it is similar to other markup types - line, curve, plane, etc. and does not assume any particular use. A geometric primitive has multiple uses (point can represent a fiducial marker or an anatomical landmark; a line can represent a distance measurement ruler or caliper, or an axis of rotation, etc.) so it makes sense to not name them based on one specific use.
I would prefer “Point List”, as indicates that order of points matter; in contrast to “point set”, which often means that the items are unordered.
Changing class names and code strings would be disruptive but changing the display name from “Fiducial” to “Point List” should have minimal impact.
I support “Point List” as yes we use this node type for placing locations to move our motion stage to, so it isn’t always for Landmarking purposes.
Just to chime in, I like Point List too. It is good timing for such a change before releasing Slicer 5.
I tend to agree with @jamesobutler that “node” may be too technical, but still I think the users know what they are (it can be seen in many places in the UI), so adding “node” to the end of the delete action text would make things more explicit.
As it relates to “node” being too technical I say that because multiple of our users had previously asked “what is a node?” when we had some mention of it. I had to explain that it essentially just meant “object”. So we changed from saying “creating a line node” to simply “creating a line”.
If there are places in Slicer that say “node” in the GUI those should be removed. I know in the Segment Editor module the main combobox objects at the top are “Segmentation” and “Master Volume” which don’t have “node” mentions even though technically it is a node selector.
We should try to avoid using “node” in the GUI as much as possible, but I don’t think it is reasonable to ban it. For example, It would be not easy to remove “node” term from everywhere in Data module and anywhere where we collectively refer to different types of nodes or we don’t know the exact node type.
I agree that “object” would sound less technical, but it would be confusing when it is used for constructs that are not physical objects, such as a parameter node, display node, storage node, unit node, selection node, interaction node. “Object” is also a heavily used term in programming, so it would be hard to talk about how to use methods if the inputs/outputs were just objects. If we used “object” on the GUI and “node” in the API then it would complicate things.
To follow up, there was a PR created that includes changing “Fiducial” to “Point List”.