OS X, correct background of wxStaticText in a wxStaticBox (PR #26088)
RobertRoeb
The attached first screenshot shows the wrong background colour, in this case in a wxRadioBox in dark mode. The second screenshot shows the corrected background colour of a wxStatictext in a wxStaticBox, but the uncorrected wrong colour of a wxCheckBox in the same wxStaticBox.
It also shows the macOS system settings dialog of macOS Tahoe 26.2 on the left, which clearly uses a different colour theme altogether, so the wxWidgets app uses some legacy rendering, which eventually we should get rid of (maybe due to overriding drawRect as has been suggested in a different patch).
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/26088
Commit Summary
- fabcfe0 OS X, correct background of wxStaticText in a wxStaticBox
File Changes
(2 files)
- M include/wx/osx/stattext.h (6)
- M src/osx/stattext_osx.cpp (29)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/26088.patch
- https://github.com/wxWidgets/wxWidgets/pull/26088.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maarten
@MaartenBent commented on this pull request.
> +wxBEGIN_EVENT_TABLE(wxStaticText, wxStaticTextBase) + EVT_SYS_COLOUR_CHANGED( wxStaticText::OnSysColourChanged ) +wxEND_EVENT_TABLE() +
Hi, I'm not sure what the best practices are here, but I think new code always tries to use Bind/Unbind instead of event tables.
> return true;
}
+void wxStaticText::DoUpdateBackground()
+{
+#if wxOSX_USE_COCOA_OR_CARBON
+ if ((GetParent() != nullptr) && GetParent()->IsKindOf(wxCLASSINFO(wxStaticBox)))
+ {
+ if (wxSystemSettings::GetAppearance().IsDark())
+ {
+ SetBackgroundColour( wxColour( 37, 37, 37 ) );
+ }
+ else
+ {
+ SetBackgroundColour( wxColour( 247, 247, 247 ) );
Are there maybe wxSystemColour constants that map to these colours?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz requested changes on this pull request.
I won't merge this without approval from @csomor as I'm far from sure that this is correct — it would be far better to handle all these controls (i.e. those with HasTransparentBackground() returning true) in some generic way somewhere else.
Also, as written, this overwrites any custom background set for wxStaticText which should, of course, be avoided.
> return true;
}
+void wxStaticText::DoUpdateBackground()
+{
+#if wxOSX_USE_COCOA_OR_CARBON
+ if ((GetParent() != nullptr) && GetParent()->IsKindOf(wxCLASSINFO(wxStaticBox)))
Parent can never be null for a non-TLW, so the first half of the test is not necessary.
Second part of the test would, of course, be better written as a call to some virtual OSXGetChildBackground() or similar.
> +#if wxOSX_USE_COCOA_OR_CARBON
+ if ((GetParent() != nullptr) && GetParent()->IsKindOf(wxCLASSINFO(wxStaticBox)))
+ {
+ if (wxSystemSettings::GetAppearance().IsDark())
+ {
+ SetBackgroundColour( wxColour( 37, 37, 37 ) );
+ }
+ else
+ {
+ SetBackgroundColour( wxColour( 247, 247, 247 ) );
+ }
+ }
+#endif
+}
+
+void wxStaticText::OnSysColourChanged(wxSysColourChangedEvent& WXUNUSED(event))
Should call event.Skip(), especially if/when this is converted to use Bind().
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Stefan Csomor
csomor left a comment (wxWidgets/wxWidgets#26088)these controls should have their background style be wxBG_STYLE_TRANSPARENT but right now it's wxBG_STYLE_ERASE, adding a
SetBackgroundStyle(wxBG_STYLE_TRANSPARENT);
in the constructor shows the fix, but I don't think they always had wxBG_STYLE_ERASE as style in the first place, I'll try to find out
—
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#26088)Closing in favour of #26090, please test if it fixes the problem.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.