From 0f78f972322af79bdcef9713646e8cb2eea583da Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Tue, 6 Mar 2018 05:33:04 +0000 Subject: [PATCH] Get rid of 5.0 dialog focus hacks. It's time to fix the focus issues. This adds a new SetInitialFocus() routine to DIALOG_SHIM which will need to be called from a lot of dialogs. (cherry picked from commit 6d9647a) --- common/dialog_shim.cpp | 76 ++++++------------- eeschema/dialogs/dialog_edit_label.cpp | 3 +- .../dialogs/dialog_fields_editor_global.cpp | 2 +- include/dialog_shim.h | 33 ++++---- pcbnew/dialogs/dialog_set_grid.cpp | 1 + .../dialogs/dialog_track_via_properties.cpp | 2 + 6 files changed, 46 insertions(+), 71 deletions(-) diff --git a/common/dialog_shim.cpp b/common/dialog_shim.cpp index 11dc62dbb6..de7283fd76 100644 --- a/common/dialog_shim.cpp +++ b/common/dialog_shim.cpp @@ -58,7 +58,8 @@ 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_firstPaintEvent( true ), + m_initialFocusTarget( nullptr ), m_qmodal_loop( 0 ), m_qmodal_showing( false ), m_qmodal_parent_disabler( 0 ) @@ -229,38 +230,29 @@ bool DIALOG_SHIM::Enable( bool enable ) } -// 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 ) +#ifdef __WXMAC__ +// Recursive descent doing a SelectAll() in wxTextCtrls. +// MacOS User Interface Guidelines state that when tabbing to a text control all its +// text should be selected. Since wxWidgets fails to implement this, we do it here. +static void selectAllInTextCtrls( wxWindowList& children ) { - for( wxWindowList::iterator it = children.begin(); it != children.end(); ++it ) + for( wxWindow* child : children ) { - wxWindow* child = *it; - - if( child->HasFocus() ) - windowWithFocus = child; - wxTextCtrl* childTextCtrl = dynamic_cast( child ); if( childTextCtrl ) { - if( !firstTextCtrl && childTextCtrl->IsEnabled() && childTextCtrl->IsEditable() ) - firstTextCtrl = childTextCtrl; + wxTextEntry* asTextEntry = dynamic_cast( childTextCtrl ); - if( selectTextInTextCtrls ) - { - wxTextEntry* asTextEntry = dynamic_cast( childTextCtrl ); - // Respect an existing selection - if( asTextEntry->GetStringSelection().IsEmpty() ) - asTextEntry->SelectAll(); - } + // Respect an existing selection + if( asTextEntry->GetStringSelection().IsEmpty() ) + asTextEntry->SelectAll(); } - - recursiveDescent( child->GetChildren(), selectTextInTextCtrls, firstTextCtrl, - windowWithFocus ); + else + selectAllInTextCtrls( child->GetChildren() ); } } +#endif + #ifdef __WXMAC__ static void fixOSXCancelButtonIssue( wxWindow *aWindow ) @@ -284,37 +276,19 @@ static void fixOSXCancelButtonIssue( wxWindow *aWindow ) void DIALOG_SHIM::OnPaint( wxPaintEvent &event ) { - if( !m_fixupsRun ) + if( m_firstPaintEvent ) { -#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__ +#ifdef __WXMAC__ fixOSXCancelButtonIssue( this ); + selectAllInTextCtrls( GetChildren() ); #endif - m_fixupsRun = true; + if( m_initialFocusTarget ) + m_initialFocusTarget->SetFocus(); + else + SetFocus(); // Focus the dialog itself + + m_firstPaintEvent = false; } event.Skip(); diff --git a/eeschema/dialogs/dialog_edit_label.cpp b/eeschema/dialogs/dialog_edit_label.cpp index 5ef8be77bd..37ab9b7833 100644 --- a/eeschema/dialogs/dialog_edit_label.cpp +++ b/eeschema/dialogs/dialog_edit_label.cpp @@ -154,6 +154,8 @@ DIALOG_LABEL_EDITOR::DIALOG_LABEL_EDITOR( SCH_EDIT_FRAME* aParent, SCH_TEXT* aTe m_textLabelMultiLine->Show( false ); } + SetInitialFocus( m_activeTextCtrl ); + if( m_CurrentText->Type() != SCH_TEXT_T ) ( (wxTextValidator*) m_activeTextCtrl->GetValidator() )->SetCharExcludes( wxT( " /" ) ); @@ -209,7 +211,6 @@ bool DIALOG_LABEL_EDITOR::TransferDataToWindow() return false; m_activeTextCtrl->SetValue( m_CurrentText->GetText() ); - m_activeTextCtrl->SetFocus(); // Set text options: int orientation = mapOrientation( m_CurrentText->Type(), m_CurrentText->GetLabelSpinStyle() ); diff --git a/eeschema/dialogs/dialog_fields_editor_global.cpp b/eeschema/dialogs/dialog_fields_editor_global.cpp index 43e0cc097c..836bb7fd79 100644 --- a/eeschema/dialogs/dialog_fields_editor_global.cpp +++ b/eeschema/dialogs/dialog_fields_editor_global.cpp @@ -611,7 +611,7 @@ DIALOG_FIELDS_EDITOR_GLOBAL::DIALOG_FIELDS_EDITOR_GLOBAL( SCH_EDIT_FRAME* parent m_grid->AutoSizeColumns( false ); m_grid->SetGridCursor( 0, 1 ); - m_grid->SetFocus(); + SetInitialFocus( m_grid ); m_sdbSizer1OK->SetDefault(); diff --git a/include/dialog_shim.h b/include/dialog_shim.h index a281b54960..6fc811c382 100644 --- a/include/dialog_shim.h +++ b/include/dialog_shim.h @@ -29,24 +29,7 @@ #include #include -#ifdef __WXMAC__ -/** - * MACOS requires this option to be set to 1 in order to set dialogs focus. - **/ -#define DLGSHIM_USE_SETFOCUS 1 -#else -#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; @@ -129,6 +112,15 @@ protected: */ void FinishDialogSettings(); + /** + * Sets the window (usually a wxTextCtrl) that should be focused when the dialog is + * shown. + */ + void SetInitialFocus( wxWindow* aWindow ) + { + m_initialFocusTarget = aWindow; + } + /** * Set the dialog to the given dimensions in "dialog units". These are units equivalent * to 4* the average character width and 8* the average character height, allowing a dialog @@ -149,9 +141,14 @@ protected: int VertPixelsFromDU( int y ); EDA_UNITS_T m_units; // userUnits for display and parsing - bool m_fixupsRun; // indicates various wxWidgets fixups have run std::string m_hash_key; // alternate for class_map when classname re-used + // On MacOS (at least) SetFocus() calls made in the constructor will fail because a + // window that isn't yet visible will return false to AcceptsFocus(). So we must delay + // the initial-focus SetFocus() call to the first paint event. + bool m_firstPaintEvent; + wxWindow* m_initialFocusTarget; + // variables for quasi-modal behavior support, only used by a few derivatives. EVENT_LOOP* m_qmodal_loop; // points to nested event_loop, NULL means not qmodal and dismissed bool m_qmodal_showing; diff --git a/pcbnew/dialogs/dialog_set_grid.cpp b/pcbnew/dialogs/dialog_set_grid.cpp index 642dd36815..498244ce76 100644 --- a/pcbnew/dialogs/dialog_set_grid.cpp +++ b/pcbnew/dialogs/dialog_set_grid.cpp @@ -82,6 +82,7 @@ DIALOG_SET_GRID::DIALOG_SET_GRID( PCB_BASE_FRAME* aParent, const wxArrayString& m_comboBoxGrid2->Append( m_fast_grid_opts ); m_sdbSizerOK->SetDefault(); // set OK button as default response to 'Enter' key + SetInitialFocus( m_GridOriginXCtrl ); Layout(); diff --git a/pcbnew/dialogs/dialog_track_via_properties.cpp b/pcbnew/dialogs/dialog_track_via_properties.cpp index 68813abe15..e2116dc60b 100644 --- a/pcbnew/dialogs/dialog_track_via_properties.cpp +++ b/pcbnew/dialogs/dialog_track_via_properties.cpp @@ -275,6 +275,8 @@ DIALOG_TRACK_VIA_PROPERTIES::DIALOG_TRACK_VIA_PROPERTIES( PCB_BASE_FRAME* aParen else m_lockedCbox->Set3StateValue( wxCHK_UNCHECKED ); + SetInitialFocus( m_tracks ? m_TrackStartXCtrl : m_ViaXCtrl ); + m_StdButtonsOK->SetDefault(); // Now all widgets have the size fixed, call FinishDialogSettings