MSW: Fix dark mode wxSpinCtrl border and wxSpinButton switching (PR #26656)
Steve Cornett
On Windows in dark mode, set the border on the wxSpinCtrl's text control to a simple gray 1-pixel thick border. See #26655.
This also fixes dark mode switching for wxSpinButton, for Windows 11 22H2 and later.
This is a relatively low-risk solution with an imminent upcoming release in mind. A more complicated idea is for the spin control's text control to be managed using wxTextCtrl rather than an HWND to a low-level EDIT control. The low-level control does not get the WM_NCPAINT handling that draws dark mode and themed borders.
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/26656
Commit Summary
- 3f14b9a Fix dark mode wxSpinCtrl border and wxSpinButton switching
File Changes
(3 files)
- M include/wx/msw/spinctrl.h (6)
- M src/msw/spinbutt.cpp (14)
- M src/msw/spinctrl.cpp (43)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/26656.patch
- https://github.com/wxWidgets/wxWidgets/pull/26656.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Steve Cornett
@stevecor commented on this pull request.
> @@ -134,15 +134,12 @@ bool wxSpinButton::Create(wxWindow *parent,
SubclassWin(m_hWnd);
Bind(wxEVT_PAINT, &wxSpinButton::OnPaint, this);
- Bind(wxEVT_ERASE_BACKGROUND, [](wxEraseEvent& event)
- {
- // Do nothing in dark mode, the background will be erased in OnPaint().
- if ( !wxMSWDarkMode::IsActive() )
- event.Skip();
- });
I could not see any difference in behavior with or without this wxEVT_ERASE_BACKGROUND handler. Rather than duplicate the OS version check done for the corresponding code in the paint handler, I removed this event handler.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26656)Thanks, but I'm not really sure I understand what's going on here.
I think it could be better to revert the change to wxWindow::DoTranslateBorder() in 8a573dd or otherwise arrange things to use wxBORDER_SIMPLE for the buddy EDIT by default, e.g. something like this:
diff --git a/src/msw/spinctrl.cpp b/src/msw/spinctrl.cpp index 99e0960bf3..a3babcdcf4 100644 --- a/src/msw/spinctrl.cpp +++ b/src/msw/spinctrl.cpp @@ -280,9 +280,23 @@ bool wxSpinCtrl::Create(wxWindow *parent, // set style for the base class style |= wxSP_VERTICAL; - // the border is only used for the text control part - if ( (style & wxBORDER_MASK) == wxBORDER_DEFAULT ) - style |= DoTranslateBorder(wxBORDER_THEME); + // Any of border styles except for wxBORDER_SIMPLE look ugly with the spin + // button, so always use simple border which seamlessly transitions into + // the button -- except when we shouldn't have any border at all. + switch ( style & wxBORDER_MASK ) + { + case wxBORDER_NONE: + break; + + case wxBORDER_SIMPLE: + // Keep it. + break; + + default: + // Replace anything else with wxBORDER_SIMPLE. + style &= ~wxBORDER_MASK; + style |= wxBORDER_SIMPLE; + } SetWindowStyle(style);
What do you think?
This is still not perfect because the border colour differs from the spin button border: it's #646464 around the text and #808080 around the button. And it also differs from #9B9B9B used around the standard wxTextCtrl (and hence wxSpinCtrlDouble), but this is less noticeable and I actually think #646464 looks better in dark mode, #9B9B9B is too bright.
And in light mode this results in a different colour too: spin button border is #ABADB3 here and standard text border is #7A7A7A while the same #646464 is used for simple border.
Still, the above is a much simpler change than this PR so I'd rather apply it if you don't see any huge problems with it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Steve Cornett
stevecor left a comment (wxWidgets/wxWidgets#26656)I agree we should use your proposal. That is good enough for now. The differences in gray colors are not very noticeable.
Unfortunately, with wxBORDER_SIMPLE in light mode the text control loses the standard themed behavior of showing the control has the focus with the blue line at the bottom. Perhaps we can consider some ideas to improve the appearance of this control later.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Steve Cornett
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26656)Thanks for testing/confirmation! I agree that losing highlight is bad (BTW, I wonder if we could emulate this in wxSpinCtrlDouble somehow?), we probably should use sunken border in light mode and simple only in dark, but I don't have enough time to test all the combinations of modes/Windows versions, so for now I'll just push the diff above — and hopefully this could be improved further before 3.4.0.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.