From 58c94d9ec00a31da0be189c14a2964b8df1f4344 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=CC=81clav=20Slavi=CC=81k?= Date: Thu, 27 Aug 2020 18:12:18 +0200 Subject: [PATCH 1/3] Draw wxTextCtrl focus ring natively on Mac NSTextView doesn't display focus ring by default, which is why wxOSX did draw it manually, but this behavior can be overriden since OS X 10.3 with NSView.focusRingType property. The HITheme-based rendering suffered from a number of non-nativeness issues: - didn't respect macOS 10.14+ accent colors - not animated as the native focus ring - subtly different shape of the outline - noticeably different outline shape on macOS 11 Remove NeedsFocusRect() and associated workaround for manually drawing focus ring inside NSTextView (i.e. multiline text controls). This private interface was only used for wxTextCtrl and nothing else, so this shouldn't have any impact elsewhere. --- include/wx/osx/cocoa/private.h | 2 ++ include/wx/osx/core/private.h | 4 +--- src/generic/grideditors.cpp | 2 +- src/osx/cocoa/textctrl.mm | 2 +- src/osx/cocoa/window.mm | 9 +++++---- src/osx/iphone/window.mm | 4 ---- src/osx/window_osx.cpp | 22 ---------------------- 7 files changed, 10 insertions(+), 35 deletions(-) diff --git a/include/wx/osx/cocoa/private.h b/include/wx/osx/cocoa/private.h index 8394ebb6ff..ea782e9fae 100644 --- a/include/wx/osx/cocoa/private.h +++ b/include/wx/osx/cocoa/private.h @@ -111,6 +111,8 @@ public : virtual void SetNeedsDisplay( const wxRect* where = NULL ) wxOVERRIDE; virtual bool GetNeedsDisplay() const wxOVERRIDE; + virtual void EnableFocusRing(bool enabled) wxOVERRIDE; + virtual void SetDrawingEnabled(bool enabled) wxOVERRIDE; virtual bool CanFocus() const wxOVERRIDE; diff --git a/include/wx/osx/core/private.h b/include/wx/osx/core/private.h index ba57b86157..eb27786c47 100644 --- a/include/wx/osx/core/private.h +++ b/include/wx/osx/core/private.h @@ -301,8 +301,7 @@ public : virtual void SetNeedsDisplay( const wxRect* where = NULL ) = 0; virtual bool GetNeedsDisplay() const = 0; - virtual bool NeedsFocusRect() const; - virtual void SetNeedsFocusRect( bool needs ); + virtual void EnableFocusRing(bool WXUNUSED(enabled)) {} virtual bool NeedsFrame() const; virtual void SetNeedsFrame( bool needs ); @@ -598,7 +597,6 @@ protected : bool m_wantsUserKey; bool m_wantsUserMouse; wxWindowMac* m_wxPeer; - bool m_needsFocusRect; bool m_needsFrame; bool m_shouldSendEvents; diff --git a/src/generic/grideditors.cpp b/src/generic/grideditors.cpp index 7a328d9b97..b7866e0b57 100644 --- a/src/generic/grideditors.cpp +++ b/src/generic/grideditors.cpp @@ -462,7 +462,7 @@ void wxGridCellTextEditor::DoCreate(wxWindow* parent, #ifdef __WXOSX__ wxWidgetImpl* impl = m_control->GetPeer(); - impl->SetNeedsFocusRect(false); + impl->EnableFocusRing(false); #endif // set max length allowed in the textctrl, if the parameter was set if ( m_maxChars != 0 ) diff --git a/src/osx/cocoa/textctrl.mm b/src/osx/cocoa/textctrl.mm index 1e46578226..1f3abd2d65 100644 --- a/src/osx/cocoa/textctrl.mm +++ b/src/osx/cocoa/textctrl.mm @@ -1590,7 +1590,7 @@ wxWidgetImplType* wxWidgetImpl::CreateTextControl( wxTextCtrl* wxpeer, v = [[wxNSTextScrollView alloc] initWithFrame:r]; wxNSTextViewControl* t = new wxNSTextViewControl( wxpeer, v, style ); c = t; - c->SetNeedsFocusRect( true ); + c->EnableFocusRing( true ); t->SetStringValue(str); } diff --git a/src/osx/cocoa/window.mm b/src/osx/cocoa/window.mm index 80a0838032..813bb1face 100644 --- a/src/osx/cocoa/window.mm +++ b/src/osx/cocoa/window.mm @@ -3849,10 +3849,6 @@ void wxWidgetCocoaImpl::DoNotifyFocusLost() void wxWidgetCocoaImpl::DoNotifyFocusEvent(bool receivedFocus, wxWidgetImpl* otherWindow) { wxWindow* thisWindow = GetWXPeer(); - if ( thisWindow->MacGetTopLevelWindow() && NeedsFocusRect() ) - { - thisWindow->MacInvalidateBorders(); - } if ( receivedFocus ) { @@ -3925,6 +3921,11 @@ void wxWidgetCocoaImpl::SetFlipped(bool flipped) #endif +void wxWidgetCocoaImpl::EnableFocusRing(bool enabled) +{ + [m_osxView setFocusRingType:enabled ? NSFocusRingTypeExterior : NSFocusRingTypeNone]; +} + void wxWidgetCocoaImpl::SetDrawingEnabled(bool enabled) { if ( [m_osxView window] == nil ) diff --git a/src/osx/iphone/window.mm b/src/osx/iphone/window.mm index 607c92f839..50bdd3ff7c 100644 --- a/src/osx/iphone/window.mm +++ b/src/osx/iphone/window.mm @@ -642,10 +642,6 @@ void wxWidgetIPhoneImpl::InstallEventHandler( WXWidget control ) void wxWidgetIPhoneImpl::DoNotifyFocusEvent(bool receivedFocus, wxWidgetImpl* otherWindow) { wxWindow* thisWindow = GetWXPeer(); - if ( thisWindow->MacGetTopLevelWindow() && NeedsFocusRect() ) - { - thisWindow->MacInvalidateBorders(); - } if ( receivedFocus ) { diff --git a/src/osx/window_osx.cpp b/src/osx/window_osx.cpp index 46fa8188a1..1d91b52a13 100644 --- a/src/osx/window_osx.cpp +++ b/src/osx/window_osx.cpp @@ -938,9 +938,6 @@ void wxWindowMac::MacInvalidateBorders() int outerBorder = MacGetLeftBorderSize() ; - if ( GetPeer()->NeedsFocusRect() ) - outerBorder += 4 ; - if ( outerBorder == 0 ) return ; @@ -1578,8 +1575,6 @@ void wxWindowMac::MacPaintBorders( int WXUNUSED(leftOrigin) , int WXUNUSED(right #if wxOSX_USE_COCOA_OR_CARBON { - const bool hasFocus = GetPeer()->NeedsFocusRect() && HasFocus(); - CGRect cgrect = CGRectMake( tx-1 , ty-1 , tw+2 , th+2 ) ; @@ -1594,7 +1589,6 @@ void wxWindowMac::MacPaintBorders( int WXUNUSED(leftOrigin) , int WXUNUSED(right info.version = 0 ; info.kind = 0 ; info.state = IsEnabled() ? kThemeStateActive : kThemeStateInactive ; - info.isFocused = hasFocus ; if ( HasFlag(wxRAISED_BORDER) || HasFlag(wxSUNKEN_BORDER) || HasFlag(wxDOUBLE_BORDER) ) { @@ -1607,11 +1601,6 @@ void wxWindowMac::MacPaintBorders( int WXUNUSED(leftOrigin) , int WXUNUSED(right HIThemeDrawFrame( &cgrect , &info , cgContext , kHIThemeOrientationNormal ) ; } } - - if ( hasFocus ) - { - HIThemeDrawFocusRect( &cgrect , true , cgContext , kHIThemeOrientationNormal ) ; - } } #endif // wxOSX_USE_COCOA_OR_CARBON } @@ -2746,20 +2735,9 @@ void wxWidgetImpl::Init() m_wantsUserKey = false; m_wantsUserMouse = false; m_wxPeer = NULL; - m_needsFocusRect = false; m_needsFrame = true; } -void wxWidgetImpl::SetNeedsFocusRect( bool needs ) -{ - m_needsFocusRect = needs; -} - -bool wxWidgetImpl::NeedsFocusRect() const -{ - return m_needsFocusRect; -} - void wxWidgetImpl::SetNeedsFrame( bool needs ) { m_needsFrame = needs; From 9f66b03c5c934b1f348ad92a44c0ff82faf168b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=CC=81clav=20Slavi=CC=81k?= Date: Mon, 31 Aug 2020 10:38:39 +0200 Subject: [PATCH 2/3] Allow configuring visible focus on Mac Add wxWindow::EnableVisibleFocus() for changing focus ring behavior on macOS (currently not implemented elsewhere, although GTK+ has a discouraged option to do it). --- include/wx/osx/window.h | 1 + include/wx/window.h | 3 +++ interface/wx/window.h | 16 ++++++++++++++++ src/osx/window_osx.cpp | 5 +++++ 4 files changed, 25 insertions(+) diff --git a/include/wx/osx/window.h b/include/wx/osx/window.h index e327b0a55b..f761a598bc 100644 --- a/include/wx/osx/window.h +++ b/include/wx/osx/window.h @@ -157,6 +157,7 @@ public: void MacOnScroll( wxScrollEvent&event ); virtual bool AcceptsFocus() const wxOVERRIDE; + virtual void EnableVisibleFocus(bool enabled) wxOVERRIDE; virtual bool IsDoubleBuffered() const wxOVERRIDE { return true; } diff --git a/include/wx/window.h b/include/wx/window.h index 1c3cc3d759..e6d987812f 100644 --- a/include/wx/window.h +++ b/include/wx/window.h @@ -767,6 +767,9 @@ public: // call this when the return value of AcceptsFocus() changes virtual void SetCanFocus(bool WXUNUSED(canFocus)) { } + // call to customize visible focus indicator if possible in the port + virtual void EnableVisibleFocus(bool WXUNUSED(enabled)) { } + // navigates inside this window bool NavigateIn(int flags = wxNavigationKeyEvent::IsForward) { return DoNavigateIn(flags); } diff --git a/interface/wx/window.h b/interface/wx/window.h index 3776331ddc..9d1d235c9f 100644 --- a/interface/wx/window.h +++ b/interface/wx/window.h @@ -540,6 +540,22 @@ public: */ virtual void SetCanFocus(bool canFocus); + /** + Enables or disables visible indication of keyboard focus. + + By default, controls behave in platform-appropriate way and show focus + in the same way native applications do. In some very rare circumstances + it may be desirable to change the default (notably multiline text + controls don't normally have a focus indicator on Mac), which this + method allows. It should only be used if you have a good understanding + of the native platform's guidelines and user expectations. + + This method is only implemented on Mac. + + @since 3.1.5 + */ + virtual void EnableVisibleFocus(bool enable); + /** This sets the window to receive keyboard input. diff --git a/src/osx/window_osx.cpp b/src/osx/window_osx.cpp index 1d91b52a13..6e349163ff 100644 --- a/src/osx/window_osx.cpp +++ b/src/osx/window_osx.cpp @@ -2171,6 +2171,11 @@ bool wxWindowMac::AcceptsFocus() const return GetPeer()->CanFocus(); } +void wxWindowMac::EnableVisibleFocus(bool enabled) +{ + GetPeer()->EnableFocusRing(enabled); +} + void wxWindowMac::MacSuperChangedPosition() { // only window-absolute structures have to be moved i.e. controls From 9c5abea2661598af98cde565bc26a4db9e2af89e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=CC=81clav=20Slavi=CC=81k?= Date: Sun, 30 Aug 2020 18:35:45 +0200 Subject: [PATCH 3/3] =?UTF-8?q?Don=E2=80=99t=20add=20non-default=20focus?= =?UTF-8?q?=20ring=20to=20NSTextView?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NSTextView does not natively show a focus ring by default, and it is extremely rare thing to do in native applications. Don't do it in wxTextCtrl either. --- src/generic/grideditors.cpp | 4 ---- src/osx/cocoa/textctrl.mm | 1 - 2 files changed, 5 deletions(-) diff --git a/src/generic/grideditors.cpp b/src/generic/grideditors.cpp index b7866e0b57..dc01ce4465 100644 --- a/src/generic/grideditors.cpp +++ b/src/generic/grideditors.cpp @@ -460,10 +460,6 @@ void wxGridCellTextEditor::DoCreate(wxWindow* parent, text->SetMargins(0, 0); m_control = text; -#ifdef __WXOSX__ - wxWidgetImpl* impl = m_control->GetPeer(); - impl->EnableFocusRing(false); -#endif // set max length allowed in the textctrl, if the parameter was set if ( m_maxChars != 0 ) { diff --git a/src/osx/cocoa/textctrl.mm b/src/osx/cocoa/textctrl.mm index 1f3abd2d65..3029096b03 100644 --- a/src/osx/cocoa/textctrl.mm +++ b/src/osx/cocoa/textctrl.mm @@ -1590,7 +1590,6 @@ wxWidgetImplType* wxWidgetImpl::CreateTextControl( wxTextCtrl* wxpeer, v = [[wxNSTextScrollView alloc] initWithFrame:r]; wxNSTextViewControl* t = new wxNSTextViewControl( wxpeer, v, style ); c = t; - c->EnableFocusRing( true ); t->SetStringValue(str); }