When no patient image is selected, the "New" segmentation button can still be clicked (but nothing happens). It would be better if it was only enabled when an image is selected, and was not clickable/"greyed out" otherwise.
Description
Description
| Status | Assigned | Task | ||
|---|---|---|---|---|
| Open | None | T28176 [Segmentation] Inspection of module and plugin | ||
| Resolved | kalali | T27713 [Segmentation] Disable "New" button when no image is selected |
Event Timeline
Comment Actions
Also, the "red warning" does not change back to "Please select an image" if an image is removed (which was automatically selected before).
That leads me to the question, @kompan: Does the bug only happen, after you removed a selected node or also, if no node was initially selected?
Comment Actions
Good point! It only occurs after I have removed the image node. When I initially open the segmentation plugin, the new button still is deactivated as it should be.
Comment Actions
I looked into it and I have the impression that there is a lot of unnecessary complexity created in order to generate a valid UI state. The problem, as far as I can see, lies in the overuse of if-statements and having functions that have way to many responsibilities. This is a typical "clean code" task and I suggest to go through the whole QmitkSegmentationView.cpp file and apply some best practices of writing clean code.
But since it is not clear how much effort we want to put into it (thinking of T28142), it might be wise to not work on this right now but to use a general refactoring task (parent task) to fix the mentioned bug.
I advise also to have a look https://phabricator.mitk.org/file/data/7bghshzurxwupnipv3dl/PHID-FILE-n2hwm6xv2mil3ola47sq/QtStateMachineFramework.pdf.
Comment Actions
I looked into this (to be honest, I did not read Amirs last comment): I totally agree. I fixed the issue, which was not to complicated, but there is a lot of spaghetti code involved. I agree with Amirs suggestion to not fix it this way.
Comment Actions
I started working on T28142: [Segmentation] Remove plugin redundancy with MultiLabelSegmentation which makes this task and some mentioned bugs invalid. We can close this task, once D533 is landed. The "New"-button is correctly enabled / disabled using the mentioned changes / differential and some warning messages were removed / simplified.