Return empty rectangle from GetItemRect() for collapsed items

GetItemByRow() returned a valid row even for an item which was collapsed
which is clearly inappropriate for its use in GetItemRect(), which is
supposed return an invalid rectangle if the item is not visible. It also
might be inappropriate in other cases, but this is not totally clear and
it seems like it is supposed to return a valid row even for collapsed
items at least sometimes, so just make its behaviour conditional by
adding a new flags parameter to GetItemRect() and to Walker() helper
used by it itself, so that it could skip over collapsed items.

Update the test to show that it succeeds now even when the item is
present in the tree, as it only passed before because the item had never
been expanded at all, and so wasn't really present in the tree
structure and the updated test would have failed without the changes to
the code in this commit.

src/generic/datavgen.cpp

@@ -74,6 +74,13 @@ static const int PADDING_RIGHTLEFT = 3;
 namespace
 {
 
+// Flags for Walker() function defined below.
+enum WalkFlags
+{
+    Walk_All,               // Visit all items.
+    Walk_ExpandedOnly       // Visit only expanded items.
+};
+
 // The column is either the index of the column to be used for sorting or one
 // of the special values in this enum:
 enum
@@ -854,7 +861,8 @@ public:
 
     // Some useful functions for row and item mapping
     wxDataViewItem GetItemByRow( unsigned int row ) const;
-    int GetRowByItem( const wxDataViewItem & item ) const;
+    int GetRowByItem( const wxDataViewItem & item,
+                      WalkFlags flags = Walk_All ) const;
 
     wxDataViewTreeNode * GetTreeNodeByRow( unsigned int row ) const;
     // We did not need this temporarily
@@ -3014,7 +3022,8 @@ public:
     virtual int operator() ( wxDataViewTreeNode * node ) = 0;
 };
 
-bool Walker( wxDataViewTreeNode * node, DoJob & func )
+bool
+Walker(wxDataViewTreeNode * node, DoJob & func, WalkFlags flags = Walk_All)
 {
     wxCHECK_MSG( node, false, "can't walk NULL node" );
 
@@ -3028,7 +3037,7 @@ bool Walker( wxDataViewTreeNode * node, DoJob & func )
             break;
     }
 
-    if ( node->HasChildren() )
+    if ( node->HasChildren() && (flags != Walk_ExpandedOnly || node->IsOpen()) )
     {
         const wxDataViewTreeNodes& nodes = node->GetChildNodes();
 
@@ -3036,7 +3045,7 @@ bool Walker( wxDataViewTreeNode * node, DoJob & func )
               i != nodes.end();
               ++i )
         {
-            if ( Walker(*i, func) )
+            if ( Walker(*i, func, flags) )
                 return true;
         }
     }
@@ -4143,7 +4152,7 @@ wxRect wxDataViewMainWindow::GetItemRect( const wxDataViewItem & item,
         xpos = 0;
     }
 
-    const int row = GetRowByItem(item);
+    const int row = GetRowByItem(item, Walk_ExpandedOnly);
     if ( row == -1 )
     {
         // This means the row is currently not visible at all.
@@ -4244,7 +4253,9 @@ private:
 
 };
 
-int wxDataViewMainWindow::GetRowByItem(const wxDataViewItem & item) const
+int
+wxDataViewMainWindow::GetRowByItem(const wxDataViewItem & item,
+                                   WalkFlags flags) const
 {
     const wxDataViewModel * model = GetModel();
     if( model == NULL )
@@ -4274,7 +4285,7 @@ int wxDataViewMainWindow::GetRowByItem(const wxDataViewItem & item) const
         // the parent chain was created by adding the deepest parent first.
         // so if we want to start at the root node, we have to iterate backwards through the vector
         ItemToRowJob job( item, parentChain.rbegin() );
-        if ( !Walker( m_root, job ) )
+        if ( !Walker( m_root, job, flags ) )
             return -1;
 
         return job.GetResult();

tests/controls/dataviewctrltest.cpp

@@ -297,6 +297,12 @@ TEST_CASE_METHOD(SingleSelectDataViewCtrlTestCase,
         CHECK( rect1.y < rect2.y );
     }
 
+    // This forces generic implementation to add m_grandchild to the tree, as
+    // it does it only on demand. We want the item to really be there to check
+    // that GetItemRect() returns an empty rectangle for collapsed items.
+    m_dvc->Expand(m_child1);
+    m_dvc->Collapse(m_child1);
+
     const wxRect rectNotShown = m_dvc->GetItemRect(m_grandchild);
     CHECK( rectNotShown == wxRect() );