From 31aebe6920c5cc6bf4eb302b4ff26d3e52969d92 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Wed, 7 Mar 2018 12:48:08 +0000 Subject: [PATCH] UI infrastructure enhancements and bug fixes. Work around wxWidgets failure to send first key through validator. Unify treatment of INDETERMINATE values (such as for multiple selections with mixed values). (cherry picked from commit 7308729) --- common/dialogs/dialog_env_var_config.cpp | 2 +- common/validators.cpp | 102 ++++++++++++++++- common/widgets/unit_binder.cpp | 107 +++++++++++------- .../dialogs/dialog_fields_editor_global.cpp | 10 +- .../dialogs/dialog_lib_edit_pin_table.cpp | 13 +-- eeschema/fields_grid_table.cpp | 8 +- eeschema/fields_grid_table.h | 1 - include/validators.h | 40 ++++++- include/widgets/unit_binder.h | 26 ++--- 9 files changed, 226 insertions(+), 83 deletions(-) diff --git a/common/dialogs/dialog_env_var_config.cpp b/common/dialogs/dialog_env_var_config.cpp index 017756f1f4..e29f6443d5 100644 --- a/common/dialogs/dialog_env_var_config.cpp +++ b/common/dialogs/dialog_env_var_config.cpp @@ -423,7 +423,7 @@ DIALOG_ENV_VAR_SINGLE::DIALOG_ENV_VAR_SINGLE( wxWindow* parent, { m_envVarName->SetValue( aEnvVarName ); m_envVarPath->SetValue( aEnvVarPath ); - m_envVarName->SetValidator( ENVIRONMENT_VARIABLE_CHAR_VALIDATOR() ); + m_envVarName->SetValidator( ENV_VAR_NAME_VALIDATOR() ); } diff --git a/common/validators.cpp b/common/validators.cpp index f105a557d5..94e8a0c861 100644 --- a/common/validators.cpp +++ b/common/validators.cpp @@ -30,10 +30,39 @@ #include #include +#include +#include #include #include +GRID_CELL_TEXT_EDITOR::GRID_CELL_TEXT_EDITOR() : wxGridCellTextEditor() +{ +} + + +void GRID_CELL_TEXT_EDITOR::SetValidator(const wxValidator& validator) +{ + // keep our own copy because wxGridCellTextEditor's is annoyingly private + m_validator.reset( static_cast( validator.Clone() ) ); + + wxGridCellTextEditor::SetValidator( validator ); +} + + +void GRID_CELL_TEXT_EDITOR::StartingKey( wxKeyEvent& event ) +{ + if( m_validator ) + { + m_validator.get()->SetWindow( Text() ); + m_validator.get()->ProcessEvent( event ); + } + + if( !event.WasProcessed() ) + wxGridCellTextEditor::StartingKey( event ); +} + + FILE_NAME_CHAR_VALIDATOR::FILE_NAME_CHAR_VALIDATOR( wxString* aValue ) : wxTextValidator( wxFILTER_EXCLUDE_CHAR_LIST, aValue ) { @@ -77,9 +106,74 @@ FILE_NAME_WITH_PATH_CHAR_VALIDATOR::FILE_NAME_WITH_PATH_CHAR_VALIDATOR( wxString } -ENVIRONMENT_VARIABLE_CHAR_VALIDATOR::ENVIRONMENT_VARIABLE_CHAR_VALIDATOR( wxString* aValue ) : - wxTextValidator( wxFILTER_INCLUDE_CHAR_LIST | wxFILTER_EMPTY, aValue ) +ENV_VAR_NAME_VALIDATOR::ENV_VAR_NAME_VALIDATOR( wxString* aValue ) : + wxTextValidator() { - wxString includeChars( wxT( "abcdefghijklmnopqrstuvwxyABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_" ) ); - SetCharIncludes( includeChars ); + Connect( wxEVT_CHAR, wxKeyEventHandler( ENV_VAR_NAME_VALIDATOR::OnChar ) ); +} + + +ENV_VAR_NAME_VALIDATOR::ENV_VAR_NAME_VALIDATOR( const ENV_VAR_NAME_VALIDATOR& val ) + : wxTextValidator() +{ + wxValidator::Copy( val ); + + Connect( wxEVT_CHAR, wxKeyEventHandler( ENV_VAR_NAME_VALIDATOR::OnChar ) ); +} + + +ENV_VAR_NAME_VALIDATOR::~ENV_VAR_NAME_VALIDATOR() +{ + Disconnect( wxEVT_CHAR, wxKeyEventHandler( ENV_VAR_NAME_VALIDATOR::OnChar ) ); +} + + +void ENV_VAR_NAME_VALIDATOR::OnChar( wxKeyEvent& aEvent ) +{ + if (!m_validatorWindow) + { + aEvent.Skip(); + return; + } + + int keyCode = aEvent.GetKeyCode(); + + // we don't filter special keys and delete + if (keyCode < WXK_SPACE || keyCode == WXK_DELETE || keyCode >= WXK_START) + { + aEvent.Skip(); + return; + } + + wxUniChar c = (wxUChar) keyCode; + + if( c == wxT( '_' ) ) + { + // OK anywhere + aEvent.Skip(); + } + else if( wxIsdigit( c ) ) + { + // not as first character + long from, to; + GetTextEntry()->GetSelection( &from, &to ); + if( from < 1 ) + wxBell(); + else + aEvent.Skip(); + } + else if( wxIsalpha( c ) ) + { + // Capitals only. + // Note: changing the keyCode and/or uniChar in the event and passing it on + // doesn't work. Some platforms look at the original copy as long as the event + // isn't vetoed. Insert the capitalized character by hand. + wxString s( c, 1 ); + s = s.Capitalize(); + GetTextEntry()->WriteText( s ); + } + else + { + wxBell(); + } } diff --git a/common/widgets/unit_binder.cpp b/common/widgets/unit_binder.cpp index eabaab88e3..dff050c4ea 100644 --- a/common/widgets/unit_binder.cpp +++ b/common/widgets/unit_binder.cpp @@ -32,11 +32,12 @@ #include "widgets/unit_binder.h" UNIT_BINDER::UNIT_BINDER( EDA_DRAW_FRAME* aParent, - wxStaticText* aLabel, wxTextEntry* aTextEntry, wxStaticText* aUnitLabel, + wxStaticText* aLabel, wxWindow* aValue, wxStaticText* aUnitLabel, bool aUseMils, int aMin, int aMax, bool allowEval ) : m_label( aLabel ), - m_textEntry( aTextEntry ), + m_value( aValue ), m_unitLabel( aUnitLabel ), + m_showMessage( true ), m_eval( aParent->GetUserUnits(), aUseMils ) { // Fix the units (to the current units) for the life of the binder @@ -44,31 +45,30 @@ UNIT_BINDER::UNIT_BINDER( EDA_DRAW_FRAME* aParent, m_useMils = aUseMils; m_min = aMin; m_max = aMax; - m_allowEval = allowEval; + m_allowEval = allowEval && dynamic_cast( m_value ); + + auto textEntry = dynamic_cast( m_value ); + if( textEntry ) + textEntry->SetValue( wxT( "0" ) ); - m_textEntry->SetValue( wxT( "0" ) ); m_unitLabel->SetLabel( GetAbbreviatedUnitsLabel( m_units, aUseMils ) ); - wxWindow* textInput = dynamic_cast( m_textEntry ); - textInput->Connect( wxEVT_SET_FOCUS, wxFocusEventHandler( UNIT_BINDER::onSetFocus ), NULL, this ); - textInput->Connect( wxEVT_KILL_FOCUS, wxFocusEventHandler( UNIT_BINDER::onKillFocus ), NULL, this ); - textInput->Connect( wxEVT_TEXT_ENTER, wxCommandEventHandler( UNIT_BINDER::onTextEnter ), NULL, this ); -} - - -UNIT_BINDER::~UNIT_BINDER() -{ + m_value->Connect( wxEVT_SET_FOCUS, wxFocusEventHandler( UNIT_BINDER::onSetFocus ), NULL, this ); + m_value->Connect( wxEVT_KILL_FOCUS, wxFocusEventHandler( UNIT_BINDER::onKillFocus ), NULL, this ); + m_value->Connect( wxEVT_TEXT_ENTER, wxCommandEventHandler( UNIT_BINDER::onTextEnter ), NULL, this ); } void UNIT_BINDER::onSetFocus( wxFocusEvent& aEvent ) { - if( m_allowEval ) + auto textEntry = dynamic_cast( m_value ); + + if( m_allowEval && textEntry ) { wxString oldStr = m_eval.OriginalText(); if( oldStr.length() ) - m_textEntry->SetValue( oldStr ); + textEntry->SetValue( oldStr ); } aEvent.Skip(); @@ -97,17 +97,22 @@ void UNIT_BINDER::onTextEnter( wxCommandEvent& aEvent ) // Send an OK event to the parent dialog wxCommandEvent event( wxEVT_COMMAND_BUTTON_CLICKED, wxID_OK ); - wxPostEvent( dynamic_cast( m_textEntry )->GetParent(), event ); + wxPostEvent( m_value->GetParent(), event ); } void UNIT_BINDER::evaluate() { - if( m_textEntry->GetValue().IsEmpty() ) - m_textEntry->ChangeValue( "0" ); + auto textEntry = dynamic_cast( m_value ); - if( m_eval.Process( m_textEntry->GetValue() ) ) - m_textEntry->ChangeValue( m_eval.Result() ); + if( !textEntry ) + return; + + if( textEntry->GetValue().IsEmpty() ) + textEntry->ChangeValue( "0" ); + + if( m_eval.Process( textEntry->GetValue() ) ) + textEntry->ChangeValue( m_eval.Result() ); } @@ -122,29 +127,36 @@ wxString valueDescriptionFromLabel( wxStaticText* aLabel ) void UNIT_BINDER::delayedFocusHandler( wxIdleEvent& ) { - wxWindow* textInput = dynamic_cast( m_textEntry ); - textInput->SetFocus(); + m_value->SetFocus(); + m_showMessage = true; - textInput->Unbind( wxEVT_IDLE, &UNIT_BINDER::delayedFocusHandler, this ); + m_value->Unbind( wxEVT_IDLE, &UNIT_BINDER::delayedFocusHandler, this ); } bool UNIT_BINDER::Validate( bool setFocusOnError ) { - wxWindow* textInput = dynamic_cast( m_textEntry ); + auto textEntry = dynamic_cast( m_value ); + + if( !textEntry || textEntry->GetValue() == INDETERMINATE ) + return true; if( m_min > INT_MIN && GetValue() < m_min ) { - wxString msg = wxString::Format( _( "%s must be larger than %s." ), - valueDescriptionFromLabel( m_label ), - StringFromValue( m_units, m_min, true ) ); - DisplayError( textInput->GetParent(), msg ); + if( m_showMessage ) + { + wxString msg = wxString::Format( _( "%s must be larger than %s." ), + valueDescriptionFromLabel( m_label ), + StringFromValue( m_units, m_min, true ) ); + DisplayError( m_value->GetParent(), msg ); + } if( setFocusOnError ) { - m_textEntry->SelectAll(); + textEntry->SelectAll(); // Don't focus directly; we might be inside a KillFocus event handler - textInput->Bind( wxEVT_IDLE, &UNIT_BINDER::delayedFocusHandler, this ); + m_value->Bind( wxEVT_IDLE, &UNIT_BINDER::delayedFocusHandler, this ); + m_showMessage = false; } return false; @@ -152,16 +164,20 @@ bool UNIT_BINDER::Validate( bool setFocusOnError ) if( m_max < INT_MAX && GetValue() > m_max ) { - wxString msg = wxString::Format( _( "%s must be smaller than %s." ), - valueDescriptionFromLabel( m_label ), - StringFromValue( m_units, m_max, true ) ); - DisplayError( textInput->GetParent(), msg ); + if( m_showMessage ) + { + wxString msg = wxString::Format( _( "%s must be smaller than %s." ), + valueDescriptionFromLabel( m_label ), + StringFromValue( m_units, m_max, true ) ); + DisplayError( m_value->GetParent(), msg ); + } if( setFocusOnError ) { - m_textEntry->SelectAll(); + textEntry->SelectAll(); // Don't focus directly; we might be inside a KillFocus event handler - textInput->Bind( wxEVT_IDLE, &UNIT_BINDER::delayedFocusHandler, this ); + m_value->Bind( wxEVT_IDLE, &UNIT_BINDER::delayedFocusHandler, this ); + m_showMessage = false; } return false; @@ -179,7 +195,10 @@ void UNIT_BINDER::SetValue( int aValue ) void UNIT_BINDER::SetValue( wxString aValue ) { - m_textEntry->SetValue( aValue ); + if( dynamic_cast( m_value ) ) + dynamic_cast( m_value )->SetValue( aValue ); + else if( dynamic_cast( m_value ) ) + dynamic_cast( m_value )->SetLabel( aValue ); if( m_allowEval ) m_eval.Clear(); @@ -190,7 +209,12 @@ void UNIT_BINDER::SetValue( wxString aValue ) int UNIT_BINDER::GetValue() const { - wxString s = m_textEntry->GetValue(); + wxString s; + + if( dynamic_cast( m_value ) ) + s = dynamic_cast( m_value )->GetValue(); + else if( dynamic_cast( m_value ) ) + s = dynamic_cast( m_value )->GetLabel(); return ValueFromString( m_units, s, m_useMils ); } @@ -198,14 +222,17 @@ int UNIT_BINDER::GetValue() const bool UNIT_BINDER::IsIndeterminate() const { - return m_textEntry->GetValue() == INDETERMINATE; + if( dynamic_cast( m_value ) ) + return dynamic_cast( m_value )->GetValue() == INDETERMINATE; + + return false; } void UNIT_BINDER::Enable( bool aEnable ) { m_label->Enable( aEnable ); - dynamic_cast( m_textEntry )->Enable( aEnable ); + m_value->Enable( aEnable ); m_unitLabel->Enable( aEnable ); } diff --git a/eeschema/dialogs/dialog_fields_editor_global.cpp b/eeschema/dialogs/dialog_fields_editor_global.cpp index 67b83848bc..e5029ce670 100644 --- a/eeschema/dialogs/dialog_fields_editor_global.cpp +++ b/eeschema/dialogs/dialog_fields_editor_global.cpp @@ -25,16 +25,14 @@ #include #include - +#include #include #include #include #include - #include #include #include - #include #include #include @@ -102,10 +100,6 @@ struct DATA_MODEL_ROW #endif -// Indicator that multiple values exist in child rows -#define ROW_MULT_ITEMS wxString( "< ... >" ) - - class FIELDS_EDITOR_GRID_DATA_MODEL : public wxGridTableBase { protected: @@ -221,7 +215,7 @@ public: if( &ref == &group.m_Refs.front() ) fieldValue = m_dataStore[ compID ][ m_fieldNames[ aCol ] ]; else if ( fieldValue != m_dataStore[ compID ][ m_fieldNames[ aCol ] ] ) - return ROW_MULT_ITEMS; + return INDETERMINATE; } } diff --git a/eeschema/dialogs/dialog_lib_edit_pin_table.cpp b/eeschema/dialogs/dialog_lib_edit_pin_table.cpp index 671535eae7..6e44a2eb49 100644 --- a/eeschema/dialogs/dialog_lib_edit_pin_table.cpp +++ b/eeschema/dialogs/dialog_lib_edit_pin_table.cpp @@ -34,9 +34,6 @@ #include -// Indicator that multiple values exist in child rows -#define ROW_MULT_ITEMS wxString( "< ... >" ) - #define PinTableShownColumnsKey wxT( "PinTableShownColumns" ) @@ -156,7 +153,7 @@ public: if( !fieldValue.Length() ) fieldValue = val; else if( val != fieldValue ) - fieldValue = ROW_MULT_ITEMS; + fieldValue = INDETERMINATE; } } @@ -165,7 +162,7 @@ public: void SetValue( int aRow, int aCol, const wxString &aValue ) override { - if( aValue == ROW_MULT_ITEMS ) + if( aValue == INDETERMINATE ) return; LIB_PINS pins = m_rows[ aRow ]; @@ -340,18 +337,18 @@ DIALOG_LIB_EDIT_PIN_TABLE::DIALOG_LIB_EDIT_PIN_TABLE( wxWindow* parent, LIB_PART g_typeIcons.push_back( GetBitmap( static_cast( i ) ) ); for( unsigned i = 0; i < PINTYPE_COUNT; ++i ) g_typeNames.push_back( GetText( static_cast( i ) ) ); - g_typeNames.push_back( ROW_MULT_ITEMS ); + g_typeNames.push_back( INDETERMINATE ); for( unsigned i = 0; i < PINSHAPE_COUNT; ++i ) g_shapeIcons.push_back( GetBitmap( static_cast( i ) ) ); for( unsigned i = 0; i < PINSHAPE_COUNT; ++i ) g_shapeNames.push_back( GetText( static_cast( i ) ) ); - g_shapeNames.push_back( ROW_MULT_ITEMS ); + g_shapeNames.push_back( INDETERMINATE ); for( unsigned i = 0; i < LIB_PIN::GetOrientationNames().size(); ++i ) g_orientationIcons.push_back( LIB_PIN::GetOrientationSymbols()[ i ] ); g_orientationNames = LIB_PIN::GetOrientationNames(); - g_orientationNames.push_back( ROW_MULT_ITEMS ); + g_orientationNames.push_back( INDETERMINATE ); } m_dataModel = new PIN_TABLE_DATA_MODEL( GetUserUnits() ); diff --git a/eeschema/fields_grid_table.cpp b/eeschema/fields_grid_table.cpp index 7a1eebab1d..b0100a086f 100644 --- a/eeschema/fields_grid_table.cpp +++ b/eeschema/fields_grid_table.cpp @@ -25,6 +25,8 @@ #include #include #include +#include +#include #include #include #include @@ -53,9 +55,9 @@ FIELDS_GRID_TABLE::FIELDS_GRID_TABLE( bool aInLibEdit, EDA_UNITS_T aUserUnits m_readOnlyAttr->SetReadOnly( true ); m_valueColAttr = new wxGridCellAttr; - m_valueTextEditor = new wxGridCellTextEditor(); - m_valueTextEditor->SetValidator( m_valueValidator ); - m_valueColAttr->SetEditor( m_valueTextEditor ); + GRID_CELL_TEXT_EDITOR* textEditor = new GRID_CELL_TEXT_EDITOR(); + textEditor->SetValidator( m_valueValidator ); + m_valueColAttr->SetEditor( textEditor ); m_boolColAttr = new wxGridCellAttr; m_boolColAttr->SetRenderer( new wxGridCellBoolRenderer() ); diff --git a/eeschema/fields_grid_table.h b/eeschema/fields_grid_table.h index 5c2059b7b0..6e6fe46f2b 100644 --- a/eeschema/fields_grid_table.h +++ b/eeschema/fields_grid_table.h @@ -96,7 +96,6 @@ private: LIB_PART* m_part; bool m_inLibEdit; - wxGridCellTextEditor* m_valueTextEditor; SCH_FIELD_VALIDATOR m_valueValidator; wxGridCellAttr* m_readOnlyAttr; diff --git a/include/validators.h b/include/validators.h index f08fe0865c..cbf44ac2d5 100644 --- a/include/validators.h +++ b/include/validators.h @@ -31,6 +31,26 @@ #define VALIDATORS_H #include +#include + +/** + * Class GRID_CELL_TEXT_EDITOR + * + * This class works around a bug in wxGrid where the first keystroke doesn't get sent through + * the validator if the editor wasn't already open. + */ +class GRID_CELL_TEXT_EDITOR : public wxGridCellTextEditor +{ +public: + GRID_CELL_TEXT_EDITOR(); + + virtual void SetValidator(const wxValidator& validator) override; + virtual void StartingKey(wxKeyEvent& event) override; + +protected: + wxScopedPtr m_validator; +}; + /** * Class FILE_NAME_CHAR_VALIDATOR @@ -63,19 +83,31 @@ public: /** - * Class ENVIRONMENT_VARIABLE_CHAR_VALIDATOR + * Class ENV_VAR_NAME_VALIDATOR * - * This class provides a custome wxValidator object for limiting the allowable characters + * This class provides a custom wxValidator object for limiting the allowable characters * when defining an environment varaible name in a text edit control. Only uppercase, * numbers, and underscore (_) characters are valid and the first character of the name * cannot start with a number. This is according to IEEE Std 1003.1-2001. Even though * most systems support other characters, these characters guarantee compatibility for * all shells. */ -class ENVIRONMENT_VARIABLE_CHAR_VALIDATOR : public wxTextValidator +class ENV_VAR_NAME_VALIDATOR : public wxTextValidator { public: - ENVIRONMENT_VARIABLE_CHAR_VALIDATOR( wxString* aValue = NULL ); + ENV_VAR_NAME_VALIDATOR( wxString* aValue = NULL ); + ENV_VAR_NAME_VALIDATOR( const ENV_VAR_NAME_VALIDATOR& val ); + + virtual ~ENV_VAR_NAME_VALIDATOR(); + + // Make a clone of this validator (or return NULL) - currently necessary + // if you're passing a reference to a validator. + virtual wxObject *Clone() const override + { + return new ENV_VAR_NAME_VALIDATOR( *this ); + } + + void OnChar(wxKeyEvent& event); }; #endif // #ifndef VALIDATORS_H diff --git a/include/widgets/unit_binder.h b/include/widgets/unit_binder.h index bd65c426dc..2ef91f597a 100644 --- a/include/widgets/unit_binder.h +++ b/include/widgets/unit_binder.h @@ -44,8 +44,8 @@ public: * @param aParent is the parent EDA_DRAW_FRAME. * @param aLabel is the static text used to label the text input widget (note: the label * text, trimmed of its colon, will also be used in error messages) - * @param aTextEntry is the text input widget used to edit the given value (wxTextCtrl, - * wxComboBox, ...). + * @param aValue is the control used to edit or display the given value (wxTextCtrl, + * wxComboBox, wxStaticText, etc.). * @param aUnitLabel is the units label displayed after the text input widget * @param aUseMils specifies the use of mils for imperial units (instead of inches) * @param aMin a minimum value (in internal units) for validation @@ -53,13 +53,11 @@ public: * @param aAllowEval indicates \a aTextInput's content should be eval'ed before storing */ UNIT_BINDER( EDA_DRAW_FRAME* aParent, - wxStaticText* aLabel, wxTextEntry* aTextEntry, wxStaticText* aUnitLabel, + wxStaticText* aLabel, wxWindow* aValue, wxStaticText* aUnitLabel, bool aUseMils = false, int aMin = INT_MIN, int aMax = INT_MAX, bool aAllowEval = true ); - virtual ~UNIT_BINDER(); - /** * Function SetValue * Sets new value (in Internal Units) for the text field, taking care of units conversion. @@ -99,7 +97,6 @@ public: protected: - void onTextChanged( wxEvent& aEvent ); void onSetFocus( wxFocusEvent& aEvent ); void onKillFocus( wxFocusEvent& aEvent ); void onTextEnter( wxCommandEvent& aEvent ); @@ -108,21 +105,22 @@ protected: void evaluate(); ///> The bound widgets - wxStaticText* m_label; - wxTextEntry* m_textEntry; - wxStaticText* m_unitLabel; + wxStaticText* m_label; + wxWindow* m_value; + wxStaticText* m_unitLabel; ///> Currently used units. - EDA_UNITS_T m_units; - bool m_useMils; + EDA_UNITS_T m_units; + bool m_useMils; ///> Validation support. - int m_min; - int m_max; + int m_min; + int m_max; + bool m_showMessage; ///> Evaluator NUMERIC_EVALUATOR m_eval; - bool m_allowEval; + bool m_allowEval; }; #endif /* __UNIT_BINDER_H_ */