Page MenuHome GnuPG

Libkleo: KeySelectionCombo has wrong sizeHint
Open, NormalPublic

Description

Just noticed while testing T6837 the newkeyapprovaldialog had a fixed size of desk.width()/3 if I change that to take the sizeHint into account the window is way too small.

So the sizeHint of the comboBoxes is too small. Ingo I am assigning this to you since I think you know best to fix this.

Event Timeline

aheinecke triaged this task as Normal priority.Nov 25 2023, 5:16 AM
aheinecke created this task.
aheinecke added a project: vsd33.
aheinecke added a subscriber: ikloecker.

Tobias could you create an MR for this?

As far as I can tell, the sizeHint is "correct", for the items that are currently in the combobox. At the point in time of creating the dialog, the combobox only contains two items ("new key" and "no key"), which both have shorter strings than an average key description. The actual keys are only added to the combobox at a later point. I tried to make the dialog's size update when that happens, but have not managed to get it working yet, i think that some cache is not being invalidated correctly.

Shouldn't that be the difference between SizeAdjustPolicy AdjustToContentsOnFirstShow and AdjustToContents?

But for me at least the following does not work:

diff --git a/src/ui/keyselectioncombo.cpp b/src/ui/keyselectioncombo.cpp
index 8564ace..176878b 100644
--- a/src/ui/keyselectioncombo.cpp
+++ b/src/ui/keyselectioncombo.cpp
@@ -556,6 +556,8 @@ KeySelectionCombo::KeySelectionCombo(bool secretOnly, KeyUsage::Flags usage, QWi
         d->restoreCurrentSelectionAfterModelChange();
     });
 
+    setSizeAdjustPolicy(QComboBox::AdjustToContents);
+
     QTimer::singleShot(0, this, &KeySelectionCombo::init);
 }
 
diff --git a/src/ui/newkeyapprovaldialog.cpp b/src/ui/newkeyapprovaldialog.cpp
index 186c5d0..9d54acf 100644
--- a/src/ui/newkeyapprovaldialog.cpp
+++ b/src/ui/newkeyapprovaldialog.cpp
@@ -908,7 +908,7 @@ NewKeyApprovalDialog::NewKeyApprovalDialog(bool encrypt,
 
     const auto size = sizeHint();
     const auto desk = screen()->size();
-    resize(QSize(desk.width() / 3, qMin(size.height(), desk.height() / 2)));
+    resize(QSize(qMin(size.width(), desk.width() / 2), qMin(size.height(), desk.height() / 2)));
 }
 
 Kleo::NewKeyApprovalDialog::~NewKeyApprovalDialog() = default;

It still uses the very small sizes of the default items. Although I think at the time that the dialog is resized it has already set the encryption keys. So the contents should match.

I suggest to replace size.width() with qMax(size.width(), minWidth) where minWidth is the width of a reasonably sized text (to account for different text sizes) instead of trying to fight with the combo box. Combo boxes are not a good UI element for long entries.

And if it doesn't already do so then NewKeyApprovalDialog should probably remember the last used width (but maybe not the height because more keys need more height, but we don't want to have too much empty vertical space in case of just a few keys).

TobiasFella mentioned this in Unknown Object (Event).Dec 18 2023, 9:43 AM
TobiasFella mentioned this in Unknown Object (Event).Jan 2 2024, 9:30 AM

In E1020 @TobiasFella wrote about this: Sizehint is correct, but only at a later point in time; also, apparently some cache invalidation problem? Since the current version works fine with a fixed size, might not be worth the effort to fix.

@TobiasFella I do not think the current version behaves correctly.

is just a very simple example how this looks for me. And my desk width is huuuuuuuge.