diff --git a/include/wx/bookctrl.h b/include/wx/bookctrl.h index d1065dbde5..324fd94ca1 100644 --- a/include/wx/bookctrl.h +++ b/include/wx/bookctrl.h @@ -256,6 +256,10 @@ protected: // false otherwise. bool DoSetSelectionAfterInsertion(size_t n, bool bSelect); + // Update the selection after removing the page at the given index, + // typically called from the derived class overridden DoRemovePage(). + void DoSetSelectionAfterRemoval(size_t n); + // set the selection to the given page, sending the events (which can // possibly prevent the page change from taking place) if SendEvent flag is // included @@ -292,7 +296,11 @@ protected: // having nodes without any associated page) virtual bool AllowNullPage() const { return false; } - // remove the page and return a pointer to it + // Remove the page and return a pointer to it. + // + // It also needs to update the current selection if necessary, i.e. if the + // page being removed comes before the selected one and the helper method + // DoSetSelectionAfterRemoval() can be used for this. virtual wxWindow *DoRemovePage(size_t page) = 0; // our best size is the size which fits all our pages diff --git a/include/wx/simplebook.h b/include/wx/simplebook.h index 9b6474b0c4..795391466c 100644 --- a/include/wx/simplebook.h +++ b/include/wx/simplebook.h @@ -156,8 +156,15 @@ protected: virtual wxWindow *DoRemovePage(size_t page) { - m_pageTexts.erase(m_pageTexts.begin() + page); - return wxBookCtrlBase::DoRemovePage(page); + wxWindow* const win = wxBookCtrlBase::DoRemovePage(page); + if ( win ) + { + m_pageTexts.erase(m_pageTexts.begin() + page); + + DoSetSelectionAfterRemoval(page); + } + + return win; } virtual void DoSize() diff --git a/src/common/bookctrl.cpp b/src/common/bookctrl.cpp index 9ec08bad1c..8d86c5343e 100644 --- a/src/common/bookctrl.cpp +++ b/src/common/bookctrl.cpp @@ -456,6 +456,26 @@ bool wxBookCtrlBase::DoSetSelectionAfterInsertion(size_t n, bool bSelect) return true; } +void wxBookCtrlBase::DoSetSelectionAfterRemoval(size_t n) +{ + if ( m_selection >= (int)n ) + { + // ensure that the selection is valid + int sel; + if ( GetPageCount() == 0 ) + sel = wxNOT_FOUND; + else + sel = m_selection ? m_selection - 1 : 0; + + // if deleting current page we shouldn't try to hide it + m_selection = m_selection == (int)n ? wxNOT_FOUND + : m_selection - 1; + + if ( sel != wxNOT_FOUND && sel != m_selection ) + SetSelection(sel); + } +} + int wxBookCtrlBase::DoSetSelection(size_t n, int flags) { wxCHECK_MSG( n < GetPageCount(), wxNOT_FOUND, diff --git a/src/generic/choicbkg.cpp b/src/generic/choicbkg.cpp index c8b2c5acf5..5c0ac7a514 100644 --- a/src/generic/choicbkg.cpp +++ b/src/generic/choicbkg.cpp @@ -209,22 +209,7 @@ wxWindow *wxChoicebook::DoRemovePage(size_t page) { GetChoiceCtrl()->Delete(page); - if ( m_selection >= (int)page ) - { - // ensure that the selection is valid - int sel; - if ( GetPageCount() == 0 ) - sel = wxNOT_FOUND; - else - sel = m_selection ? m_selection - 1 : 0; - - // if deleting current page we shouldn't try to hide it - m_selection = m_selection == (int)page ? wxNOT_FOUND - : m_selection - 1; - - if ( sel != wxNOT_FOUND && sel != m_selection ) - SetSelection(sel); - } + DoSetSelectionAfterRemoval(page); } return win; diff --git a/src/generic/listbkg.cpp b/src/generic/listbkg.cpp index ccdc712780..ba736367c7 100644 --- a/src/generic/listbkg.cpp +++ b/src/generic/listbkg.cpp @@ -348,28 +348,13 @@ wxListbook::InsertPage(size_t n, wxWindow *wxListbook::DoRemovePage(size_t page) { - const size_t page_count = GetPageCount(); wxWindow *win = wxBookCtrlBase::DoRemovePage(page); if ( win ) { GetListView()->DeleteItem(page); - if (m_selection >= (int)page) - { - // force new sel valid if possible - int sel = m_selection - 1; - if (page_count == 1) - sel = wxNOT_FOUND; - else if ((page_count == 2) || (sel == wxNOT_FOUND)) - sel = 0; - - // force sel invalid if deleting current page - don't try to hide it - m_selection = (m_selection == (int)page) ? wxNOT_FOUND : m_selection - 1; - - if ((sel != wxNOT_FOUND) && (sel != m_selection)) - SetSelection(sel); - } + DoSetSelectionAfterRemoval(page); GetListView()->Arrange(); UpdateSize(); diff --git a/src/generic/toolbkg.cpp b/src/generic/toolbkg.cpp index 1fe2ad781d..f324ea96db 100644 --- a/src/generic/toolbkg.cpp +++ b/src/generic/toolbkg.cpp @@ -333,28 +333,13 @@ bool wxToolbook::InsertPage(size_t n, wxWindow *wxToolbook::DoRemovePage(size_t page) { - const size_t page_count = GetPageCount(); wxWindow *win = wxBookCtrlBase::DoRemovePage(page); if ( win ) { GetToolBar()->DeleteTool(page + 1); - if (m_selection >= (int)page) - { - // force new sel valid if possible - int sel = m_selection - 1; - if (page_count == 1) - sel = wxNOT_FOUND; - else if ((page_count == 2) || (sel == wxNOT_FOUND)) - sel = 0; - - // force sel invalid if deleting current page - don't try to hide it - m_selection = (m_selection == (int)page) ? wxNOT_FOUND : m_selection - 1; - - if ((sel != wxNOT_FOUND) && (sel != m_selection)) - SetSelection(sel); - } + DoSetSelectionAfterRemoval(page); } return win; diff --git a/tests/controls/bookctrlbasetest.cpp b/tests/controls/bookctrlbasetest.cpp index c5065607e2..acd787f6fa 100644 --- a/tests/controls/bookctrlbasetest.cpp +++ b/tests/controls/bookctrlbasetest.cpp @@ -95,19 +95,25 @@ void BookCtrlBaseTestCase::PageManagement() CPPUNIT_ASSERT_EQUAL(0, base->GetSelection()); CPPUNIT_ASSERT_EQUAL(4, base->GetPageCount()); + // Change the selection to verify that deleting a page before the currently + // selected one correctly updates the selection. + base->SetSelection(2); + CPPUNIT_ASSERT_EQUAL(2, base->GetSelection()); + base->DeletePage(1); CPPUNIT_ASSERT_EQUAL(3, base->GetPageCount()); + CPPUNIT_ASSERT_EQUAL(1, base->GetSelection()); base->RemovePage(0); CPPUNIT_ASSERT_EQUAL(2, base->GetPageCount()); + CPPUNIT_ASSERT_EQUAL(0, base->GetSelection()); base->DeleteAllPages(); CPPUNIT_ASSERT_EQUAL(0, base->GetPageCount()); - - AddPanels(); + CPPUNIT_ASSERT_EQUAL(-1, base->GetSelection()); } void BookCtrlBaseTestCase::ChangeEvents()