BUG: Make sure the delete DICOM object dialog is not too tall #1140
Conversation
| // add the information about the selected UIDs | ||
| int numUIDs = uids.size(); | ||
| for (int i = 0; i < numUIDs; ++i) | ||
| { | ||
| if (i >= maxNumberOfPatientsToShow) | ||
| { | ||
| message += QString("\n(and %1 more)").arg(numUIDs - maxNumberOfPatientsToShow); |
There was a problem hiding this comment.
Use tr() and explain the message using a translator's comment (it shows up in Qt translator and Weblate) - indicated by //:
| message += QString("\n(and %1 more)").arg(numUIDs - maxNumberOfPatientsToShow); | |
| message += QString("\n") + tr("(and %1 more)").arg(numUIDs - maxNumberOfPatientsToShow); //: displayed when there are additional DICOM items to delete that do not fit on screen |
There was a problem hiding this comment.
Thanks! Yes I was wondering about the translation, if it worked or not in CTK. OK I'll do this.
| // on the items to show in the dialog | ||
| int browserHeight = this->geometry().height(); | ||
| int patientsTableRowHeight = d->dicomTableManager->patientsTable()->tableView()->rowHeight(0); | ||
| int maxNumberOfPatientsToShow = browserHeight / patientsTableRowHeight - 1; // Subtract 1 due to the do not show checkbox |
There was a problem hiding this comment.
If it is safer then we could subtract 2-3. Also make sure that the value is at least 3 (or so).
There was a problem hiding this comment.
I just didn't want to use arbitrary numbers (1 seemed easy to justify with the checkbox that is within the same layout). With the status bar and the toolbar and window header and OS taskbar even 1 is more than safe.
But I can make it 3, it's safer for sure. And I'll add the minimum number.
cpinter
force-pushed
the
fix-delete-dicom-object-dialog-max-height
branch
from
b1a938b
to
8b0346e
Compare
|
I made the requested small changes last Tuesday. If it looks good please approve. Thank you! |
|
Just a really small thing: is it possible that something like this is displayed: 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). |
|
Thanks Andras! I'll make sure the message is not triggered with only one left. One sec. |
|
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! |
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.
cpinter
force-pushed
the
fix-delete-dicom-object-dialog-max-height
branch
from
8b0346e
to
30d4903
Compare
|
Thank you for reviewing all my PRs :) |
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:
