From 02a92e23f35fe183901273ddfca2d6d604b921da Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 25 Mar 2018 00:13:15 +0100 Subject: [PATCH] Reuse wxBookCtrlBase::DoSetSelection() in wxTreebook too Avoid duplicating base class DoSetSelection() implementation in wxTreebook, just extend it slightly by using DoGetNonNullPage() to allow using a (sub-)page if the page associated to the selected item is null and reuse it. Also get rid of wxTreebook::m_actualSelection, it seems completely unnecessary to bother keeping and updating it when we can just find it whenever we need (which actually seems to only have been the case in the now removed DoSetSelection() implementation anyhow). As a side effect of this, wxTreebook pages should now be sizer correctly when switching to them as DoSetSelection() in the base class does call SetSize() on the page before showing it, unlike the previously used version in wxTreebook, which omitted this call for some reason. There should be no other user-visible changes. Closes #4379. --- include/wx/bookctrl.h | 6 ++ include/wx/treebook.h | 24 ++----- src/common/bookctrl.cpp | 13 +++- src/generic/treebkg.cpp | 139 +++++++++------------------------------- 4 files changed, 53 insertions(+), 129 deletions(-) diff --git a/include/wx/bookctrl.h b/include/wx/bookctrl.h index aad4834a55..963de7a91a 100644 --- a/include/wx/bookctrl.h +++ b/include/wx/bookctrl.h @@ -292,6 +292,12 @@ protected: // having nodes without any associated page) virtual bool AllowNullPage() const { return false; } + // For classes that allow null pages, we also need a way to find the + // closest non-NULL page corresponding to the given index, e.g. the first + // leaf item in wxTreebook tree and this method must be overridden to + // return it if AllowNullPage() is overridden. + virtual wxWindow *DoGetNonNullPage(size_t page) { return m_pages[page]; } + // Remove the page and return a pointer to it. // // It also needs to update the current selection if necessary, i.e. if the diff --git a/include/wx/treebook.h b/include/wx/treebook.h index abc95a87bc..119eb7dddd 100644 --- a/include/wx/treebook.h +++ b/include/wx/treebook.h @@ -38,7 +38,6 @@ public: // Default ctor doesn't create the control, use Create() afterwards wxTreebook() { - Init(); } // This ctor creates the tree book control @@ -49,8 +48,6 @@ public: long style = wxBK_DEFAULT, const wxString& name = wxEmptyString) { - Init(); - (void)Create(parent, id, pos, size, style, name); } @@ -145,22 +142,16 @@ protected: // This subclass of wxBookCtrlBase accepts NULL page pointers (empty pages) virtual bool AllowNullPage() const wxOVERRIDE { return true; } + virtual wxWindow *DoGetNonNullPage(size_t page) wxOVERRIDE; // event handlers void OnTreeSelectionChange(wxTreeEvent& event); void OnTreeNodeExpandedCollapsed(wxTreeEvent& event); - // array of page ids and page windows + // array of tree item ids corresponding to the page indices wxVector m_treeIds; - // in the situation when m_selection page is not wxNOT_FOUND but page is - // NULL this is the first (sub)child that has a non-NULL page - int m_actualSelection; - private: - // common part of all constructors - void Init(); - // The real implementations of page insertion functions // ------------------------------------------------------ // All DoInsert/Add(Sub)Page functions add the page into : @@ -182,12 +173,11 @@ private: bool bSelect = false, int imageId = NO_IMAGE); - // Sets selection in the tree control and updates the page being shown. - int DoSetSelection(size_t pos, int flags = 0) wxOVERRIDE; - - // Returns currently shown page. In a case when selected the node - // has empty (NULL) page finds first (sub)child with not-empty page. - wxTreebookPage *DoGetCurrentPage() const; + // Overridden methods used by the base class DoSetSelection() + // implementation. + void UpdateSelectedPage(size_t newsel) wxOVERRIDE; + wxBookCtrlEvent* CreatePageChangingEvent() const wxOVERRIDE; + void MakeChangedEvent(wxBookCtrlEvent &event) wxOVERRIDE; // Does the selection update. Called from page insertion functions // to update selection if the selected page was pushed by the newly inserted diff --git a/src/common/bookctrl.cpp b/src/common/bookctrl.cpp index e027fd9052..c2dde5fcfe 100644 --- a/src/common/bookctrl.cpp +++ b/src/common/bookctrl.cpp @@ -493,9 +493,9 @@ int wxBookCtrlBase::DoSetSelection(size_t n, int flags) if ( allowed ) { if ( oldSel != wxNOT_FOUND ) - DoShowPage(m_pages[oldSel], false); + DoShowPage(DoGetNonNullPage(oldSel), false); - wxWindow *page = m_pages[n]; + wxWindow* const page = DoGetNonNullPage(n); page->SetSize(GetPageRect()); DoShowPage(page, true); @@ -510,6 +510,15 @@ int wxBookCtrlBase::DoSetSelection(size_t n, int flags) (void)GetEventHandler()->ProcessEvent(*event); } } + else + { + // Selection in the control might have already had changed. + if ( oldSel != wxNOT_FOUND ) + { + m_selection = oldSel; + UpdateSelectedPage(oldSel); + } + } delete event; } diff --git a/src/generic/treebkg.cpp b/src/generic/treebkg.cpp index 5146bd7be2..0abc57f62c 100644 --- a/src/generic/treebkg.cpp +++ b/src/generic/treebkg.cpp @@ -66,12 +66,6 @@ wxEND_EVENT_TABLE() // wxTreebook creation // ---------------------------------------------------------------------------- -void wxTreebook::Init() -{ - m_selection = - m_actualSelection = wxNOT_FOUND; -} - bool wxTreebook::Create(wxWindow *parent, wxWindowID id, @@ -314,8 +308,6 @@ bool wxTreebook::DeleteAllPages() { wxBookCtrlBase::DeleteAllPages(); m_treeIds.clear(); - m_selection = - m_actualSelection = wxNOT_FOUND; wxTreeCtrl *tree = GetTreeCtrl(); tree->DeleteChildren(tree->GetRootItem()); @@ -347,13 +339,6 @@ void wxTreebook::DoInternalAddPage(size_t newPos, { // selection has been moved one unit toward the end ++m_selection; - if ( m_actualSelection != wxNOT_FOUND ) - ++m_actualSelection; - } - else if ( m_actualSelection != wxNOT_FOUND && - newPos <= (size_t)m_actualSelection ) - { - DoSetSelection(m_selection); } } } @@ -377,10 +362,6 @@ void wxTreebook::DoInternalRemovePageRange(size_t pagePos, size_t subCount) { // selection is far after the deleted page, so just update the index and move on m_selection -= 1 + subCount; - if ( m_actualSelection != wxNOT_FOUND) - { - m_actualSelection -= subCount + 1; - } } else if ( (size_t)m_selection >= pagePos ) { @@ -391,7 +372,6 @@ void wxTreebook::DoInternalRemovePageRange(size_t pagePos, size_t subCount) wxTreeItemId nodeId = tree->GetNextSibling(pageId); m_selection = wxNOT_FOUND; - m_actualSelection = wxNOT_FOUND; if ( nodeId.IsOk() ) { @@ -413,17 +393,6 @@ void wxTreebook::DoInternalRemovePageRange(size_t pagePos, size_t subCount) } } } - else if ( m_actualSelection != wxNOT_FOUND && - (size_t)m_actualSelection >= pagePos ) - { - // nothing to do -- selection is before the deleted node, but - // actually shown page (the first (sub)child with page != NULL) is - // already deleted - m_actualSelection = m_selection; - - // send event as documented - DoSetSelection(m_selection, SetSelection_SendEvent); - } //else: nothing to do -- selection is before the deleted node } else @@ -558,91 +527,41 @@ bool wxTreebook::SetPageImage(size_t n, int imageId) return true; } -int wxTreebook::DoSetSelection(size_t pagePos, int flags) +void wxTreebook::UpdateSelectedPage(size_t newsel) { - wxCHECK_MSG( IS_VALID_PAGE(pagePos), wxNOT_FOUND, - wxT("invalid page index in wxListbook::DoSetSelection()") ); - wxASSERT_MSG( GetPageCount() == DoInternalGetPageCount(), - wxT("wxTreebook logic error: m_treeIds and m_pages not in sync!")); - - wxBookCtrlEvent event(wxEVT_TREEBOOK_PAGE_CHANGING, m_windowId); - const int oldSel = m_selection; - wxTreeCtrl *tree = GetTreeCtrl(); - bool allowed = false; - - if (flags & SetSelection_SendEvent) - { - event.SetEventObject(this); - event.SetSelection(pagePos); - event.SetOldSelection(m_selection); - - // don't send the event if the old and new pages are the same; do send it - // otherwise and be prepared for it to be vetoed - allowed = (int)pagePos == m_selection || - !GetEventHandler()->ProcessEvent(event) || - event.IsAllowed(); - } - - if ( !(flags & SetSelection_SendEvent) || allowed ) - { - // hide the previously shown page - wxTreebookPage * const oldPage = DoGetCurrentPage(); - if ( oldPage ) - oldPage->Hide(); - - // then show the new one - m_selection = pagePos; - wxTreebookPage *page = wxBookCtrlBase::GetPage(m_selection); - if ( !page ) - { - // find the next page suitable to be shown: the first (grand)child - // of this one with a non-NULL associated page - wxTreeItemId childId = m_treeIds[pagePos]; - int actualPagePos = pagePos; - while ( !page && childId.IsOk() ) - { - wxTreeItemIdValue cookie; - childId = tree->GetFirstChild( childId, cookie ); - if ( childId.IsOk() ) - { - page = wxBookCtrlBase::GetPage(++actualPagePos); - } - } - - m_actualSelection = page ? actualPagePos : m_selection; - } - - if ( page ) - page->Show(); - - tree->SelectItem(DoInternalGetPage(pagePos)); - - if (flags & SetSelection_SendEvent) - { - // notify about the (now completed) page change - event.SetEventType(wxEVT_TREEBOOK_PAGE_CHANGED); - (void)GetEventHandler()->ProcessEvent(event); - } - } - else if ( (flags & SetSelection_SendEvent) && !allowed) // page change vetoed - { - // tree selection might have already had changed - if ( oldSel != wxNOT_FOUND ) - tree->SelectItem(DoInternalGetPage(oldSel)); - } - - return oldSel; + GetTreeCtrl()->SelectItem(DoInternalGetPage(newsel)); } -wxTreebookPage *wxTreebook::DoGetCurrentPage() const +wxBookCtrlEvent* wxTreebook::CreatePageChangingEvent() const { - if ( m_selection == wxNOT_FOUND ) - return NULL; + return new wxBookCtrlEvent(wxEVT_TREEBOOK_PAGE_CHANGING, m_windowId); +} - wxTreebookPage *page = wxBookCtrlBase::GetPage(m_selection); - if ( !page && m_actualSelection != wxNOT_FOUND ) +void wxTreebook::MakeChangedEvent(wxBookCtrlEvent &event) +{ + event.SetEventType(wxEVT_TREEBOOK_PAGE_CHANGED); +} + +wxWindow *wxTreebook::DoGetNonNullPage(size_t n) +{ + wxWindow* page = wxBookCtrlBase::GetPage(n); + + if ( !page ) { - page = wxBookCtrlBase::GetPage(m_actualSelection); + // Find the next suitable page, i.e. the first (grand)child + // of this one with a non-NULL associated page + wxTreeCtrl* const tree = GetTreeCtrl(); + for ( wxTreeItemId childId = m_treeIds[n]; childId.IsOk(); ) + { + wxTreeItemIdValue cookie; + childId = tree->GetFirstChild( childId, cookie ); + if ( childId.IsOk() ) + { + page = wxBookCtrlBase::GetPage(++n); + if ( page ) + break; + } + } } return page;