From fedc80eee3c449cec894f3bdfacd8ef01e41f71b Mon Sep 17 00:00:00 2001 From: ali kettab Date: Sun, 6 Sep 2020 00:53:45 +0100 Subject: [PATCH] Improve selection and focus events generation in wxGenericLisCtrl Avoid sending spurious wxEVT_LIST_ITEM_{FOCUSED, SELECTED, DESELECTED} events and make the generic version consistent with the behaviour of the native wxMSW one. Also add/extend the tests and slightly improve the sample. Closes https://github.com/wxWidgets/wxWidgets/pull/2044 --- include/wx/generic/private/listctrl.h | 51 ++++- samples/listctrl/listtest.cpp | 59 ++++-- src/generic/listctrl.cpp | 286 +++++++++++++++++++------- tests/controls/listbasetest.cpp | 125 ++++++++++- tests/controls/listbasetest.h | 2 + tests/controls/listviewtest.cpp | 8 +- 6 files changed, 432 insertions(+), 99 deletions(-) diff --git a/include/wx/generic/private/listctrl.h b/include/wx/generic/private/listctrl.h index c5741b6b13..4087889098 100644 --- a/include/wx/generic/private/listctrl.h +++ b/include/wx/generic/private/listctrl.h @@ -519,14 +519,26 @@ public: // all these functions only do something if the line is currently visible + // Make sure that _line_ is the only item highlighted in the control. + // _oldLine_ is the old focused item. + void HighlightOnly( size_t line, size_t oldLine = (size_t)-1 ); + + // In multiple selection mode, instead of sending one notification per item + // (which is too slow if a lot of items are selected) we send only one notification + // for all of them which is the wxMSW behaviour. Currently done for virtual + // list controls and for deselection only. + enum SendEvent { SendEvent_None, SendEvent_Normal }; + // change the line "selected" state, return true if it really changed - bool HighlightLine( size_t line, bool highlight = true); + bool HighlightLine( size_t line, bool highlight = true, + SendEvent sendEvent = SendEvent_Normal ); // as HighlightLine() but do it for the range of lines: this is incredibly // more efficient for virtual list controls! // // NB: unlike HighlightLine() this one does refresh the lines on screen - void HighlightLines( size_t lineFrom, size_t lineTo, bool on = true ); + void HighlightLines( size_t lineFrom, size_t lineTo, bool on = true, + SendEvent sendEvent = SendEvent_Normal ); // toggle the line state and refresh it void ReverseHighlight( size_t line ) @@ -753,6 +765,16 @@ public: return m_hasFocus; } + void UpdateSelectionCount(bool selected) + { + wxASSERT_MSG( !IsVirtual(), "Can be called for non virtual lists only" ); + + if ( IsSingleSel() ) + return; + + selected ? ++m_selCount : --m_selCount; + } + protected: // the array of all line objects for a non virtual list control (for the // virtual list control we only ever use m_lines[0]) @@ -803,11 +825,18 @@ protected: m_lineBeforeLastClicked, m_lineSelectSingleOnUp; + // Multiple selection extends from the anchor. Not used in single-selection mode. + size_t m_anchor; + bool m_hasCheckBoxes; protected: wxWindow *GetMainWindowOfCompositeControl() wxOVERRIDE { return GetParent(); } + // the total count of items selected in a non virtual list control with + // multiple selections (always 0 otherwise) + size_t m_selCount; + // the total count of items in a virtual list control size_t m_countVirt; @@ -858,6 +887,24 @@ private: // initialize the current item if needed void UpdateCurrent(); + // change the current (== focused) item, without sending any event + // return true if m_current really changed. + bool ChangeCurrentWithoutEvent(size_t current); + + // Trying to activate the current item from keyboard is only possible + // if it is actually selected. We don't send wxEVT_LIST_ITEM_ACTIVATED + // event if it is not, and wxEVT_LIST_KEY_DOWN event should carry -1 + // in this case, as the wxMSW implementation does. + bool ShouldSendEventForCurrent() const + { + return HasCurrent() && IsHighlighted(m_current); + } + + // For multiple selection mode. + // Change the selected range from [anchor, oldCurrent] to [anchor, newCurrent] + // without generating unnecessary wxEVT_LIST_ITEM_{DE}SELECTED events. + void ExtendSelection(size_t oldCurrent, size_t newCurrent); + // delete all items but don't refresh: called from dtor void DoDeleteAllItems(); diff --git a/samples/listctrl/listtest.cpp b/samples/listctrl/listtest.cpp index 2218622b4e..4ef123fa73 100644 --- a/samples/listctrl/listtest.cpp +++ b/samples/listctrl/listtest.cpp @@ -564,6 +564,15 @@ void MyFrame::InitWithReportItems() itemCol.SetAlign(wxLIST_FORMAT_RIGHT); m_listCtrl->InsertColumn(2, itemCol); + if ( m_numListItems <= 0 ) + { + m_listCtrl->SetColumnWidth( 0, 100 ); + m_listCtrl->SetColumnWidth( 1, wxLIST_AUTOSIZE ); + m_listCtrl->SetColumnWidth( 2, wxLIST_AUTOSIZE_USEHEADER ); + + return; + } + // to speed up inserting we hide the control temporarily m_listCtrl->Hide(); @@ -584,25 +593,38 @@ void MyFrame::InitWithReportItems() item.SetTextColour(*wxRED); m_listCtrl->SetItem( item ); - item.m_itemId = 2; - item.SetTextColour(*wxGREEN); - m_listCtrl->SetItem( item ); - item.m_itemId = 4; - item.SetTextColour(*wxLIGHT_GREY); - item.SetFont(*wxITALIC_FONT); - item.SetBackgroundColour(*wxRED); - m_listCtrl->SetItem( item ); + if ( m_numListItems > 2 ) + { + item.m_itemId = 2; + item.SetTextColour(*wxGREEN); + m_listCtrl->SetItem( item ); + } + + if ( m_numListItems > 4 ) + { + item.m_itemId = 4; + item.SetTextColour(*wxLIGHT_GREY); + item.SetFont(*wxITALIC_FONT); + item.SetBackgroundColour(*wxRED); + m_listCtrl->SetItem( item ); + } m_listCtrl->SetTextColour(*wxBLUE); - // Set images in columns - m_listCtrl->SetItemColumnImage(1, 1, 0); + if ( m_numListItems > 1 ) + { + // Set images in columns + m_listCtrl->SetItemColumnImage(1, 1, 0); + } - wxListItem info; - info.SetImage(0); - info.SetId(3); - info.SetColumn(2); - m_listCtrl->SetItem(info); + if ( m_numListItems > 3 ) + { + wxListItem info; + info.SetImage(0); + info.SetId(3); + info.SetColumn(2); + m_listCtrl->SetItem(info); + } // test SetItemFont too m_listCtrl->SetItemFont(0, *wxITALIC_FONT); @@ -1073,6 +1095,11 @@ void MyListCtrl::OnColClick(wxListEvent& event) { int col = event.GetColumn(); + if ( col == -1 ) + { + return; // clicked outside any column. + } + // set or unset image static bool x = false; x = !x; @@ -1409,8 +1436,6 @@ void MyListCtrl::OnListKeyDown(wxListEvent& event) wxFALLTHROUGH; default: - LogEvent(event, "OnListKeyDown"); - event.Skip(); } } diff --git a/src/generic/listctrl.cpp b/src/generic/listctrl.cpp index 45e698960d..c47314ee24 100644 --- a/src/generic/listctrl.cpp +++ b/src/generic/listctrl.cpp @@ -950,6 +950,7 @@ bool wxListLineData::Highlight( bool on ) return false; m_highlighted = on; + m_owner->UpdateSelectionCount(on); return true; } @@ -1571,6 +1572,7 @@ wxEND_EVENT_TABLE() void wxListMainWindow::Init() { m_dirty = true; + m_selCount = m_countVirt = 0; m_lineFrom = m_lineTo = (size_t)-1; @@ -1598,7 +1600,8 @@ void wxListMainWindow::Init() m_current = m_lineLastClicked = m_lineSelectSingleOnUp = - m_lineBeforeLastClicked = (size_t)-1; + m_lineBeforeLastClicked = + m_anchor = (size_t)-1; m_hasCheckBoxes = false; } @@ -1866,8 +1869,13 @@ bool wxListMainWindow::IsHighlighted(size_t line) const void wxListMainWindow::HighlightLines( size_t lineFrom, size_t lineTo, - bool highlight ) + bool highlight, + SendEvent sendEvent ) { + // It is safe to swap the bounds here if they are not in order. + if ( lineFrom > lineTo ) + wxSwap(lineFrom, lineTo); + if ( IsVirtual() ) { wxArrayInt linesChanged; @@ -1890,13 +1898,13 @@ void wxListMainWindow::HighlightLines( size_t lineFrom, { for ( size_t line = lineFrom; line <= lineTo; line++ ) { - if ( HighlightLine(line, highlight) ) + if ( HighlightLine(line, highlight, sendEvent) ) RefreshLine(line); } } } -bool wxListMainWindow::HighlightLine( size_t line, bool highlight ) +bool wxListMainWindow::HighlightLine( size_t line, bool highlight, SendEvent sendEvent ) { bool changed; @@ -1912,7 +1920,7 @@ bool wxListMainWindow::HighlightLine( size_t line, bool highlight ) changed = ld->Highlight(highlight); } - if ( changed ) + if ( changed && sendEvent ) { SendNotify( line, highlight ? wxEVT_LIST_ITEM_SELECTED : wxEVT_LIST_ITEM_DESELECTED ); @@ -2197,6 +2205,58 @@ void wxListMainWindow::HighlightAll( bool on ) } } +void wxListMainWindow::HighlightOnly( size_t line, size_t oldLine ) +{ + const unsigned selCount = GetSelectedItemCount(); + + if ( selCount == 1 && IsHighlighted(line) ) + { + return; // Nothing changed. + } + + if ( oldLine != (size_t)-1 ) + { + IsHighlighted(oldLine) ? ReverseHighlight(oldLine) + : RefreshLine(oldLine); // refresh the old focus to remove it + } + + if ( selCount > 1 ) // multiple-selection only + { + // Deselecting many items at once will generate wxEVT_XXX_DESELECTED event + // for each one of them. although this may be inefficient if the number of + // deselected items is too much, we keep doing this (for non-virtual list + // controls) for backward compatibility concerns. For virtual listctrl (in + // multi-selection mode), wxMSW sends only a notification to indicate that + // something has been deselected. Notice that to be fully compatible with + // wxMSW behaviour, _line_ shouldn't be deselected if it was selected. + + const SendEvent sendEvent = IsVirtual() ? SendEvent_None : SendEvent_Normal; + + size_t lineFrom = 0, + lineTo = GetItemCount() - 1; + + if ( line > lineFrom && line < lineTo ) + { + HighlightLines(lineFrom, line - 1, false, sendEvent); + HighlightLines(line + 1, lineTo, false, sendEvent); + } + else // _line_ is equal to lineFrom or lineTo + { + line == lineFrom ? ++lineFrom : --lineTo; + HighlightLines(lineFrom, lineTo, false, sendEvent); + } + + // If we didn't send the event for individual items above, send it for all of them now. + if ( sendEvent == SendEvent_None ) + SendNotify((size_t)-1, wxEVT_LIST_ITEM_DESELECTED); + } + + // _line_ should be the only selected item. + HighlightLine(line); + // refresh the new focus to add it. + RefreshLine(line); +} + void wxListMainWindow::OnChildFocus(wxChildFocusEvent& WXUNUSED(event)) { // Do nothing here. This prevents the default handler in wxScrolledWindow @@ -2241,8 +2301,13 @@ void wxListMainWindow::SendNotify( size_t line, GetParent()->GetEventHandler()->ProcessEvent( le ); } -void wxListMainWindow::ChangeCurrent(size_t current) +bool wxListMainWindow::ChangeCurrentWithoutEvent(size_t current) { + if ( current == m_current ) + { + return false; // Nothing changed! + } + m_current = current; // as the current item changed, we shouldn't start editing it when the @@ -2250,7 +2315,13 @@ void wxListMainWindow::ChangeCurrent(size_t current) if ( m_renameTimer->IsRunning() ) m_renameTimer->Stop(); - SendNotify(current, wxEVT_LIST_ITEM_FOCUSED); + return true; +} + +void wxListMainWindow::ChangeCurrent(size_t current) +{ + if ( ChangeCurrentWithoutEvent(current) ) + SendNotify(current, wxEVT_LIST_ITEM_FOCUSED); } wxTextCtrl *wxListMainWindow::EditLabel(long item, wxClassInfo* textControlClass) @@ -2396,7 +2467,11 @@ void wxListMainWindow::OnMouse( wxMouseEvent &event ) evtCtx.SetEventObject(GetParent()); GetParent()->GetEventHandler()->ProcessEvent(evtCtx); } - return; + + if ( IsEmpty() ) + return; + + // Continue processing... } if (m_dirty) @@ -2521,8 +2596,7 @@ void wxListMainWindow::OnMouse( wxMouseEvent &event ) if (m_lineSelectSingleOnUp != (size_t)-1) { // select single line - HighlightAll( false ); - ReverseHighlight(m_lineSelectSingleOnUp); + HighlightOnly(m_lineSelectSingleOnUp); } if (m_lastOnSame) @@ -2542,6 +2616,14 @@ void wxListMainWindow::OnMouse( wxMouseEvent &event ) m_lastOnSame = false; } + if ( GetSelectedItemCount() == 1 || event.CmdDown() ) + { + // In multiple selection mode, the anchor is set to the first selected + // item or can be changed to m_current if Ctrl key is down, as is the + // case under wxMSW. + m_anchor = m_current; + } + m_lineSelectSingleOnUp = (size_t)-1; } else @@ -2561,9 +2643,8 @@ void wxListMainWindow::OnMouse( wxMouseEvent &event ) // Multi-selections should not be cleared if a selected item is clicked. if (!IsHighlighted(current)) { - HighlightAll(false); ChangeCurrent(current); - ReverseHighlight(m_current); + HighlightOnly(m_current); } SendNotify( current, wxEVT_LIST_ITEM_RIGHT_CLICK, event.GetPosition() ); @@ -2581,7 +2662,7 @@ void wxListMainWindow::OnMouse( wxMouseEvent &event ) m_lineLastClicked = current; size_t oldCurrent = m_current; - bool oldWasSelected = IsHighlighted(m_current); + bool oldWasSelected = HasCurrent() && IsHighlighted(m_current); bool cmdModifierDown = event.CmdDown(); if ( IsSingleSel() || !(cmdModifierDown || event.ShiftDown()) ) @@ -2592,11 +2673,8 @@ void wxListMainWindow::OnMouse( wxMouseEvent &event ) } else if (IsSingleSel() || !IsHighlighted(current)) { - HighlightAll(false); - ChangeCurrent(current); - - ReverseHighlight(m_current); + HighlightOnly(m_current, oldWasSelected ? oldCurrent : (size_t)-1); } else // multi sel & current is highlighted & no mod keys { @@ -2616,16 +2694,15 @@ void wxListMainWindow::OnMouse( wxMouseEvent &event ) { ChangeCurrent(current); - size_t lineFrom = oldCurrent, - lineTo = current; - - if ( lineTo < lineFrom ) + if ( oldCurrent == (size_t)-1 ) { - lineTo = lineFrom; - lineFrom = m_current; + // Highlight m_current only if there is no previous selection. + HighlightLine(m_current); + } + else if ( oldCurrent != current && m_anchor != (size_t)-1 ) + { + ExtendSelection(oldCurrent, current); } - - HighlightLines(lineFrom, lineTo); } else // !ctrl, !shift { @@ -2634,7 +2711,7 @@ void wxListMainWindow::OnMouse( wxMouseEvent &event ) } } - if (m_current != oldCurrent) + if (m_current != oldCurrent && oldCurrent != (size_t)-1) RefreshLine( oldCurrent ); // Set the flag telling us whether the next click on this item should @@ -2739,6 +2816,81 @@ bool wxListMainWindow::ScrollList(int WXUNUSED(dx), int dy) // keyboard handling // ---------------------------------------------------------------------------- +// Helper function which handles items selection correctly and efficiently. and +// simply, mimic the wxMSW behaviour. And The benefit of this function is that it +// ensures that wxEVT_LIST_ITEM_{DE}SELECTED events are only generated for items +// freshly (de)selected (i.e. items that have really changed state). Useful for +// multi-selection mode when selecting using mouse or arrows with Shift key down. +void wxListMainWindow::ExtendSelection(size_t oldCurrent, size_t newCurrent) +{ + // Refresh the old/new focus to remove/add it + RefreshLine(oldCurrent); + RefreshLine(newCurrent); + + // Given a selection [anchor, old], to change/extend it to new (i.e. the + // selection becomes [anchor, new]) we discriminate three possible cases: + // + // Case 1) new < old <= anchor || anchor <= old < new + // i.e. oldCurrent between anchor and newCurrent, in which case we: + // - Highlight everything between anchor and newCurrent (inclusive). + // + // Case 2) old < new <= anchor || anchor <= new < old + // i.e. newCurrent between anchor and oldCurrent, in which case we: + // - Unhighlight everything between oldCurrent and newCurrent (exclusive). + // + // Case 3) old < anchor < new || new < anchor < old + // i.e. anchor between oldCurrent and newCurrent, in which case we + // - Highlight everything between anchor and newCurrent (inclusive). + // - Unhighlight everything between anchor (exclusive) and oldCurrent. + + size_t lineFrom, lineTo; + + if ( (newCurrent < oldCurrent && oldCurrent <= m_anchor) || + (newCurrent > oldCurrent && oldCurrent >= m_anchor) ) + { + lineFrom = m_anchor; + lineTo = newCurrent; + + HighlightLines(lineFrom, lineTo); + } + else if ( (oldCurrent < newCurrent && newCurrent <= m_anchor) || + (oldCurrent > newCurrent && newCurrent >= m_anchor) ) + { + lineFrom = oldCurrent; + lineTo = newCurrent; + + // Exclude newCurrent from being deselected + (lineTo < lineFrom) ? ++lineTo : --lineTo; + + HighlightLines(lineFrom, lineTo, false); + + // For virtual listctrl (in multi-selection mode), wxMSW sends only + // a notification to indicate that something has been deselected. + if ( IsVirtual() ) + SendNotify((size_t)-1, wxEVT_LIST_ITEM_DESELECTED); + } + else if ( (oldCurrent < m_anchor && m_anchor < newCurrent) || + (newCurrent < m_anchor && m_anchor < oldCurrent) ) + { + lineFrom = m_anchor; + lineTo = oldCurrent; + + // Exclude anchor from being deselected + (lineTo < lineFrom) ? --lineFrom : ++lineFrom; + + HighlightLines(lineFrom, lineTo, false); + + // See above. + if ( IsVirtual() ) + SendNotify((size_t)-1, wxEVT_LIST_ITEM_DESELECTED); + + lineFrom = m_anchor; + lineTo = newCurrent; + + HighlightLines(lineFrom, lineTo); + } +} + void wxListMainWindow::OnArrowChar(size_t newCurrent, const wxKeyEvent& event) { wxCHECK_RET( newCurrent < (size_t)GetItemCount(), @@ -2746,43 +2898,34 @@ void wxListMainWindow::OnArrowChar(size_t newCurrent, const wxKeyEvent& event) size_t oldCurrent = m_current; + ChangeCurrent(newCurrent); + // in single selection we just ignore Shift as we can't select several // items anyhow if ( event.ShiftDown() && !IsSingleSel() ) { - ChangeCurrent(newCurrent); - - // refresh the old focus to remove it - RefreshLine( oldCurrent ); - - // select all the items between the old and the new one - if ( oldCurrent > newCurrent ) - { - newCurrent = oldCurrent; - oldCurrent = m_current; - } - - HighlightLines(oldCurrent, newCurrent); + ExtendSelection(oldCurrent, newCurrent); } else // !shift { - // all previously selected items are unselected unless ctrl is held - // in a multiselection control + // all previously selected items are unselected unless ctrl is held in + // a multi-selection control. in single selection mode we must always + // have a selected item. if ( !event.ControlDown() || IsSingleSel() ) - HighlightAll(false); + { + HighlightOnly(m_current, oldCurrent); - ChangeCurrent(newCurrent); - - // refresh the old focus to remove it - RefreshLine( oldCurrent ); - - // in single selection mode we must always have a selected item - if ( !event.ControlDown() || IsSingleSel() ) - HighlightLine( m_current, true ); + // Update anchor + m_anchor = m_current; + } + else + { + // refresh the old/new focus to remove/add it + RefreshLine(oldCurrent); + RefreshLine(m_current); + } } - RefreshLine( m_current ); - MoveToFocus(); } @@ -2799,10 +2942,11 @@ void wxListMainWindow::OnKeyDown( wxKeyEvent &event ) // send a list event wxListEvent le( wxEVT_LIST_KEY_DOWN, parent->GetId() ); + const size_t current = ShouldSendEventForCurrent() ? m_current : (size_t)-1; le.m_item.m_itemId = - le.m_itemIndex = m_current; - if (HasCurrent()) - GetLine(m_current)->GetItem( 0, le.m_item ); + le.m_itemIndex = current; + if ( current != (size_t)-1 ) + GetLine(current)->GetItem( 0, le.m_item ); le.m_code = event.GetKeyCode(); le.SetEventObject( parent ); if (parent->GetEventHandler()->ProcessEvent( le )) @@ -2956,7 +3100,7 @@ void wxListMainWindow::OnChar( wxKeyEvent &event ) { ReverseHighlight(m_current); } - else // normal space press + else if ( ShouldSendEventForCurrent() ) // normal space press { SendNotify( m_current, wxEVT_LIST_ITEM_ACTIVATED ); } @@ -2969,7 +3113,7 @@ void wxListMainWindow::OnChar( wxKeyEvent &event ) case WXK_RETURN: case WXK_EXECUTE: - if ( event.HasModifiers() ) + if ( event.HasModifiers() || !ShouldSendEventForCurrent() ) { event.Skip(); break; @@ -3078,6 +3222,7 @@ void wxListMainWindow::OnSetFocus( wxFocusEvent &WXUNUSED(event) ) { m_hasFocus = true; + UpdateCurrent(); RefreshSelected(); } } @@ -3621,17 +3766,7 @@ int wxListMainWindow::GetSelectedItemCount() const if ( IsVirtual() ) return m_selStore.GetSelectedCount(); - // TODO: we probably should maintain the number of items selected even for - // non virtual controls as enumerating all lines is really slow... - size_t countSel = 0; - size_t count = GetItemCount(); - for ( size_t line = 0; line < count; line++ ) - { - if ( GetLine(line)->IsHighlighted() ) - countSel++; - } - - return countSel; + return m_selCount; } // ---------------------------------------------------------------------------- @@ -4036,9 +4171,6 @@ void wxListMainWindow::RecalculatePositions(bool noRefresh) if ( !noRefresh ) { - // FIXME: why should we call it from here? - UpdateCurrent(); - RefreshAll(); } } @@ -4059,7 +4191,13 @@ void wxListMainWindow::RefreshAll() void wxListMainWindow::UpdateCurrent() { if ( !HasCurrent() && !IsEmpty() ) - ChangeCurrent(0); + { + // Initialise m_current to the first item without sending any + // wxEVT_LIST_ITEM_FOCUSED event (typicaly when the control gains focus) + // and this is to allow changing the focused item using the arrow keys. + // which is the behaviour found in the wxMSW port. + ChangeCurrentWithoutEvent(0); + } } long wxListMainWindow::GetNextItem( long item, @@ -4248,6 +4386,10 @@ void wxListMainWindow::DoDeleteAllItems() m_countVirt = 0; m_selStore.Clear(); } + else + { + m_selCount = 0; + } if ( InReportView() ) ResetVisibleLinesRange(); diff --git a/tests/controls/listbasetest.cpp b/tests/controls/listbasetest.cpp index 2bee21811d..5673676e74 100644 --- a/tests/controls/listbasetest.cpp +++ b/tests/controls/listbasetest.cpp @@ -26,6 +26,7 @@ #include "wx/uiaction.h" #include "wx/imaglist.h" #include "wx/artprov.h" +#include "wx/stopwatch.h" void ListBaseTestCase::ColumnsOrder() { @@ -177,6 +178,123 @@ void ListBaseTestCase::ChangeMode() CPPUNIT_ASSERT_EQUAL( "First", list->GetItemText(0) ); } +#ifdef __WXGTK__ + #define wxGTK_TIMED_YIELD(t) \ + if ( !IsRunningUnderXVFB() ) \ + for ( wxStopWatch sw; sw.Time() < t; ) wxYield() +#else // !__WXGTK__ + #define wxGTK_TIMED_YIELD(t) +#endif // __WXGTK__ + +void ListBaseTestCase::MultiSelect() +{ +#if wxUSE_UIACTIONSIMULATOR + +#ifndef __WXMSW__ + // FIXME: This test fails on Travis CI although works fine on + // development machine, no idea why though! + if ( IsAutomaticTest() ) + return; +#endif // !__WXMSW__ + + wxListCtrl* const list = GetList(); + + EventCounter focused(list, wxEVT_LIST_ITEM_FOCUSED); + EventCounter selected(list, wxEVT_LIST_ITEM_SELECTED); + EventCounter deselected(list, wxEVT_LIST_ITEM_DESELECTED); + + list->InsertColumn(0, "Header"); + + for ( int i = 0; i < 10; ++i ) + list->InsertItem(i, wxString::Format("Item %d", i)); + + wxUIActionSimulator sim; + + wxRect pos; + list->GetItemRect(2, pos); // Choose the third item as anchor + + // We move in slightly so we are not on the edge + wxPoint point = list->ClientToScreen(pos.GetPosition()) + wxPoint(10, 10); + + sim.MouseMove(point); + wxYield(); + + sim.MouseClick(); // select the anchor + wxYield(); + + wxGTK_TIMED_YIELD(50); + + list->GetItemRect(5, pos); + point = list->ClientToScreen(pos.GetPosition()) + wxPoint(10, 10); + + sim.MouseMove(point); + wxYield(); + + sim.KeyDown(WXK_SHIFT); + sim.MouseClick(); + sim.KeyUp(WXK_SHIFT); + wxYield(); + + wxGTK_TIMED_YIELD(10); + + // when the first item was selected the focus changes to it, but not + // on subsequent clicks + CPPUNIT_ASSERT_EQUAL(4, list->GetSelectedItemCount()); // item 2 to 5 (inclusive) are selected + CPPUNIT_ASSERT_EQUAL(2, focused.GetCount()); // count the focus which was on the anchor + CPPUNIT_ASSERT_EQUAL(4, selected.GetCount()); + CPPUNIT_ASSERT_EQUAL(0, deselected.GetCount()); + + focused.Clear(); + selected.Clear(); + deselected.Clear(); + + sim.Char(WXK_END, wxMOD_SHIFT); // extend the selection to the last item + wxYield(); + + wxGTK_TIMED_YIELD(10); + + CPPUNIT_ASSERT_EQUAL(8, list->GetSelectedItemCount()); // item 2 to 9 (inclusive) are selected + CPPUNIT_ASSERT_EQUAL(1, focused.GetCount()); // focus is on the last item + CPPUNIT_ASSERT_EQUAL(4, selected.GetCount()); // only newly selected items got the event + CPPUNIT_ASSERT_EQUAL(0, deselected.GetCount()); + + focused.Clear(); + selected.Clear(); + deselected.Clear(); + + sim.Char(WXK_HOME, wxMOD_SHIFT); // select from anchor to the first item + wxYield(); + + wxGTK_TIMED_YIELD(10); + + CPPUNIT_ASSERT_EQUAL(3, list->GetSelectedItemCount()); // item 0 to 2 (inclusive) are selected + CPPUNIT_ASSERT_EQUAL(1, focused.GetCount()); // focus is on item 0 + CPPUNIT_ASSERT_EQUAL(2, selected.GetCount()); // events are only generated for item 0 and 1 + CPPUNIT_ASSERT_EQUAL(7, deselected.GetCount()); // item 2 (exclusive) to 9 are deselected + + focused.Clear(); + selected.Clear(); + deselected.Clear(); + + list->EnsureVisible(0); + wxYield(); + + list->GetItemRect(2, pos); + point = list->ClientToScreen(pos.GetPosition()) + wxPoint(10, 10); + + sim.MouseMove(point); + wxYield(); + + sim.MouseClick(); + wxYield(); + + CPPUNIT_ASSERT_EQUAL(1, list->GetSelectedItemCount()); // anchor is the only selected item + CPPUNIT_ASSERT_EQUAL(1, focused.GetCount()); // because the focus changed from item 0 to anchor + CPPUNIT_ASSERT_EQUAL(0, selected.GetCount()); // anchor is already in selection state + CPPUNIT_ASSERT_EQUAL(2, deselected.GetCount()); // items 0 and 1 are deselected +#endif // wxUSE_UIACTIONSIMULATOR +} + void ListBaseTestCase::ItemClick() { #if wxUSE_UIACTIONSIMULATOR @@ -236,16 +354,9 @@ void ListBaseTestCase::ItemClick() // when the first item was selected the focus changes to it, but not // on subsequent clicks - - // FIXME: This test fail under wxGTK & wxOSX because we get 3 FOCUSED events and - // 2 SELECTED ones instead of the one of each we expect for some - // reason, this needs to be debugged as it may indicate a bug in the - // generic wxListCtrl implementation. -#ifndef _WX_GENERIC_LISTCTRL_H_ CPPUNIT_ASSERT_EQUAL(1, focused.GetCount()); CPPUNIT_ASSERT_EQUAL(1, selected.GetCount()); CPPUNIT_ASSERT_EQUAL(1, deselected.GetCount()); -#endif CPPUNIT_ASSERT_EQUAL(1, activated.GetCount()); CPPUNIT_ASSERT_EQUAL(1, rclick.GetCount()); #endif // wxUSE_UIACTIONSIMULATOR diff --git a/tests/controls/listbasetest.h b/tests/controls/listbasetest.h index d5840767c9..8b8df808b7 100644 --- a/tests/controls/listbasetest.h +++ b/tests/controls/listbasetest.h @@ -26,6 +26,7 @@ protected: CPPUNIT_TEST( ChangeMode ); \ WXUISIM_TEST( ItemClick ); \ WXUISIM_TEST( KeyDown ); \ + WXUISIM_TEST( MultiSelect ); \ CPPUNIT_TEST( DeleteItems ); \ CPPUNIT_TEST( InsertItem ); \ CPPUNIT_TEST( Find ); \ @@ -40,6 +41,7 @@ protected: void ItemRect(); void ItemText(); void ChangeMode(); + void MultiSelect(); void ItemClick(); void KeyDown(); void DeleteItems(); diff --git a/tests/controls/listviewtest.cpp b/tests/controls/listviewtest.cpp index 94162d3eab..1ddd6c6f33 100644 --- a/tests/controls/listviewtest.cpp +++ b/tests/controls/listviewtest.cpp @@ -20,6 +20,7 @@ #include "wx/listctrl.h" #include "listbasetest.h" +#include "testableframe.h" class ListViewTestCase : public ListBaseTestCase, public CppUnit::TestCase { @@ -61,7 +62,8 @@ void ListViewTestCase::setUp() void ListViewTestCase::tearDown() { - wxDELETE(m_list); + DeleteTestWindow(m_list); + m_list = NULL; } void ListViewTestCase::Selection() @@ -104,6 +106,8 @@ void ListViewTestCase::Selection() void ListViewTestCase::Focus() { + EventCounter focused(m_list, wxEVT_LIST_ITEM_FOCUSED); + m_list->InsertColumn(0, "Column 0"); m_list->InsertItem(0, "Item 0"); @@ -111,10 +115,12 @@ void ListViewTestCase::Focus() m_list->InsertItem(2, "Item 2"); m_list->InsertItem(3, "Item 3"); + CPPUNIT_ASSERT_EQUAL(0, focused.GetCount()); CPPUNIT_ASSERT_EQUAL(-1, m_list->GetFocusedItem()); m_list->Focus(0); + CPPUNIT_ASSERT_EQUAL(1, focused.GetCount()); CPPUNIT_ASSERT_EQUAL(0, m_list->GetFocusedItem()); }