From 5591a2009394c9bae43bcd2fa08f2fb51391cbc1 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 18 Nov 2015 02:08:34 +0100 Subject: [PATCH 01/13] Detect attempts to catch wxEVT_TEXT_ENTER without wxTE_PROCESS_ENTER This is never going to work, so complain about trying to do it to help with catching this bug. This is possible thanks to the new OnDynamicBind() method invoked whenever a dynamic event handler is bound to a control, so this doesn't detect all possible occurrences of the bug (such as specifying the handler in a static event table or in a validator), but it's still better than nothing. In the future OnDynamicBind() should be extended for other invalid calls, e.g. binding a handler for wxEVT_TEXT_ENTER to a non-text control shouldn't work neither, ideally. --- include/wx/event.h | 9 +++++++++ include/wx/textctrl.h | 5 +++++ src/common/event.cpp | 7 +++++++ src/common/textcmn.cpp | 15 +++++++++++++++ 4 files changed, 36 insertions(+) diff --git a/include/wx/event.h b/include/wx/event.h index 0666f3c8f4..c70f159f6e 100644 --- a/include/wx/event.h +++ b/include/wx/event.h @@ -3671,6 +3671,15 @@ protected: virtual bool TryParent(wxEvent& event), return DoTryApp(event); ) #endif // WXWIN_COMPATIBILITY_2_8 + // Overriding this method allows filtering the event handlers dynamically + // connected to this object. If this method returns false, the handler is + // not connected at all. If it returns true, it is connected using the + // possibly modified fields of the given entry. + virtual bool OnDynamicBind(wxDynamicEventTableEntry& WXUNUSED(entry)) + { + return true; + } + static const wxEventTable sm_eventTable; virtual const wxEventTable *GetEventTable() const; diff --git a/include/wx/textctrl.h b/include/wx/textctrl.h index e83221f48a..cd7a815a6e 100644 --- a/include/wx/textctrl.h +++ b/include/wx/textctrl.h @@ -743,6 +743,11 @@ public: } protected: + // Override wxEvtHandler method to check for a common problem of binding + // wxEVT_TEXT_ENTER to a control without wxTE_PROCESS_ENTER style, which is + // never going to work. + virtual bool OnDynamicBind(wxDynamicEventTableEntry& entry); + // override streambuf method #if wxHAS_TEXT_WINDOW_STREAM int overflow(int i) wxOVERRIDE; diff --git a/src/common/event.cpp b/src/common/event.cpp index 3e7f76f614..2faf29971c 100644 --- a/src/common/event.cpp +++ b/src/common/event.cpp @@ -1693,6 +1693,13 @@ void wxEvtHandler::DoBind(int id, wxDynamicEventTableEntry *entry = new wxDynamicEventTableEntry(eventType, id, lastId, func, userData); + // Check if the derived class allows binding such event handlers. + if ( !OnDynamicBind(*entry) ) + { + delete entry; + return; + } + if (!m_dynamicEvents) m_dynamicEvents = new DynamicEvents; diff --git a/src/common/textcmn.cpp b/src/common/textcmn.cpp index baefa1c8c6..2774635bbe 100644 --- a/src/common/textcmn.cpp +++ b/src/common/textcmn.cpp @@ -1194,6 +1194,21 @@ void wxTextCtrlBase::DoUpdateWindowUI(wxUpdateUIEvent& event) } } +bool wxTextCtrlBase::OnDynamicBind(wxDynamicEventTableEntry& entry) +{ + if ( entry.m_eventType == wxEVT_TEXT_ENTER ) + { + wxCHECK_MSG + ( + HasFlag(wxTE_PROCESS_ENTER), + false, + wxS("Must have wxTE_PROCESS_ENTER for wxEVT_TEXT_ENTER to work") + ); + } + + return wxControl::OnDynamicBind(entry); +} + // ---------------------------------------------------------------------------- // hit testing // ---------------------------------------------------------------------------- From 6b2a6baf2e9733c4a98838dac15fcc1476f10e98 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 18 Nov 2015 22:56:09 +0100 Subject: [PATCH 02/13] Allow custom handling of Enter/Tab with modifiers in wxDataViewCtrl While Enter and Tab on their own should be used to finish cell editing, the cell editor itself may want to process key combinations involving these keys with modifiers, e.g. Shift-Enter, so don't intercept those in at least the generic version of wxDataViewCtrl to allow catching them in the editor. --- src/common/datavcmn.cpp | 17 ++++++++++------- src/generic/datavgen.cpp | 10 ++++++++++ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/common/datavcmn.cpp b/src/common/datavcmn.cpp index e94da72067..3f4f8299af 100644 --- a/src/common/datavcmn.cpp +++ b/src/common/datavcmn.cpp @@ -1044,17 +1044,20 @@ void wxDataViewEditorCtrlEvtHandler::OnChar( wxKeyEvent &event ) { switch ( event.m_keyCode ) { - case WXK_RETURN: - m_finished = true; - m_owner->FinishEditing(); - break; - case WXK_ESCAPE: - { m_finished = true; m_owner->CancelEditing(); break; - } + + case WXK_RETURN: + if ( !event.HasAnyModifiers() ) + { + m_finished = true; + m_owner->FinishEditing(); + break; + } + wxFALLTHROUGH; // Ctrl/Alt/Shift-Enter is not handled specially + default: event.Skip(); } diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index cd2c2b4d95..3cad87b018 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -3632,7 +3632,17 @@ void wxDataViewMainWindow::OnCharHook(wxKeyEvent& event) return; case WXK_RETURN: + // Shift-Enter is not special neither. + if ( event.ShiftDown() ) + break; + wxFALLTHROUGH; + case WXK_TAB: + // Ctrl/Alt-Tab or Enter could be used for something else, so + // don't handle them here. + if ( event.HasModifiers() ) + break; + m_editorRenderer->FinishEditing(); return; } From 9bcaa058a0ff21bc4ab73cd9043b96d9f079c03e Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 18 Nov 2015 23:32:14 +0100 Subject: [PATCH 03/13] Replace wxTextCtrl::MSWWindowProc() with MSWHandleMessage() Override newer and more flexible virtual method: this doesn't change anything yet but will allow to provide default handling for some messages in a single overridden method in the future commits, when it would have required to also override MSWDefWindowProc() with the old method. --- include/wx/msw/textctrl.h | 5 ++++- src/msw/textctrl.cpp | 25 +++++++++++++++++++------ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/include/wx/msw/textctrl.h b/include/wx/msw/textctrl.h index 153e0a4f07..a2027003f7 100644 --- a/include/wx/msw/textctrl.h +++ b/include/wx/msw/textctrl.h @@ -181,7 +181,10 @@ public: void OnSetFocus(wxFocusEvent& event); // intercept WM_GETDLGCODE - virtual WXLRESULT MSWWindowProc(WXUINT nMsg, WXWPARAM wParam, WXLPARAM lParam); + virtual bool MSWHandleMessage(WXLRESULT *result, + WXUINT message, + WXWPARAM wParam, + WXLPARAM lParam); virtual bool MSWShouldPreProcessMessage(WXMSG* pMsg); virtual WXDWORD MSWGetStyle(long style, WXDWORD *exstyle) const; diff --git a/src/msw/textctrl.cpp b/src/msw/textctrl.cpp index 90c96676b1..4359c26d0a 100644 --- a/src/msw/textctrl.cpp +++ b/src/msw/textctrl.cpp @@ -2058,14 +2058,27 @@ void wxTextCtrl::OnKeyDown(wxKeyEvent& event) event.Skip(); } -WXLRESULT wxTextCtrl::MSWWindowProc(WXUINT nMsg, WXWPARAM wParam, WXLPARAM lParam) +bool +wxTextCtrl::MSWHandleMessage(WXLRESULT *rc, + WXUINT nMsg, + WXWPARAM wParam, + WXLPARAM lParam) { - WXLRESULT lRc = wxTextCtrlBase::MSWWindowProc(nMsg, wParam, lParam); + bool processed = wxTextCtrlBase::MSWHandleMessage(rc, nMsg, wParam, lParam); switch ( nMsg ) { case WM_GETDLGCODE: { + // Ensure that the result value is initialized even if the base + // class didn't handle WM_GETDLGCODE but just update the value + // returned by it if it did handle it. + if ( !processed ) + { + *rc = MSWDefWindowProc(nMsg, wParam, lParam); + processed = true; + } + // we always want the chars and the arrows: the arrows for // navigation and the chars because we want Ctrl-C to work even // in a read only control @@ -2089,7 +2102,7 @@ WXLRESULT wxTextCtrl::MSWWindowProc(WXUINT nMsg, WXWPARAM wParam, WXLPARAM lPara if ( HasFlag(wxTE_PROCESS_TAB) ) lDlgCode |= DLGC_WANTTAB; - lRc |= lDlgCode; + *rc |= lDlgCode; } else // !editable { @@ -2098,11 +2111,11 @@ WXLRESULT wxTextCtrl::MSWWindowProc(WXUINT nMsg, WXWPARAM wParam, WXLPARAM lPara // including DLGC_WANTMESSAGE). This is strange (how // does it work in the native Win32 apps?) but for now // live with it. - lRc = lDlgCode; + *rc = lDlgCode; } if (IsMultiLine()) // Clear the DLGC_HASSETSEL bit from the return value - lRc &= ~DLGC_HASSETSEL; + *rc &= ~DLGC_HASSETSEL; } break; @@ -2122,7 +2135,7 @@ WXLRESULT wxTextCtrl::MSWWindowProc(WXUINT nMsg, WXWPARAM wParam, WXLPARAM lPara #endif // wxUSE_MENUS } - return lRc; + return processed; } // ---------------------------------------------------------------------------- From d3e7833a909f32ffe55e8ab1e872822905562f38 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 19 Nov 2015 00:25:26 +0100 Subject: [PATCH 04/13] No changes, just replace a useless comment with a more helpful one Don't describe in English what the next line of code does, which is useless, but rather explain why do we do this. --- src/msw/textctrl.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/msw/textctrl.cpp b/src/msw/textctrl.cpp index 4359c26d0a..64bda4e190 100644 --- a/src/msw/textctrl.cpp +++ b/src/msw/textctrl.cpp @@ -2113,9 +2113,15 @@ wxTextCtrl::MSWHandleMessage(WXLRESULT *rc, // live with it. *rc = lDlgCode; } - if (IsMultiLine()) - // Clear the DLGC_HASSETSEL bit from the return value + + if ( IsMultiLine() ) + { + // The presence of this style, coming from the default EDIT + // WndProc, indicates that the control contents should be + // selected when it gets focus, but we don't want this to + // happen for the multiline controls, so clear it. *rc &= ~DLGC_HASSETSEL; + } } break; From 99a1526ee326ca463339e0f6f92db6df8604dc1a Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 19 Nov 2015 01:03:56 +0100 Subject: [PATCH 05/13] Factor out methods for clicking the default button in wxMSW This will make it possible to reproduce the default "Enter" key functionality from elsewhere. Almost no changes yet, the only minor change is that we now wouldn't try to "click" any windows using DLGC_DEFPUSHBUTTON dialog code but which are not really buttons -- but then this shouldn't ever happen anyhow. --- include/wx/msw/window.h | 12 +++++++++ src/msw/window.cpp | 56 ++++++++++++++++++++++++++++------------- 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/include/wx/msw/window.h b/include/wx/msw/window.h index fb44301155..9647e080b2 100644 --- a/include/wx/msw/window.h +++ b/include/wx/msw/window.h @@ -14,6 +14,8 @@ #include "wx/settings.h" // solely for wxSystemColour +class WXDLLIMPEXP_FWD_CORE wxButton; + // if this is set to 1, we use deferred window sizing to reduce flicker when // resizing complicated window hierarchies, but this can in theory result in // different behaviour than the old code so we keep the possibility to use it @@ -534,6 +536,16 @@ public: virtual wxMenu* MSWFindMenuFromHMENU(WXHMENU hMenu); #endif // wxUSE_MENUS && !__WXUNIVERSAL__ + // Return the default button for the TLW containing this window or NULL if + // none. + static wxButton* MSWGetDefaultButtonFor(wxWindow* win); + + // Simulate a click on the given button if it is non-null, enabled and + // shown. + // + // Return true if the button was clicked, false otherwise. + static bool MSWClickButtonIfPossible(wxButton* btn); + protected: // this allows you to implement standard control borders without // repeating the code in different classes that are not derived from diff --git a/src/msw/window.cpp b/src/msw/window.cpp index 8337da6091..01c9d59584 100644 --- a/src/msw/window.cpp +++ b/src/msw/window.cpp @@ -2316,7 +2316,7 @@ bool wxWindowMSW::MSWProcessMessage(WXMSG* pMsg) // currently active button should get enter press even // if there is a default button elsewhere so check if // this window is a button first - wxWindow *btn = NULL; + wxButton *btn = NULL; if ( lDlgCode & DLGC_DEFPUSHBUTTON ) { // let IsDialogMessage() handle this for all @@ -2325,8 +2325,11 @@ bool wxWindowMSW::MSWProcessMessage(WXMSG* pMsg) long style = ::GetWindowLong(msg->hwnd, GWL_STYLE); if ( (style & BS_OWNERDRAW) == BS_OWNERDRAW ) { - // emulate the button click - btn = wxFindWinFromHandle(msg->hwnd); + btn = wxDynamicCast + ( + wxFindWinFromHandle(msg->hwnd), + wxButton + ); } } else // not a button itself, do we have default button? @@ -2367,25 +2370,12 @@ bool wxWindowMSW::MSWProcessMessage(WXMSG* pMsg) ); } } - else // bCtrlDown - { - win = wxGetTopLevelParent(win); - } - wxTopLevelWindow * const - tlw = wxDynamicCast(win, wxTopLevelWindow); - if ( tlw ) - { - btn = wxDynamicCast(tlw->GetDefaultItem(), - wxButton); - } + btn = MSWGetDefaultButtonFor(win); } - if ( btn && btn->IsEnabled() && btn->IsShownOnScreen() ) - { - btn->MSWCommand(BN_CLICKED, 0 /* unused */); + if ( MSWClickButtonIfPossible(btn) ) return true; - } // This "Return" key press won't be actually used for // navigation so don't generate wxNavigationKeyEvent @@ -2544,6 +2534,36 @@ bool wxWindowMSW::MSWSafeIsDialogMessage(WXMSG* msg) #endif // __WXUNIVERSAL__ +/* static */ +wxButton* wxWindowMSW::MSWGetDefaultButtonFor(wxWindow* win) +{ +#if wxUSE_BUTTON + win = wxGetTopLevelParent(win); + + wxTopLevelWindow *const tlw = wxDynamicCast(win, wxTopLevelWindow); + if ( tlw ) + return wxDynamicCast(tlw->GetDefaultItem(), wxButton); +#endif // wxUSE_BUTTON + + return NULL; +} + +/* static */ +bool wxWindowMSW::MSWClickButtonIfPossible(wxButton* btn) +{ +#if wxUSE_BUTTON + if ( btn && btn->IsEnabled() && btn->IsShownOnScreen() ) + { + btn->MSWCommand(BN_CLICKED, 0 /* unused */); + return true; + } +#endif // wxUSE_BUTTON + + wxUnusedVar(btn); + + return false; +} + // --------------------------------------------------------------------------- // message params unpackers // --------------------------------------------------------------------------- From 4edae7238a324398ff0ba2ca9b441360715458d0 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 19 Nov 2015 01:15:07 +0100 Subject: [PATCH 06/13] Fix skipping the event in wxEVT_TEXT_ENTER handler in wxMSW Skipping the event is supposed to have the same effect as not handling the event at all, but in wxMSW wxTE_PROCESS_ENTER style must be specified for a wxEVT_TEXT_ENTER handler to be executed at all and if this style is used, then the default handling in MSWProcessMessage() which normally happens before calling the handler doesn't take place at all. Work around this by explicitly performing the default "Enter" key action if the event generated by it wasn't handled to make wxMSW behaviour more intuitive. --- samples/dialogs/dialogs.cpp | 9 ++++++++- src/msw/textctrl.cpp | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/samples/dialogs/dialogs.cpp b/samples/dialogs/dialogs.cpp index 8e5e133c0f..23e7e5c4d9 100644 --- a/samples/dialogs/dialogs.cpp +++ b/samples/dialogs/dialogs.cpp @@ -2603,7 +2603,14 @@ void TestDefaultActionDialog::OnCatchListBoxDClick(wxCommandEvent& WXUNUSED(even void TestDefaultActionDialog::OnTextEnter(wxCommandEvent& event) { - wxLogMessage("Text \"%s\" entered.", event.GetString()); + const wxString& text = event.GetString(); + if ( text.empty() ) + { + event.Skip(); + return; + } + + wxLogMessage("Text \"%s\" entered.", text); } void MyFrame::OnTestDefaultActionDialog(wxCommandEvent& WXUNUSED(event)) diff --git a/src/msw/textctrl.cpp b/src/msw/textctrl.cpp index 64bda4e190..9b4124e9da 100644 --- a/src/msw/textctrl.cpp +++ b/src/msw/textctrl.cpp @@ -2066,6 +2066,27 @@ wxTextCtrl::MSWHandleMessage(WXLRESULT *rc, { bool processed = wxTextCtrlBase::MSWHandleMessage(rc, nMsg, wParam, lParam); + // Handle the special case of "Enter" key: the user code needs to specify + // wxTE_PROCESS_ENTER style to get it in the first place, but if this flag + // is used, then even if the wxEVT_TEXT_ENTER handler skips the event, the + // normal action of this key is not performed because IsDialogMessage() is + // not called and, also, an annoying beep is generated by EDIT default + // WndProc. + // + // Fix these problems by explicitly performing the default function of this + // key (which would be done by MSWProcessMessage() if we didn't have + // wxTE_PROCESS_ENTER) and preventing the default WndProc from getting it. + if ( nMsg == WM_CHAR && + !processed && + HasFlag(wxTE_PROCESS_ENTER) && + wParam == VK_RETURN && + !wxIsAnyModifierDown() ) + { + MSWClickButtonIfPossible(MSWGetDefaultButtonFor(this)); + + processed = true; + } + switch ( nMsg ) { case WM_GETDLGCODE: From f9d907a1d401a5ffb62e3ce070aa4808111a9f1e Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 25 Nov 2015 21:54:54 +0100 Subject: [PATCH 07/13] Just simplify wxPoint/wxSize creation from wxRect Use the existing wxRect::Get{Position,Size}() methods instead of explicitly creating the objects from the wxRect components, this is simpler and more readable. No real changes. --- src/generic/datavgen.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index 3cad87b018..a8408c641c 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -120,8 +120,8 @@ wxDataViewColumn* GetExpanderColumnOrFirstOne(wxDataViewCtrl* dataview) wxTextCtrl *CreateEditorTextCtrl(wxWindow *parent, const wxRect& labelRect, const wxString& value) { wxTextCtrl* ctrl = new wxTextCtrl(parent, wxID_ANY, value, - wxPoint(labelRect.x,labelRect.y), - wxSize(labelRect.width,labelRect.height), + labelRect.GetPosition(), + labelRect.GetSize(), wxTE_PROCESS_ENTER); // Adjust size of wxTextCtrl editor to fit text, even if it means being From 40502651f9e349818dad513951c29b371963f479 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 26 Nov 2015 00:56:10 +0100 Subject: [PATCH 08/13] Always delete the editor when finishing editing in wxDataViewCtrl We need to delete the editor control even if GetValueFromEditorCtrl() failed as otherwise we're never going to be able to edit another item again, at least with the GTK native implementation, which does nothing when starting to edit an item if an editor control already exists. --- src/common/datavcmn.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/common/datavcmn.cpp b/src/common/datavcmn.cpp index 3f4f8299af..de6f21f9ea 100644 --- a/src/common/datavcmn.cpp +++ b/src/common/datavcmn.cpp @@ -745,9 +745,10 @@ bool wxDataViewRendererBase::FinishEditing() if (!m_editorCtrl) return true; + // Try to get the value, normally we should succeed but if we fail, don't + // return immediately, we still need to destroy the edit control. wxVariant value; - if ( !GetValueFromEditorCtrl(m_editorCtrl, value) ) - return false; + const bool gotValue = GetValueFromEditorCtrl(m_editorCtrl, value); wxDataViewCtrl* dv_ctrl = GetOwner()->GetOwner(); @@ -755,6 +756,9 @@ bool wxDataViewRendererBase::FinishEditing() dv_ctrl->GetMainWindow()->SetFocus(); + if ( !gotValue ) + return false; + bool isValid = Validate(value); unsigned int col = GetOwner()->GetModelColumn(); From 235e8ebd1a28655232d0c38f0f664e4c8de4016c Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 26 Nov 2015 01:18:22 +0100 Subject: [PATCH 09/13] Factor out wxDataViewRendererBase::NotifyEditingStarted() Reuse the same code from the generic and native GTK and OS X implementations of wxDataViewCtrl instead of triplicating it. This fixes a small discrepancy between the wxOSX version, which didn't see the model pointer correctly in the generated event, and all the others, but mainly paves way for future improvements. --- include/wx/dvrenderers.h | 3 +++ src/common/datavcmn.cpp | 15 +++++++++++---- src/gtk/dataview.cpp | 6 +----- src/osx/cocoa/dataview.mm | 22 ++++++++++------------ 4 files changed, 25 insertions(+), 21 deletions(-) diff --git a/include/wx/dvrenderers.h b/include/wx/dvrenderers.h index ca14b16a42..a9c5156186 100644 --- a/include/wx/dvrenderers.h +++ b/include/wx/dvrenderers.h @@ -180,6 +180,9 @@ public: // wxDVR_DEFAULT_ALIGNMENT. int GetEffectiveAlignment() const; + // Send wxEVT_DATAVIEW_ITEM_EDITING_STARTED event. + void NotifyEditingStarted(const wxDataViewItem& item); + protected: // These methods are called from PrepareForItem() and should do whatever is // needed for the current platform to ensure that the item is rendered diff --git a/src/common/datavcmn.cpp b/src/common/datavcmn.cpp index de6f21f9ea..d7c7b04bb3 100644 --- a/src/common/datavcmn.cpp +++ b/src/common/datavcmn.cpp @@ -703,15 +703,22 @@ bool wxDataViewRendererBase::StartEditing( const wxDataViewItem &item, wxRect la m_editorCtrl->SetFocus(); #endif - // Now we should send Editing Started event + NotifyEditingStarted(item); + + return true; +} + +void wxDataViewRendererBase::NotifyEditingStarted(const wxDataViewItem& item) +{ + wxDataViewColumn* const column = GetOwner(); + wxDataViewCtrl* const dv_ctrl = column->GetOwner(); + wxDataViewEvent event( wxEVT_DATAVIEW_ITEM_EDITING_STARTED, dv_ctrl->GetId() ); - event.SetDataViewColumn( GetOwner() ); + event.SetDataViewColumn( column ); event.SetModel( dv_ctrl->GetModel() ); event.SetItem( item ); event.SetEventObject( dv_ctrl ); dv_ctrl->GetEventHandler()->ProcessEvent( event ); - - return true; } void wxDataViewRendererBase::DestroyEditControl() diff --git a/src/gtk/dataview.cpp b/src/gtk/dataview.cpp index 3455a205b0..0e93eeda0a 100644 --- a/src/gtk/dataview.cpp +++ b/src/gtk/dataview.cpp @@ -1767,12 +1767,8 @@ wxgtk_renderer_editing_started( GtkCellRenderer *WXUNUSED(cell), GtkCellEditable wxDataViewColumn *column = wxrenderer->GetOwner(); wxDataViewCtrl *dv = column->GetOwner(); - wxDataViewEvent event( wxEVT_DATAVIEW_ITEM_EDITING_STARTED, dv->GetId() ); - event.SetDataViewColumn( column ); - event.SetModel( dv->GetModel() ); wxDataViewItem item(dv->GTKPathToItem(wxGtkTreePath(path))); - event.SetItem( item ); - dv->HandleWindowEvent( event ); + wxrenderer->NotifyEditingStarted(item); if (GTK_IS_CELL_EDITABLE(editable)) { diff --git a/src/osx/cocoa/dataview.mm b/src/osx/cocoa/dataview.mm index 259e9a62a5..0690d4456f 100644 --- a/src/osx/cocoa/dataview.mm +++ b/src/osx/cocoa/dataview.mm @@ -1886,22 +1886,20 @@ outlineView:(NSOutlineView*)outlineView wxDataViewColumn* const col([static_cast(tableColumn) getColumnPointer]); - wxDataViewCtrl* const dvc = implementation->GetDataViewCtrl(); - - // stop editing of a custom item first (if necessary) dvc->FinishCustomItemEditing(); // now, send the event: - wxDataViewEvent - event(wxEVT_DATAVIEW_ITEM_EDITING_STARTED,dvc->GetId()); - - event.SetEventObject(dvc); - event.SetItem( - wxDataViewItemFromItem([self itemAtRow:currentlyEditedRow])); - event.SetColumn(dvc->GetColumnPosition(col)); - event.SetDataViewColumn(col); - dvc->GetEventHandler()->ProcessEvent(event); + wxDataViewRenderer* const renderer = col->GetRenderer(); + if ( renderer ) + { + renderer->NotifyEditingStarted + ( + wxDataViewItemFromItem([self itemAtRow:currentlyEditedRow]) + ); + } + //else: we should always have a renderer but don't crash if for some + // unfathomable reason we don't have it } -(void) textDidEndEditing:(NSNotification*)notification From 29024e39ca5e353c335170d36f6f9c3b309e4c50 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 26 Nov 2015 01:20:37 +0100 Subject: [PATCH 10/13] Fix duplicate wxEVT_DATAVIEW_ITEM_EDITING_STARTED under GTK When using a custom renderer, wxEVT_DATAVIEW_ITEM_EDITING_STARTED was sent twice: once from the generic base class StartEditing() and another time from the GTK-specific "editing_started" signal handler. And we must send it from the latter, because otherwise no event would be generated at all for the standard renderers (i.e. text cells) for which we don't call StartEditing() ourselves, so don't call it from the former and instead generate the event by explicitly calling NotifyEditingStarted() after calling StartEditing() in the generic version (as for wxOSX version, it doesn't use StartEditing() at all and so doesn't need to be changed). --- src/common/datavcmn.cpp | 2 -- src/generic/datavgen.cpp | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/datavcmn.cpp b/src/common/datavcmn.cpp index d7c7b04bb3..ce8c0530ff 100644 --- a/src/common/datavcmn.cpp +++ b/src/common/datavcmn.cpp @@ -703,8 +703,6 @@ bool wxDataViewRendererBase::StartEditing( const wxDataViewItem &item, wxRect la m_editorCtrl->SetFocus(); #endif - NotifyEditingStarted(item); - return true; } diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index a8408c641c..d116af19fb 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -2251,6 +2251,8 @@ wxDataViewMainWindow::StartEditing(const wxDataViewItem& item, const wxRect itemRect = GetItemRect(item, col); if ( renderer->StartEditing(item, itemRect) ) { + renderer->NotifyEditingStarted(item); + // Save the renderer to be able to finish/cancel editing it later and // save the control to be able to detect if we're still editing it. m_editorRenderer = renderer; From 7e37c6763f8b4f6057fa16e672cc6f3ac88c7a77 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 27 Nov 2015 15:43:21 +0100 Subject: [PATCH 11/13] Test custom editor in the dataview sample Show how to create and use a custom editor in the custom renderer. --- samples/dataview/dataview.cpp | 42 ++++++++++++++++++++++++++++++----- samples/dataview/mymodels.cpp | 13 +++++++++-- samples/dataview/mymodels.h | 5 +++++ 3 files changed, 52 insertions(+), 8 deletions(-) diff --git a/samples/dataview/dataview.cpp b/samples/dataview/dataview.cpp index 92d57e45db..70a543f6a9 100644 --- a/samples/dataview/dataview.cpp +++ b/samples/dataview/dataview.cpp @@ -173,10 +173,12 @@ private: class MyCustomRenderer: public wxDataViewCustomRenderer { public: - MyCustomRenderer() - : wxDataViewCustomRenderer("string", - wxDATAVIEW_CELL_ACTIVATABLE, - wxALIGN_CENTER) + // This renderer can be either activatable or editable, for demonstration + // purposes. In real programs, you should select whether the user should be + // able to activate or edit the cell and it doesn't make sense to switch + // between the two -- but this is just an example, so it doesn't stop us. + explicit MyCustomRenderer(wxDataViewCellMode mode) + : wxDataViewCustomRenderer("string", mode, wxALIGN_CENTER) { } virtual bool Render( wxRect rect, wxDC *dc, int state ) wxOVERRIDE @@ -223,6 +225,34 @@ public: virtual bool GetValue( wxVariant &WXUNUSED(value) ) const wxOVERRIDE { return true; } + virtual bool HasEditorCtrl() const wxOVERRIDE { return true; } + + virtual wxWindow* + CreateEditorCtrl(wxWindow* parent, + wxRect labelRect, + const wxVariant& value) wxOVERRIDE + { + wxTextCtrl* text = new wxTextCtrl(parent, wxID_ANY, value, + labelRect.GetPosition(), + labelRect.GetSize(), + wxTE_PROCESS_ENTER); + text->SetInsertionPointEnd(); + + return text; + } + + virtual bool + GetValueFromEditorCtrl(wxWindow* ctrl, wxVariant& value) wxOVERRIDE + { + wxTextCtrl* text = wxDynamicCast(ctrl, wxTextCtrl); + if ( !text ) + return false; + + value = text->GetValue(); + + return true; + } + private: wxString m_value; }; @@ -614,7 +644,7 @@ void MyFrame::BuildDataViewCtrl(wxPanel* parent, unsigned int nPanel, unsigned l // column 5 of the view control: - MyCustomRenderer *cr = new MyCustomRenderer; + MyCustomRenderer *cr = new MyCustomRenderer(wxDATAVIEW_CELL_ACTIVATABLE); wxDataViewColumn *column5 = new wxDataViewColumn( "custom", cr, 5, -1, wxALIGN_LEFT, wxDATAVIEW_COL_RESIZABLE ); @@ -662,7 +692,7 @@ void MyFrame::BuildDataViewCtrl(wxPanel* parent, unsigned int nPanel, unsigned l m_ctrl[1]->AppendColumn( new wxDataViewColumn("custom renderer", - new MyCustomRenderer, + new MyCustomRenderer(wxDATAVIEW_CELL_EDITABLE), MyListModel::Col_Custom) ); } diff --git a/samples/dataview/mymodels.cpp b/samples/dataview/mymodels.cpp index c18f9000b9..590d3f2e67 100644 --- a/samples/dataview/mymodels.cpp +++ b/samples/dataview/mymodels.cpp @@ -444,7 +444,13 @@ void MyListModel::GetValueByRow( wxVariant &variant, break; case Col_Custom: - variant = wxString::Format("%d", row % 100); + { + IntToStringMap::const_iterator it = m_customColValues.find(row); + if ( it != m_customColValues.end() ) + variant = it->second; + else + variant = wxString::Format("%d", row % 100); + } break; case Col_Max: @@ -531,10 +537,13 @@ bool MyListModel::SetValueByRow( const wxVariant &variant, case Col_Date: case Col_TextWithAttr: - case Col_Custom: wxLogError("Cannot edit the column %d", col); break; + case Col_Custom: + m_customColValues[row] = variant.GetString(); + break; + case Col_Max: wxFAIL_MSG( "invalid column" ); } diff --git a/samples/dataview/mymodels.h b/samples/dataview/mymodels.h index 96921c1724..4c7d1a3a10 100644 --- a/samples/dataview/mymodels.h +++ b/samples/dataview/mymodels.h @@ -8,6 +8,10 @@ // Licence: wxWindows licence ///////////////////////////////////////////////////////////////////////////// +#include "wx/hashmap.h" + +WX_DECLARE_HASH_MAP(unsigned, wxString, wxIntegerHash, wxIntegerEqual, + IntToStringMap); // ---------------------------------------------------------------------------- // MyMusicTreeModelNode: a node inside MyMusicTreeModel @@ -235,6 +239,7 @@ public: private: wxArrayString m_textColValues; wxArrayString m_iconColValues; + IntToStringMap m_customColValues; wxIcon m_icon[2]; }; From 31a10d0355bbeb3398d777a5dafc7c40a31076d0 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 30 Nov 2015 03:59:05 +0100 Subject: [PATCH 12/13] Fix editing items with custom renderers in wxGTK wxDataViewCtrl Call FinishEditing() when the cell is done being edited instead of sending wxEVT_DATAVIEW_ITEM_EDITING_DONE event manually from wxGTK code. This fixes the bug with not being able to edit an item the second time if the editor was dismissed by GTK+ itself and not by our own code (which already did call FinishEditing()) as the old editor still remained alive because DestroyEditControl(), usually called from FinishEditing(), wasn't executed. It also removes code duplication and avoids the need to keep a global s_user_data pointer as the item currently being edited is already stored in wxDataViewRenderer anyhow. --- src/gtk/dataview.cpp | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/gtk/dataview.cpp b/src/gtk/dataview.cpp index 0e93eeda0a..1a309ba5f4 100644 --- a/src/gtk/dataview.cpp +++ b/src/gtk/dataview.cpp @@ -1742,20 +1742,11 @@ bool wxGtkDataViewModelNotifier::Cleared() // wxDataViewRenderer // --------------------------------------------------------- -static gpointer s_user_data = NULL; - static void wxgtk_cell_editable_editing_done( GtkCellEditable *WXUNUSED(editable), wxDataViewRenderer *wxrenderer ) { - wxDataViewColumn *column = wxrenderer->GetOwner(); - wxDataViewCtrl *dv = column->GetOwner(); - wxDataViewEvent event( wxEVT_DATAVIEW_ITEM_EDITING_DONE, dv->GetId() ); - event.SetDataViewColumn( column ); - event.SetModel( dv->GetModel() ); - wxDataViewItem item( s_user_data ); - event.SetItem( item ); - dv->HandleWindowEvent( event ); + wxrenderer->FinishEditing(); } static void @@ -1772,8 +1763,6 @@ wxgtk_renderer_editing_started( GtkCellRenderer *WXUNUSED(cell), GtkCellEditable if (GTK_IS_CELL_EDITABLE(editable)) { - s_user_data = item.GetID(); - g_signal_connect (editable, "editing_done", G_CALLBACK (wxgtk_cell_editable_editing_done), (gpointer) wxrenderer ); From e417913f4676c8b4ae2104e23cdbdd2c945ee588 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 30 Nov 2015 04:03:18 +0100 Subject: [PATCH 13/13] Reset the wxDataViewItem being edited once it's not edited any more No real changes, just some cleanup to ensure that the item being edited stored in wxDataViewRendererBase never refers to an item which is not, actually, being edited any longer. --- include/wx/dvrenderers.h | 2 +- src/common/datavcmn.cpp | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/include/wx/dvrenderers.h b/include/wx/dvrenderers.h index a9c5156186..759461cddd 100644 --- a/include/wx/dvrenderers.h +++ b/include/wx/dvrenderers.h @@ -202,7 +202,7 @@ protected: wxString m_variantType; wxDataViewColumn *m_owner; wxWeakRef m_editorCtrl; - wxDataViewItem m_item; // for m_editorCtrl + wxDataViewItem m_item; // Item being currently edited, if valid. // internal utility, may be used anywhere the window associated with the // renderer is required diff --git a/src/common/datavcmn.cpp b/src/common/datavcmn.cpp index ce8c0530ff..6775d5fc53 100644 --- a/src/common/datavcmn.cpp +++ b/src/common/datavcmn.cpp @@ -681,8 +681,6 @@ bool wxDataViewRendererBase::StartEditing( const wxDataViewItem &item, wxRect la if( !start_event.IsAllowed() ) return false; - m_item = item; // remember for later - unsigned int col = GetOwner()->GetModelColumn(); const wxVariant& value = CheckedGetValue(dv_ctrl->GetModel(), item, col); @@ -708,6 +706,9 @@ bool wxDataViewRendererBase::StartEditing( const wxDataViewItem &item, wxRect la void wxDataViewRendererBase::NotifyEditingStarted(const wxDataViewItem& item) { + // Remember the item being edited for use in FinishEditing() later. + m_item = item; + wxDataViewColumn* const column = GetOwner(); wxDataViewCtrl* const dv_ctrl = column->GetOwner(); @@ -778,13 +779,16 @@ bool wxDataViewRendererBase::FinishEditing() event.SetEventObject( dv_ctrl ); dv_ctrl->GetEventHandler()->ProcessEvent( event ); + bool accepted = false; if ( isValid && event.IsAllowed() ) { dv_ctrl->GetModel()->ChangeValue(value, m_item, col); - return true; + accepted = true; } - return false; + m_item = wxDataViewItem(); + + return accepted; } wxVariant