From c2e4bc422cea0662602a727a8a56e1457574a71b Mon Sep 17 00:00:00 2001 From: Jorge Moraleda Date: Sun, 12 Apr 2020 23:15:21 -0700 Subject: [PATCH 01/15] Make HasValue virtual in wxDataViewModel and have implementations use it. This addresses issue https://trac.wxwidgets.org/ticket/18724 --- include/wx/dataview.h | 2 +- interface/wx/dataview.h | 7 ++- src/generic/datavgen.cpp | 120 ++++++++++++++++++++++----------------- src/gtk/dataview.cpp | 11 +--- 4 files changed, 74 insertions(+), 66 deletions(-) diff --git a/include/wx/dataview.h b/include/wx/dataview.h index 99b620d84d..e796ab46c7 100644 --- a/include/wx/dataview.h +++ b/include/wx/dataview.h @@ -208,7 +208,7 @@ public: // return true if the given item has a value to display in the given // column: this is always true except for container items which by default // only show their label in the first column (but see HasContainerColumns()) - bool HasValue(const wxDataViewItem& item, unsigned col) const + virtual bool HasValue(const wxDataViewItem& item, unsigned col) const { return col == 0 || !IsContainer(item) || HasContainerColumns(item); } diff --git a/interface/wx/dataview.h b/interface/wx/dataview.h index f9decd1a4c..f2857904e0 100644 --- a/interface/wx/dataview.h +++ b/interface/wx/dataview.h @@ -304,14 +304,17 @@ public: All normal items have values in all columns but the container items only show their label in the first column (@a col == 0) by default (but - see HasContainerColumns()). So this function always returns true for + see HasContainerColumns()). So this function by default returns true for the first column while for the other ones it returns true only if the item is not a container or HasContainerColumns() was overridden to return true for it. + Override this method to explicitly specify for which columns a given + item has values. + @since 2.9.1 */ - bool HasValue(const wxDataViewItem& item, unsigned col) const; + virtual bool HasValue(const wxDataViewItem& item, unsigned col) const; /** Override this to indicate of @a item is a container, i.e.\ if diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index 7cf49aefa4..ef55a9f8c2 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -2394,15 +2394,17 @@ void wxDataViewMainWindow::OnPaint( wxPaintEvent &WXUNUSED(event) ) if ( m_useCellFocus && m_currentCol && m_currentColSetByKeyboard ) { - renderColumnFocus = true; - - // If this is container node without columns, render full-row focus: - if ( !IsList() ) + // If this is an item with a single column, render full-row focus: + numColsWithValue = 0; + unsigned int cols = GetOwner()->GetColumnCount(); + for ( unsigned int i = 0; i < cols; i++ ) { - wxDataViewTreeNode *node = GetTreeNodeByRow(item); - if ( node->HasChildren() && !model->HasContainerColumns(node->GetItem()) ) - renderColumnFocus = false; + if ( model->HasValue(item, i) ) + numColsWithValue++; + if ( numColsWithValue > 1 ) + break; } + renderColumnFocus = numColsWithValue<2; } if ( renderColumnFocus ) @@ -2547,11 +2549,8 @@ void wxDataViewMainWindow::OnPaint( wxPaintEvent &WXUNUSED(event) ) dataitem = node->GetItem(); - // Skip all columns of "container" rows except the expander - // column itself unless HasContainerColumns() overrides this. - if ( col != expander && - model->IsContainer(dataitem) && - !model->HasContainerColumns(dataitem) ) + // Skip al columns that do not have values + if ( !model->HasValue(dataitem, col->GetModelColumn()) ) { cell_rect.y += line_height; continue; @@ -3491,9 +3490,7 @@ int wxDataViewMainWindow::QueryAndCacheLineHeight(unsigned int row, wxDataViewIt if (column->IsHidden()) continue; // skip it! - if ((col != 0) && - model->IsContainer(item) && - !model->HasContainerColumns(item)) + if ( !model->HasValue(item, col) ) continue; // skip it! wxDataViewRenderer *renderer = @@ -4108,8 +4105,7 @@ wxDataViewMainWindow::FindColumnForEditing(const wxDataViewItem& item, wxDataVie // may be directly editable: if ( candidate && GetOwner()->GetExpanderColumn() != candidate && - GetModel()->IsContainer(item) && - !GetModel()->HasContainerColumns(item) ) + !GetModel()->HasValue(item, candidate->GetModelColumn()) ) { candidate = GetOwner()->GetExpanderColumn(); } @@ -4500,8 +4496,13 @@ bool wxDataViewMainWindow::TryAdvanceCurrentColumn(wxDataViewTreeNode *node, wxK if ( node ) { - // navigation shouldn't work in branch nodes without other columns: - if ( node->HasChildren() && !GetModel()->HasContainerColumns(node->GetItem()) ) + // navigation shouldn't work in nodes with fewer than two columns + numColsWithValue = 0; + unsigned int cols = GetOwner()->GetColumnCount(); + for ( unsigned int i = 0; i < cols; i++ ) + if ( GetModel()->HasValue(node->GetItem(), i) ) + numColsWithValue++; + if (numColsWithValue<2) return false; } @@ -4509,7 +4510,13 @@ bool wxDataViewMainWindow::TryAdvanceCurrentColumn(wxDataViewTreeNode *node, wxK { if ( forward ) { - m_currentCol = GetOwner()->GetColumnAt(0); + // find first column with value + unsigned int i = 0; + unsigned int cols = GetOwner()->GetColumnCount(); + for ( ; i < cols; i++ ) + if ( GetModel()->HasValue(node->GetItem(), i) ) + break; + m_currentCol = GetOwner()->GetColumnAt(i); m_currentColSetByKeyboard = true; RefreshRow(m_currentRow); return true; @@ -4521,41 +4528,49 @@ bool wxDataViewMainWindow::TryAdvanceCurrentColumn(wxDataViewTreeNode *node, wxK } } - int idx = GetOwner()->GetColumnIndex(m_currentCol) + (forward ? +1 : -1); - - if ( idx >= (int)GetOwner()->GetColumnCount() ) + int idx = GetOwner()->GetColumnIndex(m_currentCol); + unsigned int cols = GetOwner()->GetColumnCount(); + for ( unsigned int i = 0; i < cols; i++ ) { - if ( !wrapAround ) - return false; + idx += (forward ? +1 : -1); + if ( idx >= (int)GetOwner()->GetColumnCount() ) + { + if ( !wrapAround ) + return false; - if ( GetCurrentRow() < GetRowCount() - 1 ) - { - // go to the first column of the next row: - idx = 0; - OnVerticalNavigation(wxKeyEvent()/*dummy*/, +1); + if ( GetCurrentRow() < GetRowCount() - 1 ) + { + // go to the first column of the next row: + idx = 0; + OnVerticalNavigation(wxKeyEvent()/*dummy*/, +1); + } + else + { + // allow focus change + event.Skip(); + return false; + } } - else + else if ( idx < 0 ) { - // allow focus change - event.Skip(); - return false; - } - } + if ( !wrapAround ) + return false; - if ( idx < 0 && wrapAround ) - { - if ( GetCurrentRow() > 0 ) - { - // go to the last column of the previous row: - idx = (int)GetOwner()->GetColumnCount() - 1; - OnVerticalNavigation(wxKeyEvent()/*dummy*/, -1); - } - else - { - // allow focus change - event.Skip(); - return false; + if ( GetCurrentRow() > 0 ) + { + // go to the last column of the previous row: + idx = col_last-1; + OnVerticalNavigation(wxKeyEvent()/*dummy*/, -1); + } + else + { + // allow focus change + event.Skip(); + return false; + } } + if ( GetModel()->HasValue(node->GetItem(), i) ) + break; } GetOwner()->EnsureVisibleRowCol(m_currentRow, idx); @@ -4767,9 +4782,8 @@ void wxDataViewMainWindow::OnMouse( wxMouseEvent &event ) } bool ignore_other_columns = - ((expander != col) && - (model->IsContainer(item)) && - (!model->HasContainerColumns(item))); + (expander != col) && + (!model->HasValue(item, col->GetModelColumn())); if (event.LeftDClick()) { @@ -6495,7 +6509,7 @@ wxAccStatus wxDataViewCtrlAccessible::GetDescription(int childId, wxString* desc const unsigned int numCols = dvCtrl->GetColumnCount(); for ( unsigned int col = 0; col < numCols; col++ ) { - if ( model->IsContainer(item) && !model->HasContainerColumns(item) ) + if ( !model->HasValue(item, col) ) continue; // skip it wxDataViewColumn *dvCol = dvCtrl->GetColumnAt(col); diff --git a/src/gtk/dataview.cpp b/src/gtk/dataview.cpp index 17c68517b5..b8ed3c8f3d 100644 --- a/src/gtk/dataview.cpp +++ b/src/gtk/dataview.cpp @@ -3146,16 +3146,7 @@ static void wxGtkTreeCellDataFunc( GtkTreeViewColumn *WXUNUSED(column), if (!wx_model->IsVirtualListModel()) { - gboolean visible; - if (wx_model->IsContainer( item )) - { - visible = wx_model->HasContainerColumns( item ) || (column == 0); - } - else - { - visible = true; - } - + gboolean visible = wx_model->HasValue(item, column); GValue gvalue = G_VALUE_INIT; g_value_init( &gvalue, G_TYPE_BOOLEAN ); g_value_set_boolean( &gvalue, visible ); From ea4a6ec0e9bde2eaa188caa1cda900d9843d86e7 Mon Sep 17 00:00:00 2001 From: Jorge Moraleda Date: Mon, 13 Apr 2020 16:35:58 -0700 Subject: [PATCH 02/15] Bug fix. Ensure travis builds without errors on all platforms. --- src/generic/datavgen.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index ef55a9f8c2..3cb2d796e9 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -2395,11 +2395,12 @@ void wxDataViewMainWindow::OnPaint( wxPaintEvent &WXUNUSED(event) ) if ( m_useCellFocus && m_currentCol && m_currentColSetByKeyboard ) { // If this is an item with a single column, render full-row focus: - numColsWithValue = 0; + unsigned int numColsWithValue = 0; unsigned int cols = GetOwner()->GetColumnCount(); for ( unsigned int i = 0; i < cols; i++ ) { - if ( model->HasValue(item, i) ) + wxDataViewTreeNode *node = GetTreeNodeByRow(item); + if ( model->HasValue(node->GetItem(), i) ) numColsWithValue++; if ( numColsWithValue > 1 ) break; @@ -4497,7 +4498,7 @@ bool wxDataViewMainWindow::TryAdvanceCurrentColumn(wxDataViewTreeNode *node, wxK if ( node ) { // navigation shouldn't work in nodes with fewer than two columns - numColsWithValue = 0; + unsigned int numColsWithValue = 0; unsigned int cols = GetOwner()->GetColumnCount(); for ( unsigned int i = 0; i < cols; i++ ) if ( GetModel()->HasValue(node->GetItem(), i) ) @@ -4559,7 +4560,7 @@ bool wxDataViewMainWindow::TryAdvanceCurrentColumn(wxDataViewTreeNode *node, wxK if ( GetCurrentRow() > 0 ) { // go to the last column of the previous row: - idx = col_last-1; + idx = (int)GetOwner()->GetColumnCount() - 1; OnVerticalNavigation(wxKeyEvent()/*dummy*/, -1); } else From 8a3b0a9fd618d3f0818b51b5d416d7b15de5ca9f Mon Sep 17 00:00:00 2001 From: Jorge Moraleda Date: Mon, 13 Apr 2020 18:40:31 -0700 Subject: [PATCH 03/15] Consolidate common logic into a single function and revert original test for IsList() to minimize code changes. --- src/generic/datavgen.cpp | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index 3cb2d796e9..3dd5e27a08 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -910,6 +910,22 @@ private: // assumes that all columns were modified, otherwise just this one. bool DoItemChanged(const wxDataViewItem& item, int view_column); + // Return whether the item has more than one column that have values + bool HasItemMultipleValues(const wxDataViewItem& item) + { + unsigned int numColsWithValue = 0; + unsigned int cols = GetOwner()->GetColumnCount(); + wxDataViewModel* model = GetModel(); + for ( unsigned int i = 0; i < cols; i++ ) + { + if ( model->HasValue(item, i) ) + numColsWithValue++; + if ( numColsWithValue > 1 ) + return true; + } + return false; + } + private: wxDataViewCtrl *m_owner; int m_lineHeight; @@ -2394,18 +2410,15 @@ void wxDataViewMainWindow::OnPaint( wxPaintEvent &WXUNUSED(event) ) if ( m_useCellFocus && m_currentCol && m_currentColSetByKeyboard ) { - // If this is an item with a single column, render full-row focus: - unsigned int numColsWithValue = 0; - unsigned int cols = GetOwner()->GetColumnCount(); - for ( unsigned int i = 0; i < cols; i++ ) + renderColumnFocus = true; + + // If this is container node without columns, render full-row focus: + if ( !IsList() ) { wxDataViewTreeNode *node = GetTreeNodeByRow(item); - if ( model->HasValue(node->GetItem(), i) ) - numColsWithValue++; - if ( numColsWithValue > 1 ) - break; + if ( !HasItemMultipleValues(node->GetItem()) ) + renderColumnFocus = false; } - renderColumnFocus = numColsWithValue<2; } if ( renderColumnFocus ) @@ -4498,12 +4511,7 @@ bool wxDataViewMainWindow::TryAdvanceCurrentColumn(wxDataViewTreeNode *node, wxK if ( node ) { // navigation shouldn't work in nodes with fewer than two columns - unsigned int numColsWithValue = 0; - unsigned int cols = GetOwner()->GetColumnCount(); - for ( unsigned int i = 0; i < cols; i++ ) - if ( GetModel()->HasValue(node->GetItem(), i) ) - numColsWithValue++; - if (numColsWithValue<2) + if ( !HasItemMultipleValues(node->GetItem()) ) return false; } From a1f90ae0de2766b45d9246be8f373786784979b1 Mon Sep 17 00:00:00 2001 From: Jorge Moraleda Date: Mon, 13 Apr 2020 21:35:27 -0700 Subject: [PATCH 04/15] Created method to find the first column that has a value instead of assuming that the expander column is the first one. Renamed method HasItemMultipleValues with IsItemMultivalued and made it const. --- src/generic/datavgen.cpp | 44 +++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index 3dd5e27a08..1d26375352 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -911,11 +911,11 @@ private: bool DoItemChanged(const wxDataViewItem& item, int view_column); // Return whether the item has more than one column that have values - bool HasItemMultipleValues(const wxDataViewItem& item) + bool IsItemMultivalued(const wxDataViewItem& item) const { unsigned int numColsWithValue = 0; unsigned int cols = GetOwner()->GetColumnCount(); - wxDataViewModel* model = GetModel(); + const wxDataViewModel* model = GetModel(); for ( unsigned int i = 0; i < cols; i++ ) { if ( model->HasValue(item, i) ) @@ -926,6 +926,17 @@ private: return false; } + // find first column with value + wxDataViewColumn * FindFirstColumnWithValue(const wxDataViewItem& item) const + { + unsigned int cols = GetOwner()->GetColumnCount(); + const wxDataViewModel* model = GetModel(); + for ( unsigned int i ; i < cols; i++ ) + if ( model->HasValue(item, i) ) + return GetOwner()->GetColumnAt(i); + return NULL; + } + private: wxDataViewCtrl *m_owner; int m_lineHeight; @@ -2416,7 +2427,7 @@ void wxDataViewMainWindow::OnPaint( wxPaintEvent &WXUNUSED(event) ) if ( !IsList() ) { wxDataViewTreeNode *node = GetTreeNodeByRow(item); - if ( !HasItemMultipleValues(node->GetItem()) ) + if ( !IsItemMultivalued(node->GetItem()) ) renderColumnFocus = false; } } @@ -4115,14 +4126,9 @@ wxDataViewMainWindow::FindColumnForEditing(const wxDataViewItem& item, wxDataVie } } - // If on container item without columns, only the expander column - // may be directly editable: - if ( candidate && - GetOwner()->GetExpanderColumn() != candidate && - !GetModel()->HasValue(item, candidate->GetModelColumn()) ) - { - candidate = GetOwner()->GetExpanderColumn(); - } + // Switch to the first column with value if the current column has no value + if ( candidate && !GetModel()->HasValue(item, candidate->GetModelColumn()) ) + candidate = FindFirstColumnWithValue(item); if ( !candidate ) return NULL; @@ -4508,24 +4514,16 @@ bool wxDataViewMainWindow::TryAdvanceCurrentColumn(wxDataViewTreeNode *node, wxK const bool wrapAround = event.GetKeyCode() == WXK_TAB; - if ( node ) - { - // navigation shouldn't work in nodes with fewer than two columns - if ( !HasItemMultipleValues(node->GetItem()) ) - return false; - } + // navigation shouldn't work in nodes with fewer than two columns + if ( node && !IsItemMultivalued(node->GetItem()) ) + return false; if ( m_currentCol == NULL || !m_currentColSetByKeyboard ) { if ( forward ) { // find first column with value - unsigned int i = 0; - unsigned int cols = GetOwner()->GetColumnCount(); - for ( ; i < cols; i++ ) - if ( GetModel()->HasValue(node->GetItem(), i) ) - break; - m_currentCol = GetOwner()->GetColumnAt(i); + m_currentCol = FindFirstColumnWithValue(node->GetItem()); m_currentColSetByKeyboard = true; RefreshRow(m_currentRow); return true; From 8b9cf87653077d359aa7237996f8032148567c5e Mon Sep 17 00:00:00 2001 From: Jorge Moraleda Date: Mon, 13 Apr 2020 23:43:39 -0700 Subject: [PATCH 05/15] Added an example of overriding method HasValue in wxDataView to show how some items may not have values for some columns. --- samples/dataview/dataview.cpp | 55 +++++++++++++++++++++++++++++++++++ samples/dataview/mymodels.cpp | 12 ++++++++ samples/dataview/mymodels.h | 10 +++++++ 3 files changed, 77 insertions(+) diff --git a/samples/dataview/dataview.cpp b/samples/dataview/dataview.cpp index e7a00ab617..f6cab09aa8 100644 --- a/samples/dataview/dataview.cpp +++ b/samples/dataview/dataview.cpp @@ -182,6 +182,7 @@ private: Page_TreeStore, Page_VarHeight, Page_IndexList, + Page_HasValue, Page_Max }; @@ -713,6 +714,16 @@ MyFrame::MyFrame(wxFrame *frame, const wxString &title, int x, int y, int w, int sixthPanelSz->Add(button_sizer6); sixthPanel->SetSizerAndFit(sixthPanelSz); + // page showing that some columns don't have values for some items + // --------------------------------------------------------------- + + wxPanel *seventhPanel = new wxPanel( m_notebook, wxID_ANY ); + + BuildDataViewCtrl(seventhPanel, Page_HasValue); + + wxSizer *seventhPanelSz = new wxBoxSizer( wxVERTICAL ); + seventhPanelSz->Add(m_ctrl[Page_HasValue], 1, wxGROW|wxALL, 5); + seventhPanel->SetSizerAndFit(seventhPanelSz); // complete GUI // ------------ @@ -723,6 +734,7 @@ MyFrame::MyFrame(wxFrame *frame, const wxString &title, int x, int y, int w, int m_notebook->AddPage(fourthPanel, "wxDataViewTreeCtrl"); m_notebook->AddPage(fifthPanel, "Variable line height"); m_notebook->AddPage(sixthPanel, "MyIndexListModel"); + m_notebook->AddPage(seventhPanel, "MyDataViewHasValue"); wxSizer* mainSizer = new wxBoxSizer(wxVERTICAL); @@ -987,7 +999,50 @@ void MyFrame::BuildDataViewCtrl(wxPanel* parent, unsigned int nPanel, unsigned l this); } break; + + case Page_HasValue: + { + wxDataViewListCtrl* lc = + new wxDataViewListCtrl( parent, wxID_ANY, wxDefaultPosition, + wxDefaultSize, style ); + m_ctrl[Page_HasValue] = lc; + + MyListStoreDerivedModel* page7_model = new MyListStoreHasValueModel(); + lc->AssociateModel(page7_model); + page7_model->DecRef(); + + lc->AppendToggleColumn( "Toggle" ); + + // We're not limited to convenience column-appending functions, it + // can also be done fully manually, which allows us to customize + // the renderer being used. + wxDataViewToggleRenderer* const rendererRadio = + new wxDataViewToggleRenderer("bool", wxDATAVIEW_CELL_ACTIVATABLE); + rendererRadio->ShowAsRadio(); + wxDataViewColumn* const colRadio = + new wxDataViewColumn("Radio", rendererRadio, 1); + lc->AppendColumn(colRadio, "bool"); + + lc->AppendTextColumn( "Text" ); + lc->AppendProgressColumn( "Progress" )->SetMinWidth(FromDIP(100)); + + wxVector data; + for (unsigned int i=0; i<10; i++) + { + data.clear(); + data.push_back( (i%3) == 0 ); + data.push_back( i == 7 ); // select a single (random) radio item + data.push_back( wxString::Format("row %d", i) ); + data.push_back( long(5*i) ); + + lc->AppendItem( data ); + } + + lc->Bind(wxEVT_DATAVIEW_ITEM_VALUE_CHANGED, &MyFrame::OnListValueChanged, this); + } + break; } + } diff --git a/samples/dataview/mymodels.cpp b/samples/dataview/mymodels.cpp index 9c5ee3112b..fc60e8bb4f 100644 --- a/samples/dataview/mymodels.cpp +++ b/samples/dataview/mymodels.cpp @@ -626,3 +626,15 @@ bool MyListStoreDerivedModel::IsEnabledByRow(unsigned int row, unsigned int col) // disabled the last two checkboxes return !(col == 0 && 8 <= row && row <= 9); } + +// ---------------------------------------------------------------------------- +// MyListStoreHasValueModel +// ---------------------------------------------------------------------------- + +bool MyListStoreHasValueModel::HasValue(const wxDataViewItem &item, unsigned int col) const +{ + unsigned int row = GetRow( item ); + // the diagonal entries don't have values. This is just a silly example to demonstrate the + // usage of overriding HasValue to specify that some columns don't have values for some items + return row != col; +} diff --git a/samples/dataview/mymodels.h b/samples/dataview/mymodels.h index 8cab532f1f..4fbeb23e2e 100644 --- a/samples/dataview/mymodels.h +++ b/samples/dataview/mymodels.h @@ -266,6 +266,16 @@ public: virtual bool IsEnabledByRow(unsigned int row, unsigned int col) const wxOVERRIDE; }; +// ---------------------------------------------------------------------------- +// MyListStoreHasValueModel +// ---------------------------------------------------------------------------- + +class MyListStoreHasValueModel : public MyListStoreDerivedModel +{ +public: + virtual bool HasValue(const wxDataViewItem &item, unsigned int col) const wxOVERRIDE; +}; + // ---------------------------------------------------------------------------- // MyIndexListModel // ---------------------------------------------------------------------------- From 429d9d8dfcb482cff9c8b8098be08407bb87957d Mon Sep 17 00:00:00 2001 From: Jorge Moraleda Date: Tue, 14 Apr 2020 17:43:51 -0700 Subject: [PATCH 06/15] Use null as value when sorting items in a DataView control by a column for which the item does not have a value. --- src/common/datavcmn.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/datavcmn.cpp b/src/common/datavcmn.cpp index 76073485e1..bfdd3b7ca6 100644 --- a/src/common/datavcmn.cpp +++ b/src/common/datavcmn.cpp @@ -334,8 +334,8 @@ int wxDataViewModel::Compare( const wxDataViewItem &item1, const wxDataViewItem unsigned int column, bool ascending ) const { wxVariant value1,value2; - GetValue( value1, item1, column ); - GetValue( value2, item2, column ); + HasValue(item1, column) ? GetValue( value1, item1, column ) : value1.MakeNull(); + HasValue(item2, column) ? GetValue( value2, item2, column ) : value2.MakeNull(); if (!ascending) { From 1fa1aa593727fbf68b2c4dc2104adea1f57c22c7 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 17 Apr 2020 23:24:54 +0200 Subject: [PATCH 07/15] Mention that wxDataViewModel::HasValue() became virtual in 3.1.4 This method existed since earlier versions, but couldn't be overridden until now. --- interface/wx/dataview.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/interface/wx/dataview.h b/interface/wx/dataview.h index f2857904e0..d106567b76 100644 --- a/interface/wx/dataview.h +++ b/interface/wx/dataview.h @@ -309,8 +309,9 @@ public: item is not a container or HasContainerColumns() was overridden to return true for it. - Override this method to explicitly specify for which columns a given - item has values. + Since wxWidgets 3.1.4, this method is virtual and can be overridden to + explicitly specify for which columns a given item has, and doesn't + have, values. @since 2.9.1 */ From 84fb5f38be0dcdf7f098255860a56311d32fe45e Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 17 Apr 2020 23:25:30 +0200 Subject: [PATCH 08/15] Don't call wxVariant::MakeNull() unnecessary Simplify recently added code in wxDataViewModel::Compare(). No real changes. --- src/common/datavcmn.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/common/datavcmn.cpp b/src/common/datavcmn.cpp index bfdd3b7ca6..cc0b60c1c6 100644 --- a/src/common/datavcmn.cpp +++ b/src/common/datavcmn.cpp @@ -334,8 +334,13 @@ int wxDataViewModel::Compare( const wxDataViewItem &item1, const wxDataViewItem unsigned int column, bool ascending ) const { wxVariant value1,value2; - HasValue(item1, column) ? GetValue( value1, item1, column ) : value1.MakeNull(); - HasValue(item2, column) ? GetValue( value2, item2, column ) : value2.MakeNull(); + + // Avoid calling GetValue() for the cells that are not supposed to have any + // value, this might be unexpected. + if ( HasValue(item1, column) ) + GetValue( value1, item1, column ); + if ( HasValue(item2, column) ) + GetValue( value2, item2, column ); if (!ascending) { From c80626d33a81950ccfb8e2ee997eb69ca4e4b0cb Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 17 Apr 2020 23:27:21 +0200 Subject: [PATCH 09/15] Replace IsItemMultivalued() with IsItemSingleValued() This allows to simplify logic when calling it by avoiding negation. No real changes. --- src/generic/datavgen.cpp | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index 1d26375352..3dfa2a9525 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -910,20 +910,23 @@ private: // assumes that all columns were modified, otherwise just this one. bool DoItemChanged(const wxDataViewItem& item, int view_column); - // Return whether the item has more than one column that have values - bool IsItemMultivalued(const wxDataViewItem& item) const + // Return whether the item has at most one column with a value. + bool IsItemSingleValued(const wxDataViewItem& item) const { - unsigned int numColsWithValue = 0; + bool hadColumnWithValue = false; unsigned int cols = GetOwner()->GetColumnCount(); const wxDataViewModel* model = GetModel(); for ( unsigned int i = 0; i < cols; i++ ) { if ( model->HasValue(item, i) ) - numColsWithValue++; - if ( numColsWithValue > 1 ) - return true; + { + if ( hadColumnWithValue ) + return false; + hadColumnWithValue = true; + } } - return false; + + return true; } // find first column with value @@ -2423,11 +2426,11 @@ void wxDataViewMainWindow::OnPaint( wxPaintEvent &WXUNUSED(event) ) { renderColumnFocus = true; - // If this is container node without columns, render full-row focus: + // If there is just a single value, render full-row focus: if ( !IsList() ) { wxDataViewTreeNode *node = GetTreeNodeByRow(item); - if ( !IsItemMultivalued(node->GetItem()) ) + if ( IsItemSingleValued(node->GetItem()) ) renderColumnFocus = false; } } @@ -4515,7 +4518,7 @@ bool wxDataViewMainWindow::TryAdvanceCurrentColumn(wxDataViewTreeNode *node, wxK const bool wrapAround = event.GetKeyCode() == WXK_TAB; // navigation shouldn't work in nodes with fewer than two columns - if ( node && !IsItemMultivalued(node->GetItem()) ) + if ( node && IsItemSingleValued(node->GetItem()) ) return false; if ( m_currentCol == NULL || !m_currentColSetByKeyboard ) From 736628f7a0c80fcfc075224e5d9ef2dcceeac4cb Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 17 Apr 2020 23:28:03 +0200 Subject: [PATCH 10/15] Fix crash due to dereferencing null pointer in list mode TryAdvanceCurrentColumn() is called with NULL node in this case, so avoid dereferencing it. --- src/generic/datavgen.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index 3dfa2a9525..97038b4525 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -4525,8 +4525,18 @@ bool wxDataViewMainWindow::TryAdvanceCurrentColumn(wxDataViewTreeNode *node, wxK { if ( forward ) { - // find first column with value - m_currentCol = FindFirstColumnWithValue(node->GetItem()); + if ( node ) + { + // find first column with value + m_currentCol = FindFirstColumnWithValue(node->GetItem()); + } + else + { + // in the special "list" case, all columns have values, so just + // take the first one + m_currentCol = GetOwner()->GetColumnAt(0); + } + m_currentColSetByKeyboard = true; RefreshRow(m_currentRow); return true; @@ -4579,7 +4589,7 @@ bool wxDataViewMainWindow::TryAdvanceCurrentColumn(wxDataViewTreeNode *node, wxK return false; } } - if ( GetModel()->HasValue(node->GetItem(), i) ) + if ( !node || GetModel()->HasValue(node->GetItem(), i) ) break; } From b90a74f6fe80f6137e8601ba9f49ae12f9be8e4e Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 17 Apr 2020 23:28:32 +0200 Subject: [PATCH 11/15] Minor formatting and code style fixes Put braces around loop body. Use const for local variables. No real changes. --- src/generic/datavgen.cpp | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index 97038b4525..62c1cbe189 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -914,8 +914,8 @@ private: bool IsItemSingleValued(const wxDataViewItem& item) const { bool hadColumnWithValue = false; - unsigned int cols = GetOwner()->GetColumnCount(); - const wxDataViewModel* model = GetModel(); + const unsigned int cols = GetOwner()->GetColumnCount(); + const wxDataViewModel* const model = GetModel(); for ( unsigned int i = 0; i < cols; i++ ) { if ( model->HasValue(item, i) ) @@ -929,14 +929,17 @@ private: return true; } - // find first column with value - wxDataViewColumn * FindFirstColumnWithValue(const wxDataViewItem& item) const + // Find the first column with a value in it. + wxDataViewColumn* FindFirstColumnWithValue(const wxDataViewItem& item) const { - unsigned int cols = GetOwner()->GetColumnCount(); - const wxDataViewModel* model = GetModel(); - for ( unsigned int i ; i < cols; i++ ) + const unsigned int cols = GetOwner()->GetColumnCount(); + const wxDataViewModel* const model = GetModel(); + for ( unsigned int i = 0; i < cols; i++ ) + { if ( model->HasValue(item, i) ) return GetOwner()->GetColumnAt(i); + } + return NULL; } @@ -4549,7 +4552,7 @@ bool wxDataViewMainWindow::TryAdvanceCurrentColumn(wxDataViewTreeNode *node, wxK } int idx = GetOwner()->GetColumnIndex(m_currentCol); - unsigned int cols = GetOwner()->GetColumnCount(); + const unsigned int cols = GetOwner()->GetColumnCount(); for ( unsigned int i = 0; i < cols; i++ ) { idx += (forward ? +1 : -1); From ede053def6377542530e298d157998beb41d5ef4 Mon Sep 17 00:00:00 2001 From: Jorge Moraleda Date: Sat, 18 Apr 2020 13:39:14 -0700 Subject: [PATCH 12/15] A DVC cell that has no value is not editable --- src/generic/datavgen.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index 62c1cbe189..cebba8f161 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -4155,6 +4155,9 @@ bool wxDataViewMainWindow::IsCellEditableInMode(const wxDataViewItem& item, if ( !GetModel()->IsEnabled(item, col->GetModelColumn()) ) return false; + if ( !GetModel()->HasValue(item, col->GetModelColumn()) ) + return false; + return true; } From e25d86916860b1aaff9681e36b8ab9420bd020b9 Mon Sep 17 00:00:00 2001 From: Jorge Moraleda Date: Mon, 20 Apr 2020 17:11:24 -0700 Subject: [PATCH 13/15] Added dedicated command binding of HasPage tab of DVC sample so that the radio button column has always exactly one cell checked. Before this fix this tab was incorrectly bound to the same function that controlled the ListCtrl in the third tab. --- samples/dataview/dataview.cpp | 47 ++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/samples/dataview/dataview.cpp b/samples/dataview/dataview.cpp index f6cab09aa8..3b840491ce 100644 --- a/samples/dataview/dataview.cpp +++ b/samples/dataview/dataview.cpp @@ -170,6 +170,9 @@ private: enum Lang { Lang_English, Lang_French }; void FillIndexList(Lang lang); + // HasValue page. + void OnHasValueValueChanged(wxDataViewEvent& event); + wxNotebook* m_notebook; @@ -1038,7 +1041,7 @@ void MyFrame::BuildDataViewCtrl(wxPanel* parent, unsigned int nPanel, unsigned l lc->AppendItem( data ); } - lc->Bind(wxEVT_DATAVIEW_ITEM_VALUE_CHANGED, &MyFrame::OnListValueChanged, this); + lc->Bind(wxEVT_DATAVIEW_ITEM_VALUE_CHANGED, &MyFrame::OnHasValueValueChanged, this); } break; } @@ -1772,3 +1775,45 @@ void MyFrame::OnIndexListSelectionChanged(wxDataViewEvent& event) wxLogMessage("Selected week day: %s", weekday); } + +// ---------------------------------------------------------------------------- +// MyFrame - event handlers for the HasValue (wxDataViewListCtrl) page +// ---------------------------------------------------------------------------- + +void MyFrame::OnHasValueValueChanged(wxDataViewEvent& event) +{ + // Ignore changes coming from our own SetToggleValue() calls below. + if ( m_eventFromProgram ) + { + m_eventFromProgram = false; + return; + } + + wxDataViewListCtrl* const lc = static_cast(m_ctrl[Page_HasValue]); + + const int columnToggle = 1; + + // Handle selecting a radio button by unselecting all the other ones. + if ( event.GetColumn() == columnToggle ) + { + const int rowChanged = lc->ItemToRow(event.GetItem()); + if ( lc->GetToggleValue(rowChanged, columnToggle) ) + { + for ( int row = 0; row < lc->GetItemCount(); ++row ) + { + if ( row != rowChanged ) + { + m_eventFromProgram = true; + lc->SetToggleValue(false, row, columnToggle); + } + } + } + else // The item was cleared. + { + // Explicitly check it back, we want to always have exactly one + // checked radio item in this column. + m_eventFromProgram = true; + lc->SetToggleValue(true, rowChanged, columnToggle); + } + } +} From 773a1876de570e08897cd641b139e5674e5d7f7c Mon Sep 17 00:00:00 2001 From: Jorge Moraleda Date: Mon, 20 Apr 2020 19:43:34 -0700 Subject: [PATCH 14/15] In DVC, if current column was set by keyboard to something not editable and the user pressed Space/F2 then do not edit anything because focus is visually on that column and editing something else would be surprising. But if the current column was set by mouse to something not editable, treat the situation as if there was whole-row focus, because the mouse click could very well be targeted on the row rather than on an individual cell. --- src/generic/datavgen.cpp | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index cebba8f161..da847e8542 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -4099,20 +4099,25 @@ wxDataViewMainWindow::FindColumnForEditing(const wxDataViewItem& item, wxDataVie wxDataViewColumn *candidate = m_currentCol; - if ( candidate && - !IsCellEditableInMode(item, candidate, mode) && - !m_currentColSetByKeyboard ) + if ( candidate && !IsCellEditableInMode(item, candidate, mode) ) { - // If current column was set by mouse to something not editable (in - // 'mode') and the user pressed Space/F2 to edit it, treat the - // situation as if there was whole-row focus, because that's what is - // visually indicated and the mouse click could very well be targeted - // on the row rather than on an individual cell. - // - // But if it was done by keyboard, respect that even if the column - // isn't editable, because focus is visually on that column and editing - // something else would be surprising. - candidate = NULL; + if ( m_currentColSetByKeyboard ) + { + // If current column was set by keyboard to something not editable (in + // 'mode') and the user pressed Space/F2 then do not edit anything + // because focus is visually on that column and editing + // something else would be surprising. + return NULL; + } + else + { + // But if the current column was set by mouse to something not editable (in + // 'mode') and the user pressed Space/F2 to edit it, treat the + // situation as if there was whole-row focus, because that's what is + // visually indicated and the mouse click could very well be targeted + // on the row rather than on an individual cell. + candidate = NULL; + } } if ( !candidate ) @@ -4134,7 +4139,8 @@ wxDataViewMainWindow::FindColumnForEditing(const wxDataViewItem& item, wxDataVie // Switch to the first column with value if the current column has no value if ( candidate && !GetModel()->HasValue(item, candidate->GetModelColumn()) ) - candidate = FindFirstColumnWithValue(item); + candidate = FindFirstColumnWithValue(item); + if ( !candidate ) return NULL; From e3b216b75eaf146a2abc014fa327438d4686f10c Mon Sep 17 00:00:00 2001 From: Jorge Moraleda Date: Mon, 20 Apr 2020 19:50:26 -0700 Subject: [PATCH 15/15] Consistent indentation in generic DVC --- src/generic/datavgen.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index da847e8542..8f1368d17f 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -4139,16 +4139,15 @@ wxDataViewMainWindow::FindColumnForEditing(const wxDataViewItem& item, wxDataVie // Switch to the first column with value if the current column has no value if ( candidate && !GetModel()->HasValue(item, candidate->GetModelColumn()) ) - candidate = FindFirstColumnWithValue(item); - + candidate = FindFirstColumnWithValue(item); if ( !candidate ) - return NULL; + return NULL; - if ( !IsCellEditableInMode(item, candidate, mode) ) - return NULL; + if ( !IsCellEditableInMode(item, candidate, mode) ) + return NULL; - return candidate; + return candidate; } bool wxDataViewMainWindow::IsCellEditableInMode(const wxDataViewItem& item,