wxSearchCtrl default sizing problems on GTK3 (Issue #25210)
Olly Betts
ojwb created an issue (wxWidgets/wxWidgets#25210)Description
Bug description:
The default sizing behaviour of wxSearchCtrl in a toolbar in wxGTK3 makes it close to unusable by the user.
Expected vs observed behaviour:
Observed:
Adding a wxSearchCtrl to a toolbar in a wxGTK3 build (using either git master and the Debian 3.2.6+dfsg-2 packages) adds a little rounded cornered white rectangle with a magnifying glass icon inside. Reasonable so far - modern search controls often start shrunk like this.
-
There seems to be a very narrow rectangle I need to click in to give it focus which isn't great for usability.
-
The much worse problem is that if I enter text then the search control's cancel icon appears and the rectangle gets wider to accommodate that, but only just enough wider to accommodate the icon - it doesn't grow in width to actually show any of the text being entered. (If the wxSearchCtrl is created with a value set, as in the toolbar sample, then the cancel icon is there to start with, but it still doesn't resize to show either the initial text or any text subsequently entered.)
Expected:
-
I'd expect it not to be so fiddly to give search control focus.
-
I'd expect entering text to cause to cause the search control to grow up to the controls maximum width (and probably then scroll to show the text around the caret position).
To Reproduce:
Steps to reproduce the behaviour, please make them as detailed as possible.
For example:
- Run the
toolbarsample - Maximise the window
- Ctrl+F (to add a wxSearchCtrl to the right end of the toolbar)
- Notice you can't see any initial value set on the control (which is
needleaccording to the sample source code) but the "cancel" icon is present which shows there is a value - Click between the two icons in the search control to give it focus (and notice the area to click in is quite narrow)
- Type text (and notice you can't see any of the text you enter either
- wxWidgets version either git master and the Debian 3.2.6+dfsg-2 packages
- wxWidgets port wxGTK
- Debian unstable
- GTK version: 3.24.41-1
- Which GDK backend is used: X11
- Desktop environment : mate
- Current theme: Menta
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25210)Thanks, I can indeed see this and it's not X11-specific as I see it with Wayland too. I'll try to have a look.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25210)I think there are 2 problems here:
wxSearchCtrlbest size just was too small in wxGTK, this could be seen even outside of the toolbar, as creating it with its default size made it unusable (and even smaller than in the toolbar, see the next point). I don't know what is the right way to compute the best size for it, but ensuring that it's wide enough is better than nothing, IMHO, so this is what I did in the linked PR.wxToolBardoesn't respect min size of the windows added to it in wxGTK but just uses their size at GTK level — this prevented an existing call toSetMinSize(80, -1)in the sample from working. I think we should request at least the best size of the window and this is what I did in the other commit of the linked PR.
Neither solution seems ideal, but at least this was something I could do immediately and it improves things from the user point of view, so unless someone can propose a better solution, I'd like to merge this PR.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Closed #25210 as completed.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25210)This is fixed now, so closing. Thanks again for reporting!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Olly Betts
ojwb left a comment (wxWidgets/wxWidgets#25210)The fix "Respect best control size when adding it to wxGTK wxToolBar" was backported to 3.2.7:
However "Fix wxSearchCtrl best size with GTK" seems to still only be on master - was it intentional to not backport this? I particularly wonder as there's an automatic message above from it being cherry-picked, but if I follow it github says:
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
I don't know how to see where that commit actually is, but this makes me wonder if it was perhaps accidentally cherry-picked to a fork of the main repo?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Olly Betts
ojwb left a comment (wxWidgets/wxWidgets#25210)I should probably provide some more context to my question above...
With 3.2.2, my app's wxSearchCtrl in a wxToolbar sizes sensibly - I think this is thanks to a workaround I added after finding and reporting this issue, which is to call SetSize(160, -1) on it.
With 3.2.8 and the same app source code, the wxSearchCtrl becomes the "little rounded cornered white rectangle with a magnifying glass icon inside" like I reported here. My guess is that the backported fix to respect the best control size means we now use the best size, which exposes us again to the best size of a wxSearchCtrl being unhelpfully small.
In other words, backporting just one of these commits has made things worse, at least for this particular case. I think backporting the other would improve this.
It seems I can call SetMinSize() too (or perhaps instead - not tested that yet) to get app code which works for both 3.2.2 and 3.2.8, but it seems wrong for reasonable code which worked with 3.2.2 to break like this with 3.2.8.
(I'm testing with these two versions as they're what's in the most recent two Debian stable releases which means (a) I can easily test with them and (b) they're two versions that I particularly want my app to work with. Presumably this actually broke in 3.2.7 though.)
I would reopen but I don't have permission to. I can open a new issue instead if you'd prefer.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Olly Betts
ojwb left a comment (wxWidgets/wxWidgets#25210)It seems I can call
SetMinSize()too (or perhaps instead - not tested that yet) to get app code which works for both 3.2.2 and 3.2.8, but it seems wrong for reasonable code which worked with 3.2.2 to break like this with 3.2.8.
The "instead" was right - with 3.2.2 calling SetSize(160, -1) on the wxSearchCtrl gives a usable size but that doesn't work with 3.2.8; with 3.2.8 calling SetMinSize(wxSize(160, wxDefaultCoord)) gives a usable size but that doesn't work with 3.2.2; calling both gives a usable size with both versions. I don't know if it matters but I call these before I add the wxSearchCtrl to the wxToolBar.
I'd guess with 3.3.x that neither is needed but doing both shouldn't cause problems. I don't have a recent build of 3.3.x to test with though.
Anyway, there's at least a reasonable workaround for anyone hitting this problem and finding this ticket.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25210)Sorry, I've cherry picked this commit for backporting to 3.2 too now, see #25854.
I hope this doesn't result in any new problems, but please let me know if it does. TIA!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25210)Oops, it looks like cherry-picking this breaks if not ABI (because I am almost sure that overriding an existing virtual function doesn't really break it), then at least our ABI check workflow, see https://github.com/wxWidgets/wxWidgets/actions/runs/18434748499/job/52526744102, and even if this is a false positive, I don't want to disable the check because it's valuable for automatically detecting other, real, ABI compatibility violations.
So I'm going to have to revert this cherry-pick, as I don't see any reasonable way to implement this without overriding DoGetBestSize() (the only solution would be to specifically use dynamic_cast<wxSearchCtrl*>() in the base class version, but this doesn't seem "reasonable") and I don't know of any way to work around this error.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Olly Betts
ojwb left a comment (wxWidgets/wxWidgets#25210)Oh dear. I did even consider if it wasn't backported for ABI reasons but it seemed OK to me too. I can see that breaking the ABI checking machinery is a problem though.
I guess the backport of the other patch could be reverted, but I'm not sure that's making things better at this point.
Now I've finally worked out what was going on I'm OK with leaving the 3.2 branch as it now is, since I have a good workaround which allows my app to work with all 3.2.x. I really raised the apparently missing backport for the benefit of others who might hit the problem, but perhaps it was actually harder for me to get to the bottom of because I'd already found and reported the problem here!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.