From 88f23f17c15674e670afaf4cd6902e77e1e60ecc Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Tue, 26 Dec 2017 22:35:04 +0000 Subject: [PATCH] Fix dialog OK button event issues on OSX. Move the OSX dialog fix ups later so they come after some wxWindow deferred processing. Also provides a facility for doing a selectAll in each text field so that tabbing between text fields behaves correctly. Fixes: lp:1599157 * https://bugs.launchpad.net/kicad/+bug/1599157 --- common/dialog_shim.cpp | 132 +++++++++++++++++++++++------------------ include/dialog_shim.h | 27 ++++----- 2 files changed, 88 insertions(+), 71 deletions(-) diff --git a/common/dialog_shim.cpp b/common/dialog_shim.cpp index 5de406a70c..e1719d2fbf 100644 --- a/common/dialog_shim.cpp +++ b/common/dialog_shim.cpp @@ -58,6 +58,7 @@ DIALOG_SHIM::DIALOG_SHIM( wxWindow* aParent, wxWindowID id, const wxString& titl const wxPoint& pos, const wxSize& size, long style, const wxString& name ) : wxDialog( aParent, id, title, pos, size, style, name ), KIWAY_HOLDER( 0 ), + m_fixupsRun( false ), m_qmodal_loop( 0 ), m_qmodal_showing( false ), m_qmodal_parent_disabler( 0 ) @@ -85,9 +86,7 @@ DIALOG_SHIM::DIALOG_SHIM( wxWindow* aParent, wxWindowID id, const wxString& titl Pgm().App().SetTopWindow( parent_kiwayplayer ); #endif -#if DLGSHIM_USE_SETFOCUS - Connect( wxEVT_INIT_DIALOG, wxInitDialogEventHandler( DIALOG_SHIM::onInit ) ); -#endif + Connect( wxEVT_PAINT, wxPaintEventHandler( DIALOG_SHIM::OnPaint ) ); } @@ -114,20 +113,6 @@ void DIALOG_SHIM::FinishDialogSettings() Center(); } -void DIALOG_SHIM::FixOSXCancelButtonIssue() -{ -#ifdef __WXMAC__ - // A ugly hack to fix an issue on OSX: ctrl+c closes the dialog instead of - // copying a text if a button with wxID_CANCEL is used in a wxStdDialogButtonSizer - // created by wxFormBuilder: the label is &Cancel, and this accelerator key has priority - // to copy text standard accelerator, and the dlg is closed when trying to copy text - wxButton* button = dynamic_cast< wxButton* > ( wxWindow::FindWindowById( wxID_CANCEL, this ) ); - - if( button ) - button->SetLabel( _( "Cancel" ) ); -#endif -} - // our hashtable is an implementation secret, don't need or want it in a header file #include @@ -175,8 +160,6 @@ bool DIALOG_SHIM::Show( bool show ) ret = wxDialog::Show( show ); } - FixOSXCancelButtonIssue(); - return ret; } @@ -194,58 +177,93 @@ bool DIALOG_SHIM::Enable( bool enable ) } -#if DLGSHIM_USE_SETFOCUS - -static bool findWindowRecursively( const wxWindowList& children, const wxWindow* wanted ) +// Traverse all items in the dialog. If selectTextInTextCtrls, do a SelectAll() +// in each so that tab followed by typing will replace the existing value. +// Also collects the firstTextCtrl and the item with focus (if any). +static void recursiveDescent( wxWindowList& children, const bool selectTextInTextCtrls, + wxWindow* & firstTextCtrl, wxWindow* & windowWithFocus ) { - for( wxWindowList::const_iterator it = children.begin(); it != children.end(); ++it ) + for( wxWindowList::iterator it = children.begin(); it != children.end(); ++it ) { - const wxWindow* child = *it; + wxWindow* child = *it; - if( wanted == child ) - return true; - else + if( child->HasFocus() ) + windowWithFocus = child; + + wxTextCtrl* childTextCtrl = dynamic_cast( child ); + if( childTextCtrl ) { - if( findWindowRecursively( child->GetChildren(), wanted ) ) - return true; + if( !firstTextCtrl && childTextCtrl->IsEnabled() && childTextCtrl->IsEditable() ) + firstTextCtrl = childTextCtrl; + + if( selectTextInTextCtrls ) + { + wxTextEntry* asTextEntry = dynamic_cast( childTextCtrl ); + // Respect an existing selection + if( asTextEntry->GetStringSelection().IsEmpty() ) + asTextEntry->SelectAll(); + } } - } - return false; + recursiveDescent( child->GetChildren(), selectTextInTextCtrls, firstTextCtrl, + windowWithFocus ); + } } - -static bool findWindowRecursively( const wxWindow* topmost, const wxWindow* wanted ) +#ifdef __WXMAC__ +static void fixOSXCancelButtonIssue( wxWindow *aWindow ) { - // wanted may be NULL and that is ok. + // A ugly hack to fix an issue on OSX: cmd+c closes the dialog instead of + // copying the text if a button with wxID_CANCEL is used in a + // wxStdDialogButtonSizer created by wxFormBuilder: the label is &Cancel, and + // this accelerator key has priority over the standard copy accelerator. + wxButton* button = dynamic_cast( wxWindow::FindWindowById( wxID_CANCEL, aWindow ) ); - if( wanted == topmost ) - return true; - - return findWindowRecursively( topmost->GetChildren(), wanted ); -} - - -/// Set the focus if it is not already set in a derived constructor to a specific control. -void DIALOG_SHIM::onInit( wxInitDialogEvent& aEvent ) -{ - wxWindow* focusWnd = wxWindow::FindFocus(); - - // If focusWnd is not already this window or a child of it, then SetFocus(). - // Otherwise the derived class's constructor SetFocus() already to a specific - // child control. - - if( !findWindowRecursively( this, focusWnd ) ) - { - // Linux wxGTK needs this to allow the ESCAPE key to close a wxDialog window. - SetFocus(); - } - - aEvent.Skip(); // derived class's handler should be called too + if( button ) + button->SetLabel( _( "Cancel" ) ); } #endif +void DIALOG_SHIM::OnPaint( wxPaintEvent &event ) +{ + if( !m_fixupsRun ) + { +#if DLGSHIM_SELECT_ALL_IN_TEXT_CONTROLS + const bool selectAllInTextCtrls = true; +#else + const bool selectAllInTextCtrls = false; +#endif + wxWindow* firstTextCtrl = NULL; + wxWindow* windowWithFocus = NULL; + + recursiveDescent( GetChildren(), selectAllInTextCtrls, firstTextCtrl, + windowWithFocus ); + +#if DLGSHIM_USE_SETFOCUS + // While it would be nice to honour any focus already set (which was + // recorded in windowWithFocus), the reality is that it's currently wrong + // far more often than it's right. + // So just focus on the first text control if we have one; otherwise the + // focus on the dialog itself, which will at least allow esc, return, etc. + // to function. + if( firstTextCtrl ) + firstTextCtrl->SetFocus(); + else + SetFocus(); +#endif + +#ifdef __WXMAC__ + fixOSXCancelButtonIssue( this ); +#endif + + m_fixupsRun = true; + } + + event.Skip(); +} + + /* Quasi-Modal Mode Explained: diff --git a/include/dialog_shim.h b/include/dialog_shim.h index 4effc36b9e..58e33973d6 100644 --- a/include/dialog_shim.h +++ b/include/dialog_shim.h @@ -38,6 +38,16 @@ #define DLGSHIM_USE_SETFOCUS 0 #endif +#ifdef __WXMAC__ +/** + * MACOS requires this option so that tabbing between text controls will + * arrive with the text selected. + **/ +#define DLGSHIM_SELECT_ALL_IN_TEXT_CONTROLS 1 +#else +#define DLGSHIM_SELECT_ALL_IN_TEXT_CONTROLS 0 +#endif + class WDO_ENABLE_DISABLE; class EVENT_LOOP; @@ -96,6 +106,8 @@ public: bool Enable( bool enable ) override; + void OnPaint( wxPaintEvent &event ); + protected: /** @@ -115,15 +127,7 @@ protected: */ void FinishDialogSettings(); - /** A ugly hack to fix an issue on OSX: - * when typing ctrl+c in a wxTextCtrl inside a dialog, it is closed instead of - * copying a text if a button with wxID_CANCEL is used in a wxStdDialogButtonSizer, - * when the dlg is created by wxFormBuilder: - * the label is &Cancel, and this accelerator key has priority - * to copy text standard accelerator, and the dlg is closed when trying to copy text - * this function do nothing on other platforms - */ - void FixOSXCancelButtonIssue(); + bool m_fixupsRun; std::string m_hash_key; // alternate for class_map when classname re-used. @@ -131,11 +135,6 @@ protected: EVENT_LOOP* m_qmodal_loop; // points to nested event_loop, NULL means not qmodal and dismissed bool m_qmodal_showing; WDO_ENABLE_DISABLE* m_qmodal_parent_disabler; - -#if DLGSHIM_USE_SETFOCUS -private: - void onInit( wxInitDialogEvent& aEvent ); -#endif }; #endif // DIALOG_SHIM_