Remove previous cell selection fixes in favour of slow-click hack.

wxWidgets has several bugs that result in cell editors being closed
right after they're opened.  There are two wxWidgets hacks to
partially address this: the SetInSetFocus() hack, and a slow-click
hack.  We used to try and work-around these bugs ourselves for
single-click access, but this changelist moves those over to
wxWidget's slow-click hack.

Fixes: lp:1817965
* https://bugs.launchpad.net/kicad/+bug/1817965
This commit is contained in:
Jeff Young 2019-03-04 11:02:12 +00:00
parent a128ffdea6
commit 69f003ba4a
16 changed files with 48 additions and 58 deletions

View File

@ -30,6 +30,7 @@
#include <html_messagebox.h> #include <html_messagebox.h>
#include <filename_resolver.h> #include <filename_resolver.h>
#include <env_vars.h> #include <env_vars.h>
#include <grid_tricks.h>
#include <widgets/wx_grid.h> #include <widgets/wx_grid.h>
#include <widgets/grid_text_button_helpers.h> #include <widgets/grid_text_button_helpers.h>

View File

@ -36,7 +36,7 @@
#define ROW_SEP wxT( '\n' ) #define ROW_SEP wxT( '\n' )
GRID_TRICKS::GRID_TRICKS( wxGrid* aGrid ): GRID_TRICKS::GRID_TRICKS( WX_GRID* aGrid ):
m_grid( aGrid ) m_grid( aGrid )
{ {
m_sel_row_start = 0; m_sel_row_start = 0;
@ -44,15 +44,12 @@ GRID_TRICKS::GRID_TRICKS( wxGrid* aGrid ):
m_sel_row_count = 0; m_sel_row_count = 0;
m_sel_col_count = 0; m_sel_col_count = 0;
m_showEditorOnMouseUp = false;
aGrid->Connect( wxEVT_GRID_CELL_LEFT_CLICK, wxGridEventHandler( GRID_TRICKS::onGridCellLeftClick ), NULL, this ); aGrid->Connect( wxEVT_GRID_CELL_LEFT_CLICK, wxGridEventHandler( GRID_TRICKS::onGridCellLeftClick ), NULL, this );
aGrid->Connect( wxEVT_GRID_CELL_LEFT_DCLICK, wxGridEventHandler( GRID_TRICKS::onGridCellLeftDClick ), NULL, this ); aGrid->Connect( wxEVT_GRID_CELL_LEFT_DCLICK, wxGridEventHandler( GRID_TRICKS::onGridCellLeftDClick ), NULL, this );
aGrid->Connect( wxEVT_GRID_CELL_RIGHT_CLICK, wxGridEventHandler( GRID_TRICKS::onGridCellRightClick ), NULL, this ); aGrid->Connect( wxEVT_GRID_CELL_RIGHT_CLICK, wxGridEventHandler( GRID_TRICKS::onGridCellRightClick ), NULL, this );
aGrid->Connect( wxEVT_GRID_LABEL_RIGHT_CLICK, wxGridEventHandler( GRID_TRICKS::onGridLabelRightClick ), NULL, this ); aGrid->Connect( wxEVT_GRID_LABEL_RIGHT_CLICK, wxGridEventHandler( GRID_TRICKS::onGridLabelRightClick ), NULL, this );
aGrid->Connect( GRIDTRICKS_FIRST_ID, GRIDTRICKS_LAST_ID, wxEVT_COMMAND_MENU_SELECTED, wxCommandEventHandler( GRID_TRICKS::onPopupSelection ), NULL, this ); aGrid->Connect( GRIDTRICKS_FIRST_ID, GRIDTRICKS_LAST_ID, wxEVT_COMMAND_MENU_SELECTED, wxCommandEventHandler( GRID_TRICKS::onPopupSelection ), NULL, this );
aGrid->Connect( wxEVT_KEY_DOWN, wxKeyEventHandler( GRID_TRICKS::onKeyDown ), NULL, this ); aGrid->Connect( wxEVT_KEY_DOWN, wxKeyEventHandler( GRID_TRICKS::onKeyDown ), NULL, this );
aGrid->GetGridWindow()->Connect( wxEVT_LEFT_UP, wxMouseEventHandler( GRID_TRICKS::onMouseUp ), NULL, this );
aGrid->Connect( wxEVT_UPDATE_UI, wxUpdateUIEventHandler( GRID_TRICKS::onUpdateUI ), NULL, this ); aGrid->Connect( wxEVT_UPDATE_UI, wxUpdateUIEventHandler( GRID_TRICKS::onUpdateUI ), NULL, this );
} }
@ -99,18 +96,29 @@ bool GRID_TRICKS::toggleCell( int aRow, int aCol )
bool GRID_TRICKS::showEditor( int aRow, int aCol ) bool GRID_TRICKS::showEditor( int aRow, int aCol )
{ {
m_grid->SetGridCursor( aRow, aCol ); if( m_grid->GetCursorRow() != aRow || m_grid->GetCursorColumn() != aCol )
m_grid->SetGridCursor( aRow, aCol );
if( m_grid->IsEditable() && !m_grid->IsReadOnly( aRow, aCol ) ) if( m_grid->IsEditable() && !m_grid->IsReadOnly( aRow, aCol ) )
{ {
if( m_grid->GetSelectionMode() == wxGrid::wxGridSelectRows ) if( m_grid->GetSelectionMode() == wxGrid::wxGridSelectRows )
m_grid->SelectRow( aRow ); {
wxArrayInt rows = m_grid->GetSelectedRows();
if( rows.size() != 1 || rows.at( 0 ) != aRow )
m_grid->SelectRow( aRow );
}
// For several reasons we can't enable the control here. There's the whole // For several reasons we can't enable the control here. There's the whole
// SetInSetFocus() issue/hack in wxWidgets, and there's also wxGrid's MouseUp // SetInSetFocus() issue/hack in wxWidgets, and there's also wxGrid's MouseUp
// handler which doesn't notice it's processing a MouseUp until after it has // handler which doesn't notice it's processing a MouseUp until after it has
// disabled the editor yet again. So we wait for the MouseUp. // disabled the editor yet again. So we re-use wxWidgets' slow-click hack,
m_showEditorOnMouseUp = true; // which is processed later in the MouseUp handler.
//
// It should be pointed out that the fact that it's wxWidgets' hack doesn't
// make it any less of a hack. Be extra careful with any modifications here.
// See, in particular, https://bugs.launchpad.net/kicad/+bug/1817965.
m_grid->ShowEditorOnMouseUp();
return true; return true;
} }
@ -124,10 +132,8 @@ void GRID_TRICKS::onGridCellLeftClick( wxGridEvent& aEvent )
int row = aEvent.GetRow(); int row = aEvent.GetRow();
int col = aEvent.GetCol(); int col = aEvent.GetCol();
// Activate editor only if a cursor is placed on the clicked cell // Don't make users click twice to toggle a checkbox or edit a text cell
if( !aEvent.GetModifiers() && if( !aEvent.GetModifiers() )
m_grid->GetGridCursorRow() == row &&
m_grid->GetGridCursorCol() == col )
{ {
if( toggleCell( row, col ) ) if( toggleCell( row, col ) )
return; return;
@ -147,33 +153,6 @@ void GRID_TRICKS::onGridCellLeftDClick( wxGridEvent& aEvent )
} }
void GRID_TRICKS::onMouseUp( wxMouseEvent& aEvent )
{
if( m_showEditorOnMouseUp )
{
// Some wxGridCellEditors don't have the SetInSetFocus() hack. Even when they do,
// it sometimes fails. Activating the control here seems to avoid those issues.
m_showEditorOnMouseUp = false;
// Mouse button can be pressed on one cell but be released on another
// cell (when range of cells is selecting, for example).
// So it must be checked.
wxGridCellCoords curCell = wxGridCellCoords( m_grid->GetGridCursorRow(),
m_grid->GetGridCursorCol() );
wxGridCellCoords eventCell = m_grid->XYToCell( m_grid->CalcUnscrolledPosition( aEvent.GetPosition() ) );
if( eventCell == curCell && m_grid->CanEnableCellControl() )
{
// Yes, the first of these also shows the control. Well, at least sometimes.
// The second call corrects those (as yet undefined) "other times".
m_grid->EnableCellEditControl();
m_grid->ShowCellEditControl();
return;
}
}
aEvent.Skip();
}
bool GRID_TRICKS::handleDoubleClick( wxGridEvent& aEvent ) bool GRID_TRICKS::handleDoubleClick( wxGridEvent& aEvent )
{ {
// Double-click processing must be handled by specific sub-classes // Double-click processing must be handled by specific sub-classes

View File

@ -25,7 +25,6 @@
#define KICAD_WX_GRID_H #define KICAD_WX_GRID_H
#include <wx/grid.h> #include <wx/grid.h>
#include <grid_tricks.h>
class WX_GRID : public wxGrid class WX_GRID : public wxGrid
{ {
@ -91,6 +90,14 @@ public:
*/ */
void EnsureColLabelsVisible(); void EnsureColLabelsVisible();
/**
* WxWidgets has a bunch of bugs in its handling of wxGrid mouse events which close cell
* editors right after opening them. Helpfully, it already has a bunch of work-arounds in
* place (such as the SetInSetFocus() hack), including one to make slow clicks work. We
* re-purpose this hack to work-around the bugs when we want to open an editor.
*/
void ShowEditorOnMouseUp() { m_waitForSlowClick = true; }
protected: protected:
void DrawColLabel( wxDC& dc, int col ) override; void DrawColLabel( wxDC& dc, int col ) override;

View File

@ -39,6 +39,7 @@
#include <dialog_edit_components_libid_base.h> #include <dialog_edit_components_libid_base.h>
#include <wx/tokenzr.h> #include <wx/tokenzr.h>
#include <grid_tricks.h>
#include <widgets/grid_text_button_helpers.h> #include <widgets/grid_text_button_helpers.h>
#define COL_REFS 0 #define COL_REFS 0

View File

@ -55,7 +55,7 @@ enum
class FIELDS_EDITOR_GRID_TRICKS : public GRID_TRICKS class FIELDS_EDITOR_GRID_TRICKS : public GRID_TRICKS
{ {
public: public:
FIELDS_EDITOR_GRID_TRICKS( DIALOG_SHIM* aParent, wxGrid* aGrid, FIELDS_EDITOR_GRID_TRICKS( DIALOG_SHIM* aParent, WX_GRID* aGrid,
wxDataViewListCtrl* aFieldsCtrl ) : wxDataViewListCtrl* aFieldsCtrl ) :
GRID_TRICKS( aGrid ), GRID_TRICKS( aGrid ),
m_dlg( aParent ), m_dlg( aParent ),

View File

@ -83,7 +83,7 @@ public:
class SYMBOL_GRID_TRICKS : public GRID_TRICKS class SYMBOL_GRID_TRICKS : public GRID_TRICKS
{ {
public: public:
SYMBOL_GRID_TRICKS( DIALOG_EDIT_LIBRARY_TABLES* aParent, wxGrid* aGrid ) : SYMBOL_GRID_TRICKS( DIALOG_EDIT_LIBRARY_TABLES* aParent, WX_GRID* aGrid ) :
GRID_TRICKS( aGrid ), GRID_TRICKS( aGrid ),
m_dialog( aParent ) m_dialog( aParent )
{ {

View File

@ -37,7 +37,7 @@ class DIALOG_SHIM;
class FIELDS_GRID_TRICKS : public GRID_TRICKS class FIELDS_GRID_TRICKS : public GRID_TRICKS
{ {
public: public:
FIELDS_GRID_TRICKS( wxGrid* aGrid, DIALOG_SHIM* aDialog ) : FIELDS_GRID_TRICKS( WX_GRID* aGrid, DIALOG_SHIM* aDialog ) :
GRID_TRICKS( aGrid ), GRID_TRICKS( aGrid ),
m_dlg( aDialog ) m_dlg( aDialog )
{} {}

View File

@ -28,7 +28,7 @@
#include <wx/grid.h> #include <wx/grid.h>
#include <wx/event.h> #include <wx/event.h>
#include <widgets/wx_grid.h>
enum enum
{ {
@ -46,25 +46,23 @@ enum
/** /**
* Class GRID_TRICKS * Class GRID_TRICKS
* is used to add cut, copy, and paste to an otherwise unmodied wxGrid instance. * is used to add mouse and command handling (such as cut, copy, and paste) to a WX_GRID instance.
*/ */
class GRID_TRICKS : public wxEvtHandler class GRID_TRICKS : public wxEvtHandler
{ {
public: public:
explicit GRID_TRICKS( wxGrid* aGrid ); explicit GRID_TRICKS( WX_GRID* aGrid );
protected: protected:
wxGrid* m_grid; ///< I don't own the grid, but he owns me WX_GRID* m_grid; ///< I don't own the grid, but he owns me
// row & col "selection" acquisition // row & col "selection" acquisition
// selected area by cell coordinate and count // selected area by cell coordinate and count
int m_sel_row_start; int m_sel_row_start;
int m_sel_col_start; int m_sel_col_start;
int m_sel_row_count; int m_sel_row_count;
int m_sel_col_count; int m_sel_col_count;
bool m_showEditorOnMouseUp;
/// Puts the selected area into a sensible rectangle of m_sel_{row,col}_{start,count} above. /// Puts the selected area into a sensible rectangle of m_sel_{row,col}_{start,count} above.
void getSelectedArea(); void getSelectedArea();
@ -80,7 +78,6 @@ protected:
void onGridLabelRightClick( wxGridEvent& event ); void onGridLabelRightClick( wxGridEvent& event );
void onPopupSelection( wxCommandEvent& event ); void onPopupSelection( wxCommandEvent& event );
void onKeyDown( wxKeyEvent& ev ); void onKeyDown( wxKeyEvent& ev );
void onMouseUp( wxMouseEvent& aEvent );
void onUpdateUI( wxUpdateUIEvent& event ); void onUpdateUI( wxUpdateUIEvent& event );
virtual bool handleDoubleClick( wxGridEvent& aEvent ); virtual bool handleDoubleClick( wxGridEvent& aEvent );

View File

@ -156,7 +156,7 @@ public:
class FP_GRID_TRICKS : public GRID_TRICKS class FP_GRID_TRICKS : public GRID_TRICKS
{ {
public: public:
FP_GRID_TRICKS( DIALOG_EDIT_LIBRARY_TABLES* aParent, wxGrid* aGrid ) : FP_GRID_TRICKS( DIALOG_EDIT_LIBRARY_TABLES* aParent, WX_GRID* aGrid ) :
GRID_TRICKS( aGrid ), GRID_TRICKS( aGrid ),
m_dialog( aParent ) m_dialog( aParent )
{ } { }

View File

@ -27,6 +27,7 @@
#include <widgets/paged_dialog.h> #include <widgets/paged_dialog.h>
#include <footprint_edit_frame.h> #include <footprint_edit_frame.h>
#include <widgets/wx_grid.h> #include <widgets/wx_grid.h>
#include <grid_tricks.h>
#include <panel_modedit_defaults.h> #include <panel_modedit_defaults.h>

View File

@ -32,6 +32,7 @@
#include <board_design_settings.h> #include <board_design_settings.h>
#include <bitmaps.h> #include <bitmaps.h>
#include <widgets/wx_grid.h> #include <widgets/wx_grid.h>
#include <grid_tricks.h>
#include <panel_setup_netclasses.h> #include <panel_setup_netclasses.h>

View File

@ -28,6 +28,7 @@
#include <pcb_edit_frame.h> #include <pcb_edit_frame.h>
#include <board_design_settings.h> #include <board_design_settings.h>
#include <widgets/wx_grid.h> #include <widgets/wx_grid.h>
#include <grid_tricks.h>
#include <panel_setup_text_and_graphics.h> #include <panel_setup_text_and_graphics.h>

View File

@ -29,6 +29,7 @@
#include <board_design_settings.h> #include <board_design_settings.h>
#include <bitmaps.h> #include <bitmaps.h>
#include <widgets/wx_grid.h> #include <widgets/wx_grid.h>
#include <grid_tricks.h>
#include <panel_setup_tracks_and_vias.h> #include <panel_setup_tracks_and_vias.h>

View File

@ -198,7 +198,7 @@ FOOTPRINT_WIZARD_FRAME::FOOTPRINT_WIZARD_FRAME( KIWAY* aKiway,
auto divider = new wxStaticLine( m_parametersPanel, wxID_ANY, auto divider = new wxStaticLine( m_parametersPanel, wxID_ANY,
wxDefaultPosition, wxDefaultSize, wxLI_VERTICAL ); wxDefaultPosition, wxDefaultSize, wxLI_VERTICAL );
m_parameterGrid = new wxGrid( m_parametersPanel, ID_FOOTPRINT_WIZARD_PARAMETER_LIST ); m_parameterGrid = new WX_GRID( m_parametersPanel, ID_FOOTPRINT_WIZARD_PARAMETER_LIST );
initParameterGrid(); initParameterGrid();
m_parameterGrid->PushEventHandler( new GRID_TRICKS( m_parameterGrid ) ); m_parameterGrid->PushEventHandler( new GRID_TRICKS( m_parameterGrid ) );

View File

@ -35,7 +35,7 @@
#include <footprint_wizard.h> #include <footprint_wizard.h>
class wxSashLayoutWindow; class wxSashLayoutWindow;
class wxListBox; class wxListBox;
class wxGrid; class WX_GRID;
class wxGridEvent; class wxGridEvent;
class FOOTPRINT_EDIT_FRAME; class FOOTPRINT_EDIT_FRAME;
@ -55,7 +55,7 @@ class FOOTPRINT_WIZARD_FRAME : public PCB_BASE_FRAME
private: private:
wxPanel* m_parametersPanel; ///< Panel for the page list and parameter grid wxPanel* m_parametersPanel; ///< Panel for the page list and parameter grid
wxListBox* m_pageList; ///< The list of pages wxListBox* m_pageList; ///< The list of pages
wxGrid* m_parameterGrid; ///< The list of parameters WX_GRID* m_parameterGrid; ///< The list of parameters
int m_parameterGridPage; ///< the page currently displayed by m_parameterGrid int m_parameterGridPage; ///< the page currently displayed by m_parameterGrid
///< it is most of time the m_pageList selection, but can differ ///< it is most of time the m_pageList selection, but can differ
///< during transitions between pages. ///< during transitions between pages.

View File

@ -42,6 +42,7 @@
#include <wildcards_and_files_ext.h> #include <wildcards_and_files_ext.h>
#include <dialogs/dialog_footprint_wizard_list.h> #include <dialogs/dialog_footprint_wizard_list.h>
#include <base_units.h> #include <base_units.h>
#include <widgets/wx_grid.h>
#include <tool/tool_manager.h> #include <tool/tool_manager.h>