← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Add double-click handling to disambiguation menu

 

Hi Jeff-

Here is the patch from the original e-mail.

Best-
Seth

On Fri, Dec 15, 2017 at 3:35 AM, Jeff Young <jeff@xxxxxxxxx> wrote:

> Where can I find the patch (it wasn’t linked to either bug report
> mentioned)?
>
> I want to make sure it doesn’t break trackpad double-tapping.
>
> Speaking of which, tapping in general is broken in GAL (see
> https://bugs.launchpad.net/kicad/+bug/1737010).  I submitted a patch with
> that bug; any chance we can get it merged along with this?
>
> Thanks,
> Jeff
>
>
> On 15 Dec 2017, at 04:29, Seth Hillbrand <seth.hillbrand@xxxxxxxxx> wrote:
>
> Thanks Chris and Orson for testing.  As long as everyone is OK with the
> approach, I'm happy to build a second patch to extend this to the GAL
> canvas.  Does anyone have objections to how this functions in legacy for
> now?
>
> Best-
> Seth
>
> On Mon, Dec 11, 2017 at 5:02 AM, Maciej Sumiński <maciej.suminski@xxxxxxx>
> wrote:
>
>> I tested the patch on Linux and it works as advertised. Well done, we
>> need the same thing for GAL too.
>>
>> Cheers,
>> Orson
>>
>> On 12/06/2017 07:05 PM, Seth Hillbrand wrote:
>> > ​The attached patch fixes https://bugs.launchpad.net/kicad/+bug/1154020
>> >
>> > The current double-click handler in draw_panel​ gets overridden by the
>> > disambiguation menu, preventing double-clicks from being properly
>> handled.
>> > This patch introduces a delay between the mouse up and handling the
>> > single-click.  If the double-click arrives in this time, the
>> double-click
>> > is handled.  Otherwise, the normal single-click is processed.
>> >
>> > Of note, we only introduce this delay in cases where the disambiguation
>> > menu is needed.  Otherwise, single-clicks are immediately processed.
>> >
>> > Tested on MacOS and Linux.  The current delay is set to 250ms.  We might
>> > slow this down to 400-500ms but that felt a bit clunky to me.
>> >
>> > -Seth
>> >
>> >
>> >
>> > _______________________________________________
>> > Mailing list: https://launchpad.net/~kicad-developers
>> > Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> > Unsubscribe : https://launchpad.net/~kicad-developers
>> > More help   : https://help.launchpad.net/ListHelp
>> >
>>
>>
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>>
>>
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>
>
>
From f81476ff706626adf461fedfb348c1656a887eaa Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Tue, 5 Dec 2017 21:25:52 -0800
Subject: [PATCH] wx: Add double-click handling in disambiguation cases

Fixes: lp:1154020
* https://bugs.launchpad.net/kicad/+bug/1154020
---
 common/draw_panel.cpp            | 36 +++++++++++++++++++++++++++++++++++-
 eeschema/lib_collectors.cpp      | 12 ++++++++++++
 eeschema/lib_collectors.h        |  5 +++++
 eeschema/libedit_onleftclick.cpp |  2 +-
 eeschema/onleftclick.cpp         |  2 +-
 eeschema/sch_collectors.cpp      | 14 ++++++++++++++
 eeschema/sch_collectors.h        |  5 +++++
 include/class_drawpanel.h        | 16 ++++++++++++++++
 include/id.h                     |  2 ++
 9 files changed, 91 insertions(+), 3 deletions(-)

diff --git a/common/draw_panel.cpp b/common/draw_panel.cpp
index 716c1e0d2..bce9376fc 100644
--- a/common/draw_panel.cpp
+++ b/common/draw_panel.cpp
@@ -28,6 +28,7 @@
  */
 
 #include <fctsys.h>
+#include <wx/timer.h>
 #include <pgm_base.h>
 #include <kiface_i.h>
 #include <gr_basic.h>
@@ -87,6 +88,7 @@ BEGIN_EVENT_TABLE( EDA_DRAW_PANEL, wxScrolledWindow )
     EVT_ERASE_BACKGROUND( EDA_DRAW_PANEL::OnEraseBackground )
     EVT_SCROLLWIN( EDA_DRAW_PANEL::OnScroll )
     EVT_ACTIVATE( EDA_DRAW_PANEL::OnActivate )
+    EVT_TIMER( ID_MOUSE_DOUBLECLICK, EDA_DRAW_PANEL::OnTimer )
     EVT_MENU_RANGE( ID_PAN_UP, ID_PAN_RIGHT, EDA_DRAW_PANEL::OnPan )
 END_EVENT_TABLE()
 
@@ -155,6 +157,9 @@ EDA_DRAW_PANEL::EDA_DRAW_PANEL( EDA_DRAW_FRAME* parent, int id,
 
     m_cursorLevel = 0;
     m_PrintIsMirrored = false;
+
+    m_ClickTimer = (wxTimer*) NULL;
+    m_doubleClickInterval = 250;
 }
 
 
@@ -168,6 +173,8 @@ EDA_DRAW_PANEL::~EDA_DRAW_PANEL()
         cfg->Write( ENBL_ZOOM_NO_CENTER_KEY, m_enableZoomNoCenter );
         cfg->Write( ENBL_AUTO_PAN_KEY, m_enableAutoPan );
     }
+
+    wxDELETE( m_ClickTimer );
 }
 
 
@@ -404,6 +411,14 @@ void EDA_DRAW_PANEL::OnActivate( wxActivateEvent& event )
 }
 
 
+void EDA_DRAW_PANEL::OnTimer( wxTimerEvent& event )
+{
+    INSTALL_UNBUFFERED_DC( DC, this );
+    DC.SetBackground( *wxBLACK_BRUSH );
+    GetParent()->OnLeftClick( &DC, m_CursorClickPos );
+}
+
+
 void EDA_DRAW_PANEL::OnScroll( wxScrollWinEvent& event )
 {
     int id = event.GetEventType();
@@ -1145,6 +1160,11 @@ void EDA_DRAW_PANEL::OnMouseEvent( wxMouseEvent& event )
     // Calling Double Click and Click functions :
     if( localbutt == (int) ( GR_M_LEFT_DOWN | GR_M_DCLICK ) )
     {
+        if( m_ClickTimer )
+        {
+            m_ClickTimer->Stop();
+            wxDELETE( m_ClickTimer );
+        }
         GetParent()->OnLeftDClick( &DC, GetParent()->RefPos( true ) );
 
         // inhibit a response to the mouse left button release,
@@ -1162,7 +1182,21 @@ void EDA_DRAW_PANEL::OnMouseEvent( wxMouseEvent& event )
         m_ignoreNextLeftButtonRelease = false;
 
         if( screen->m_BlockLocate.GetState() == STATE_NO_BLOCK && !ignoreEvt )
-            GetParent()->OnLeftClick( &DC, GetParent()->RefPos( true ) );
+        {
+            EDA_ITEM* item = screen->GetCurItem();
+            m_CursorClickPos = GetParent()->RefPos( true );
+
+            // If we have an item already selected, or we are using a tool,
+            // we won't use the disambiguation menu so process the click immediately
+            if( ( item && item->GetFlags() ) || GetParent()->GetToolId() != ID_NO_TOOL_SELECTED )
+                GetParent()->OnLeftClick( &DC, m_CursorClickPos );
+            else
+            {
+                wxDELETE( m_ClickTimer );
+                m_ClickTimer = new wxTimer(this, ID_MOUSE_DOUBLECLICK);
+                m_ClickTimer->StartOnce( m_doubleClickInterval );
+            }
+        }
 
     }
     else if( !event.LeftIsDown() )
diff --git a/eeschema/lib_collectors.cpp b/eeschema/lib_collectors.cpp
index 88af9d8f7..2a7f1750a 100644
--- a/eeschema/lib_collectors.cpp
+++ b/eeschema/lib_collectors.cpp
@@ -92,6 +92,18 @@ const KICAD_T LIB_COLLECTOR::RotatableItems[] = {
 };
 
 
+const KICAD_T LIB_COLLECTOR::DoubleClickItems[] = {
+    LIB_PIN_T,
+    LIB_ARC_T,
+    LIB_CIRCLE_T,
+    LIB_RECTANGLE_T,
+    LIB_POLYLINE_T,
+    LIB_TEXT_T,
+    LIB_FIELD_T,
+    EOT
+};
+
+
 SEARCH_RESULT LIB_COLLECTOR::Inspect( EDA_ITEM* aItem, void* testData )
 {
     LIB_ITEM* item = (LIB_ITEM*) aItem;
diff --git a/eeschema/lib_collectors.h b/eeschema/lib_collectors.h
index 48e5099ca..4bea55f26 100644
--- a/eeschema/lib_collectors.h
+++ b/eeschema/lib_collectors.h
@@ -75,6 +75,11 @@ public:
     static const KICAD_T RotatableItems[];
 
     /**
+     * A scan list for all double-clickable library items.
+     */
+    static const KICAD_T DoubleClickItems[];
+
+    /**
      * A scan list for all schematic items except pins.
      */
     static const KICAD_T AllItemsButPins[];
diff --git a/eeschema/libedit_onleftclick.cpp b/eeschema/libedit_onleftclick.cpp
index b0f8c5730..5c8d5225f 100644
--- a/eeschema/libedit_onleftclick.cpp
+++ b/eeschema/libedit_onleftclick.cpp
@@ -156,7 +156,7 @@ void LIB_EDIT_FRAME::OnLeftDClick( wxDC* DC, const wxPoint& aPosition )
 
     if( !m_drawItem || !m_drawItem->InEditMode() )
     {   // We can locate an item
-        m_drawItem = LocateItemUsingCursor( aPosition );
+        m_drawItem = LocateItemUsingCursor( aPosition, LIB_COLLECTOR::DoubleClickItems );
 
         if( m_drawItem == NULL )
         {
diff --git a/eeschema/onleftclick.cpp b/eeschema/onleftclick.cpp
index 2cfc9e045..2a6d8574d 100644
--- a/eeschema/onleftclick.cpp
+++ b/eeschema/onleftclick.cpp
@@ -389,7 +389,7 @@ void SCH_EDIT_FRAME::OnLeftDClick( wxDC* aDC, const wxPoint& aPosition )
     case ID_NO_TOOL_SELECTED:
         if( ( item == NULL ) || ( item->GetFlags() == 0 ) )
         {
-            item = LocateAndShowItem( aPosition );
+            item = LocateAndShowItem( aPosition, SCH_COLLECTOR::DoubleClickItems );
         }
 
         if( ( item == NULL ) || ( item->GetFlags() != 0 ) )
diff --git a/eeschema/sch_collectors.cpp b/eeschema/sch_collectors.cpp
index efe859720..5a1cd2e23 100644
--- a/eeschema/sch_collectors.cpp
+++ b/eeschema/sch_collectors.cpp
@@ -210,6 +210,20 @@ const KICAD_T SCH_COLLECTOR::CopyableItems[] = {
 };
 
 
+const KICAD_T SCH_COLLECTOR::DoubleClickItems[] = {
+    SCH_TEXT_T,
+    SCH_LABEL_T,
+    SCH_GLOBAL_LABEL_T,
+    SCH_HIERARCHICAL_LABEL_T,
+    SCH_COMPONENT_T,
+    SCH_SHEET_T,
+    SCH_BITMAP_T,
+    SCH_FIELD_T,
+    SCH_MARKER_T,
+    EOT
+};
+
+
 SEARCH_RESULT SCH_COLLECTOR::Inspect( EDA_ITEM* aItem, void* aTestData )
 {
     if( aItem->Type() != LIB_PIN_T && !aItem->HitTest( m_RefPos ) )
diff --git a/eeschema/sch_collectors.h b/eeschema/sch_collectors.h
index fbef3f62b..ee57ceafe 100644
--- a/eeschema/sch_collectors.h
+++ b/eeschema/sch_collectors.h
@@ -118,6 +118,11 @@ public:
     static const KICAD_T CopyableItems[];
 
     /**
+     * A scan list for schematic items that react to a double-click.
+     */
+    static const KICAD_T DoubleClickItems[];
+
+    /**
      * Constructor SCH_COLLECTOR
      */
     SCH_COLLECTOR( const KICAD_T* aScanTypes = SCH_COLLECTOR::AllItems )
diff --git a/include/class_drawpanel.h b/include/class_drawpanel.h
index 5983a7665..a190c4d93 100644
--- a/include/class_drawpanel.h
+++ b/include/class_drawpanel.h
@@ -66,6 +66,9 @@ private:
     wxPoint m_PanStartCenter;               ///< Initial scroll center position when pan started
     wxPoint m_PanStartEventPosition;        ///< Initial position of mouse event when pan started
 
+    wxPoint m_CursorClickPos;               ///< Used for maintaining click position
+    wxTimer *m_ClickTimer;
+
     /// The drawing area used to redraw the screen which is usually the visible area
     /// of the drawing in internal units.
     EDA_RECT    m_ClipBox;
@@ -113,6 +116,8 @@ private:
     /// >= 0 (or >= n) if a block can start
     int     m_canStartBlock;
 
+    int     m_doubleClickInterval;
+
 public:
 
     EDA_DRAW_PANEL( EDA_DRAW_FRAME* parent, int id, const wxPoint& pos, const wxSize& size );
@@ -217,6 +222,17 @@ public:
     void OnActivate( wxActivateEvent& event );
 
     /**
+     * Function OnTimer
+     * handle timer events
+     * <p>
+     * The class will start a timer when a mouse-up event is handled.  If a
+     * double-click event is not handled inside of a specified interval,
+     * the timer event will fire, causing the single-click event to be handled.
+     * Otherwise, the system will process the double-click.
+     */
+    void OnTimer( wxTimerEvent& event );
+
+    /**
      * Function DoPrepareDC
      * sets up the device context \a aDC for drawing.
      * <p>
diff --git a/include/id.h b/include/id.h
index 64411163b..5b03a9ae6 100644
--- a/include/id.h
+++ b/include/id.h
@@ -248,6 +248,8 @@ enum main_id
     ID_PAN_LEFT,
     ID_PAN_RIGHT,
 
+    ID_MOUSE_DOUBLECLICK,
+
     ID_GET_NETLIST,
     ID_OPEN_CMP_TABLE,
     ID_GET_TOOLS,
-- 
2.11.0


Follow ups

References