From 77d5d316909aa99cba2e2a0e224164399bc53890 Mon Sep 17 00:00:00 2001 From: Matthew Griffin <45285214+matthew-griffin@users.noreply.github.com> Date: Thu, 30 May 2019 10:47:09 +0100 Subject: [PATCH] Close the Tree Control editor correctly Ensure that wxTreeItemData is deleted. Use a QT delegate to create editor and perform custom model update. Connect to the closeEditor signal to send out end label edit events and decide whether to accept changes. --- Makefile.in | 1 + build/bakefiles/files.bkl | 1 + build/cmake/files.cmake | 1 + build/files | 1 + include/wx/qt/private/treeitemdelegate.h | 69 ++++++++++++++++++++++++ src/qt/treectrl.cpp | 59 +++++++++++++++----- 6 files changed, 119 insertions(+), 13 deletions(-) create mode 100644 include/wx/qt/private/treeitemdelegate.h diff --git a/Makefile.in b/Makefile.in index 2afb48ef2a..d3829be762 100644 --- a/Makefile.in +++ b/Makefile.in @@ -3579,6 +3579,7 @@ COND_TOOLKIT_QT_GUI_HDR = \ wx/qt/treectrl.h \ wx/qt/private/winevent.h \ wx/qt/private/converter.h \ + wx/qt/private/treeitemdelegate.h \ wx/qt/private/treeitemfactory.h \ wx/qt/private/pointer.h \ wx/qt/private/timer.h \ diff --git a/build/bakefiles/files.bkl b/build/bakefiles/files.bkl index f84b961111..21c0127c7a 100644 --- a/build/bakefiles/files.bkl +++ b/build/bakefiles/files.bkl @@ -364,6 +364,7 @@ IMPORTANT: please read docs/tech/tn0016.txt before modifying this file! wx/qt/treectrl.h wx/qt/private/winevent.h wx/qt/private/converter.h + wx/qt/private/treeitemdelegate.h wx/qt/private/treeitemfactory.h wx/qt/private/pointer.h wx/qt/private/timer.h diff --git a/build/cmake/files.cmake b/build/cmake/files.cmake index 799d7e5b6c..bae2091acd 100644 --- a/build/cmake/files.cmake +++ b/build/cmake/files.cmake @@ -281,6 +281,7 @@ set(QT_HDR wx/qt/private/winevent.h wx/qt/private/timer.h wx/qt/private/pointer.h + wx/qt/private/treeitemdelegate.h wx/qt/private/treeitemfactory.h wx/qt/private/utils.h ) diff --git a/build/files b/build/files index 78c5e6391c..a5d2125aba 100644 --- a/build/files +++ b/build/files @@ -305,6 +305,7 @@ QT_HDR = wx/qt/private/converter.h wx/qt/private/pointer.h wx/qt/private/timer.h + wx/qt/private/treeitemdelegate.h wx/qt/private/treeitemfactory.h wx/qt/private/utils.h diff --git a/include/wx/qt/private/treeitemdelegate.h b/include/wx/qt/private/treeitemdelegate.h new file mode 100644 index 0000000000..a6d38df06d --- /dev/null +++ b/include/wx/qt/private/treeitemdelegate.h @@ -0,0 +1,69 @@ +///////////////////////////////////////////////////////////////////////////// +// Name: wx/qt/private/treeitemdelegate.h +// Purpose: Delegate to create text edit controls for the tree items +// Author: Matthew Griffin +// Created: 2019-05-29 +// Copyright: Matthew Griffin +// Licence: wxWindows licence +///////////////////////////////////////////////////////////////////////////// + +#ifndef _WX_TREEITEM_DELEGATE_H +#define _WX_TREEITEM_DELEGATE_H + +#include + +#include "wx/textctrl.h" + +#include "treeitemfactory.h" + +class wxQTTreeItemDelegate : public QStyledItemDelegate +{ +public: + explicit wxQTTreeItemDelegate(wxWindow* parent) + : m_parent(parent), + m_textCtrl(NULL) + { + } + + QWidget* createEditor(QWidget *parent, const QStyleOptionViewItem &WXUNUSED(option), const QModelIndex &index) const wxOVERRIDE + { + m_current_model_index = index; + m_textCtrl = new wxQtListTextCtrl(m_parent, parent); + m_textCtrl->SetFocus(); + return m_textCtrl->GetHandle(); + } + + void destroyEditor(QWidget *WXUNUSED(editor), const QModelIndex &WXUNUSED(index)) const wxOVERRIDE + { + m_current_model_index = QModelIndex(); + delete m_textCtrl; + m_textCtrl = NULL; + } + + void setModelData(QWidget *editor, QAbstractItemModel *model, const QModelIndex &index) const wxOVERRIDE + { + // Don't set model data until wx has had a chance to send out events + } + + wxTextCtrl* GetEditControl() const + { + return m_textCtrl; + } + + QModelIndex GetCurrentModelIndex() const + { + return m_current_model_index; + } + + void AcceptModelData(QWidget *editor, QAbstractItemModel *model, const QModelIndex &index) const + { + QStyledItemDelegate::setModelData(editor, model, index); + } + +private: + wxWindow* m_parent; + mutable wxTextCtrl* m_textCtrl; + mutable QModelIndex m_current_model_index; +}; + +#endif // _WX_TREEITEM_DELEGATE_H diff --git a/src/qt/treectrl.cpp b/src/qt/treectrl.cpp index 521a3238fd..107649dcfc 100644 --- a/src/qt/treectrl.cpp +++ b/src/qt/treectrl.cpp @@ -17,7 +17,7 @@ #include "wx/settings.h" #include "wx/qt/private/winevent.h" -#include "wx/qt/private/treeitemfactory.h" +#include "wx/qt/private/treeitemdelegate.h" #include #include @@ -27,7 +27,7 @@ namespace { struct TreeItemDataQt { - TreeItemDataQt() : data(NULL) + TreeItemDataQt() { } @@ -40,9 +40,13 @@ struct TreeItemDataQt } } - wxTreeItemData *data; + wxTreeItemData *getData() const + { + return data.get(); + } private: + wxSharedPtr data; static bool registered; }; @@ -126,7 +130,8 @@ class wxQTreeWidget : public wxQtEventSignalHandler public: wxQTreeWidget(wxWindow *parent, wxTreeCtrl *handler) : wxQtEventSignalHandler(parent, handler), - m_editorFactory(handler) + m_item_delegate(handler), + m_closing_editor(0) { connect(this, &QTreeWidget::currentItemChanged, this, &wxQTreeWidget::OnCurrentItemChanged); @@ -138,10 +143,11 @@ public: this, &wxQTreeWidget::OnItemCollapsed); connect(this, &QTreeWidget::itemExpanded, this, &wxQTreeWidget::OnItemExpanded); - connect(this, &QTreeWidget::itemChanged, - this, &wxQTreeWidget::OnItemChanged); - m_editorFactory.AttachTo(this); + connect(&m_item_delegate, &QAbstractItemDelegate::closeEditor, + this, &wxQTreeWidget::OnCloseEditor); + + setItemDelegate(&m_item_delegate); setDragEnabled(true); viewport()->setAcceptDrops(true); setDropIndicatorShown(true); @@ -157,7 +163,7 @@ public: wxTextCtrl *GetEditControl() { - return m_editorFactory.GetEditControl(); + return m_item_delegate.GetEditControl(); } void SetItemImage(QTreeWidgetItem *item, int image, wxTreeItemIcon which) @@ -346,14 +352,40 @@ private: EmitEvent(expandedEvent); } - void OnItemChanged(QTreeWidgetItem *item, int WXUNUSED(column)) + void OnCloseEditor(QWidget *editor, QAbstractItemDelegate::EndEditHint hint) { + // Close process can re-signal closeEditor so we need to guard against + // reentrant calls. + wxRecursionGuard guard(m_closing_editor); + + if (guard.IsInside()) + return; + + // There can be multiple calls to close editor when the item loses focus + const QModelIndex current_index = m_item_delegate.GetCurrentModelIndex(); + if (!current_index.isValid()) + return; + wxTreeEvent event( wxEVT_TREE_END_LABEL_EDIT, GetHandler(), - wxQtConvertTreeItem(item) + wxQtConvertTreeItem(itemFromIndex(current_index)) ); - EmitEvent(event); + if (hint == QAbstractItemDelegate::EndEditHint::RevertModelCache) + { + event.SetEditCanceled(true); + EmitEvent(event); + } + else + { + // Allow event handlers to decide whether to accept edited text + const wxString editor_text = m_item_delegate.GetEditControl()->GetLineText(0); + event.SetLabel(editor_text); + if (!GetHandler()->HandleWindowEvent(event) || event.IsAllowed()) + m_item_delegate.AcceptModelData(editor, model(), current_index); + } + + QAbstractItemView::closePersistentEditor(current_index); } void tryStartDrag(const QMouseEvent *event) @@ -451,7 +483,8 @@ private: return imageIndex; } - wxQtTreeItemEditorFactory m_editorFactory; + wxQTTreeItemDelegate m_item_delegate; + wxRecursionGuardFlag m_closing_editor; typedef std::map ImageStateMap; ImageStateMap m_imageStates; @@ -557,7 +590,7 @@ wxTreeItemData *wxTreeCtrl::GetItemData(const wxTreeItemId& item) const const QTreeWidgetItem* qTreeItem = wxQtConvertTreeItem(item); const QVariant itemData = qTreeItem->data(0, Qt::UserRole); const TreeItemDataQt value = itemData.value(); - return value.data; + return value.getData(); } wxColour wxTreeCtrl::GetItemTextColour(const wxTreeItemId& item) const