From 15ec266ae3ca232d967b1216f8173888a4996c62 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 7 Oct 2012 22:40:29 +0000 Subject: [PATCH] Fix spurious label editing in generic wx{List,Tree,DataView}Ctrl. Clicking on the control to give it focus must not start editing the label of an item in it, this is bad UI as you need to carefully select where do you click to avoid starting to edit the label and nobody else does it like this (probably because of the former reason). As a side note, it would be really great to abstract the item handling in a class that could be reused by all these controls instead of having to update 3 slightly different versions of the same code every time. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@72634 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- src/generic/datavgen.cpp | 9 ++++++++- src/generic/listctrl.cpp | 15 ++++++++++++--- src/generic/treectlg.cpp | 9 ++++++++- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index c4a11e828c..18602bcbe2 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -4289,8 +4289,15 @@ void wxDataViewMainWindow::OnMouse( wxMouseEvent &event ) m_currentCol = col; m_currentColSetByKeyboard = false; + // This flag is used to decide whether we should start editing the item + // label. We do it if the user clicks twice (but not double clicks, + // i.e. simulateClick is false) on the same item but not if the click + // was used for something else already, e.g. selecting the item (so it + // must have been already selected) or giving the focus to the control + // (so it must have had focus already). m_lastOnSame = !simulateClick && ((col == oldCurrentCol) && - (current == oldCurrentRow)) && oldWasSelected; + (current == oldCurrentRow)) && oldWasSelected && + HasFocus(); // Call ActivateCell() after everything else as under GTK+ if ( IsCellEditableInMode(item, col, wxDATAVIEW_CELL_ACTIVATABLE) ) diff --git a/src/generic/listctrl.cpp b/src/generic/listctrl.cpp index a5aee6314b..26a34afdad 100644 --- a/src/generic/listctrl.cpp +++ b/src/generic/listctrl.cpp @@ -2470,9 +2470,10 @@ void wxListMainWindow::OnMouse( wxMouseEvent &event ) m_renameTimer->Start(dclick > 0 ? dclick : 250, true); } } + + m_lastOnSame = false; } - m_lastOnSame = false; m_lineSelectSingleOnUp = (size_t)-1; } else @@ -2564,8 +2565,16 @@ void wxListMainWindow::OnMouse( wxMouseEvent &event ) if (m_current != oldCurrent) RefreshLine( oldCurrent ); - // forceClick is only set if the previous click was on another item - m_lastOnSame = !forceClick && (m_current == oldCurrent) && oldWasSelected; + // Set the flag telling us whether the next click on this item should + // start editing its label. This should happen if we clicked on the + // current item and it was already selected, i.e. if this click was not + // done to select it. + // + // It should not happen if this was a double click (forceClick is true) + // nor if we hadn't had the focus before as then this click was used to + // give focus to the control. + m_lastOnSame = (m_current == oldCurrent) && oldWasSelected && + !forceClick && HasFocus(); } } diff --git a/src/generic/treectlg.cpp b/src/generic/treectlg.cpp index 33b3eaeca3..4c631b27b3 100644 --- a/src/generic/treectlg.cpp +++ b/src/generic/treectlg.cpp @@ -3779,7 +3779,14 @@ void wxGenericTreeCtrl::OnMouse( wxMouseEvent &event ) // ==> LeftDown() || LeftDClick() if ( event.LeftDown() ) { - m_lastOnSame = item == m_current; + // If we click on an already selected item but do it to return + // the focus to the control, it shouldn't start editing the + // item label because it's too easy to start editing + // accidentally (and also because nobody else does it like + // this). So only set this flag, used to decide whether we + // should start editing the label later, if we already have + // focus. + m_lastOnSame = item == m_current && HasFocus(); } if ( flags & wxTREE_HITTEST_ONITEMBUTTON )