commontk / CTK

A set of common support code for medical imaging, surgical navigation, and related purposes.
https://commontk.org
Apache License 2.0
855 stars 490 forks source link

BUG: Make sure the delete DICOM object dialog is not too tall #1140

Closed cpinter closed 1 year ago

cpinter commented 1 year ago

When deleting many patients (or studies or series) it can occur that the confirmation dialog that is created with all the deleted object names is taller than the screen height, so the buttons are cut off and not available. This commit ensures that the said dialog cannot be higher than the DICOM browser.

Note that the height of one row is calculated from the first row height in the patients table, which may not be the same as the height of a row in the label that shows the deleted item names. This is implemented like this for simplicity, however, since the table row is typically somewhat higher than a line in the label, it is safe. Also, the height calculation does not include the window header and the buttons.

This is how the shortened message looks: 20230918_DeletePatientsDialogHeight

cpinter commented 1 year ago

I made the requested small changes last Tuesday. If it looks good please approve. Thank you!

lassoan commented 1 year ago

Just a really small thing: is it possible that something like this is displayed:

PatientName1,
PatientName2,
PatientName3,
PatientName4,
and 1 more patient

If the "more" message takes a line then it should not be displayed if only 1 more patient cannot be displayed (because that patient could be displayed on that line). Removing the "1 more patient" option would also make translation simpler (there would only be a plural mode).

PatientName1,
PatientName2,
PatientName3,
and 2 more patients
cpinter commented 1 year ago

Thanks Andras! I'll make sure the message is not triggered with only one left. One sec.

cpinter commented 1 year ago

By the way there is no singular/plural issue, because the message right now is "(and 2 more)". Just adding patient is not correct because the same code is used when deleting studies and series so I'd need to make an if/else to handle the three cases. Would you like to change the message? In the meantime I'll do the one item check. Thanks!

cpinter commented 1 year ago

Thank you for reviewing all my PRs :)