Component chooser: event and focus cleanup

common/draw_panel_gal.cpp

@@ -2,6 +2,7 @@
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
  * Copyright (C) 2013-2017 CERN
+ * Copyright (C) 2013-2017 KiCad Developers, see AUTHORS.txt for contributors.
  * @author Tomasz Wlostowski <tomasz.wlostowski@cern.ch>
  * @author Maciej Suminski <maciej.suminski@cern.ch>
  *
@@ -57,6 +58,7 @@ EDA_DRAW_PANEL_GAL::EDA_DRAW_PANEL_GAL( wxWindow* aParentWindow, wxWindowID aWin
     m_painter    = NULL;
     m_eventDispatcher = NULL;
     m_lostFocus  = false;
+    m_stealsFocus = false;
 
     SetLayoutDirection( wxLayout_LeftToRight );
 
@@ -397,7 +399,7 @@ bool EDA_DRAW_PANEL_GAL::SwitchBackend( GAL_TYPE aGalType )
 
 void EDA_DRAW_PANEL_GAL::onEvent( wxEvent& aEvent )
 {
-    if( m_lostFocus )
+    if( m_lostFocus && m_stealsFocus )
         SetFocus();
 
     if( !m_eventDispatcher )
@@ -412,7 +414,8 @@ void EDA_DRAW_PANEL_GAL::onEvent( wxEvent& aEvent )
 void EDA_DRAW_PANEL_GAL::onEnter( wxEvent& aEvent )
 {
     // Getting focus is necessary in order to receive key events properly
-    SetFocus();
+    if( m_stealsFocus )
+        SetFocus();
 
     aEvent.Skip();
 }

eeschema/component_tree_search_container.h

@@ -139,12 +139,13 @@ public:
      */
     void UpdateSearchTerm( const wxString& aSearch );
 
-    /** Function GetSelectedAlias
+    /**
+     * Get the currently selected alias.
      *
-     * @param aUnit : if not NULL, the selected sub-unit is set here.
+     * @param aUnit : if not null, the selected sub-unit is set here.
-     * @return the selected alias or NULL if there is none, or there is no tree.
+     * @return the selected alias or nullptr if there is none, or there is no tree.
      */
-    LIB_ALIAS* GetSelectedAlias( int* aUnit );
+    LIB_ALIAS* GetSelectedAlias( int* aUnit = nullptr );
 
     /**
      * Function GetComponentsCount

eeschema/dialogs/dialog_choose_component.cpp

@@ -125,7 +125,7 @@ void DIALOG_CHOOSE_COMPONENT::OnSearchBoxChange( wxCommandEvent& aEvent )
 
 void DIALOG_CHOOSE_COMPONENT::OnSearchBoxEnter( wxCommandEvent& aEvent )
 {
-    EndModal( wxID_OK );   // We are done.
+    HandleItemSelection();
 }
 
 
@@ -138,14 +138,9 @@ void DIALOG_CHOOSE_COMPONENT::selectIfValid( const wxTreeListItem& aTreeId )
 }
 
 
-void DIALOG_CHOOSE_COMPONENT::OnInterceptSearchBoxKey( wxKeyEvent& aKeyStroke )
+void DIALOG_CHOOSE_COMPONENT::OnSearchBoxKey( wxKeyEvent& aKeyStroke )
 {
-    // Cursor up/down and partiallyi cursor are use to do tree navigation operations.
+    auto const sel = m_libraryComponentTree->GetSelection();
-    // This is done by intercepting some navigational keystrokes that normally would go to
-    // the text search box (which has the focus by default). That way, we are mostly keyboard
-    // operable.
-    // (If the tree has the focus, it can handle that by itself).
-    const wxTreeListItem sel = m_libraryComponentTree->GetSelection();
 
     switch( aKeyStroke.GetKeyCode() )
     {
@@ -157,22 +152,6 @@ void DIALOG_CHOOSE_COMPONENT::OnInterceptSearchBoxKey( wxKeyEvent& aKeyStroke )
         selectIfValid( GetNextItem( *m_libraryComponentTree, sel ) );
         break;
 
-        // The following keys we can only hijack if they are not needed by the textbox itself.
-
-    case WXK_LEFT:
-        if( m_searchBox->GetInsertionPoint() == 0 )
-            m_libraryComponentTree->Collapse( sel );
-        else
-            aKeyStroke.Skip();   // Use for original purpose: move cursor.
-        break;
-
-    case WXK_RIGHT:
-        if( m_searchBox->GetInsertionPoint() >= (long) m_searchBox->GetLineText( 0 ).length() )
-            m_libraryComponentTree->Expand( sel );
-        else
-            aKeyStroke.Skip();   // Use for original purpose: move cursor.
-        break;
-
     default:
         aKeyStroke.Skip();  // Any other key: pass on to search box directly.
         break;
@@ -186,24 +165,23 @@ void DIALOG_CHOOSE_COMPONENT::OnTreeSelect( wxTreeListEvent& aEvent )
 }
 
 
-void DIALOG_CHOOSE_COMPONENT::OnDoubleClickTreeActivation( wxTreeListEvent& aEvent )
+void DIALOG_CHOOSE_COMPONENT::OnTreeActivate( wxTreeListEvent& aEvent )
 {
-    if( updateSelection() )
+    updateSelection();
+    HandleItemSelection();
+}
+
+
+void DIALOG_CHOOSE_COMPONENT::OnTreeKeyUp( wxKeyEvent& aEvent )
+{
+    if( aEvent.GetKeyCode() == WXK_RETURN )
     {
-        // Ok, got selection. We don't just end the modal dialog here, but
+        updateSelection();
-        // wait for the MouseUp event to occur. Otherwise something (broken?)
+        HandleItemSelection();
-        // happens: the dialog will close and will deliver the 'MouseUp' event
+    }
-        // to the eeschema canvas, that will immediately place the component.
+    else
-        //
+    {
-        // NOW, here's where it gets really fun. wxTreeListCtrl eats MouseUp.
+        aEvent.Skip();
-        // This isn't really feasible to bypass without a fully custom
-        // wxDataViewCtrl implementation, and even then might not be fully
-        // possible (docs are vague). To get around this, we use a one-shot
-        // timer to schedule the dialog close.
-        //
-        // See DIALOG_CHOOSE_COMPONENT::OnCloseTimer for the other end of this
-        // spaghetti noodle.
-        m_dbl_click_timer->StartOnce( DIALOG_CHOOSE_COMPONENT::DblClickDelay );
     }
 }
 
@@ -229,32 +207,10 @@ void DIALOG_CHOOSE_COMPONENT::OnCloseTimer( wxTimerEvent& aEvent )
 }
 
 
-// Test strategy to see if OnInterceptTreeEnter() works:
-//  - search for an item.
-//  - click into the tree once to set focus on tree; navigate. Press 'Enter'
-//  -> The dialog should close and the component be available to place.
-void DIALOG_CHOOSE_COMPONENT::OnInterceptTreeEnter( wxKeyEvent& aEvent )
-{
-    // We have to do some special handling for double-click on a tree-item because
-    // of some superfluous event delivery bug in wxWidgets (see OnDoubleClickTreeActivation()).
-    // In tree-activation, we assume we got a double-click and need to take special precaution
-    // that the mouse-up event is not delivered to the window one level up by going through
-    // a state-sequence OnDoubleClickTreeActivation() -> OnTreeMouseUp().
-
-    // Pressing 'Enter' within a tree will also call OnDoubleClickTreeActivation(),
-    // but since this is not due to the double-click and we have no way of knowing that it is
-    // not, we need to intercept the 'Enter' key before that to know that it is time to exit.
-    if( aEvent.GetKeyCode() == WXK_RETURN )
-        EndModal( wxID_OK );    // Dialog is done.
-    else
-        aEvent.Skip();          // Let tree handle that key for navigation.
-}
-
-
 void DIALOG_CHOOSE_COMPONENT::OnStartComponentBrowser( wxMouseEvent& aEvent )
 {
     m_external_browser_requested = true;
-    EndModal( wxID_OK );   // We are done.
+    EndModal( wxID_OK );
 }
 
 
@@ -398,6 +354,36 @@ void DIALOG_CHOOSE_COMPONENT::renderPreview( LIB_PART* aComponent, int aUnit )
 }
 
 
+void DIALOG_CHOOSE_COMPONENT::HandleItemSelection()
+{
+    if( m_search_container->GetSelectedAlias() )
+    {
+        // Got a selection. We can't just end the modal dialog here, because
+        // wx leaks some events back to the parent window (in particular, the
+        // MouseUp following a double click).
+        //
+        // NOW, here's where it gets really fun. wxTreeListCtrl eats MouseUp.
+        // This isn't really feasible to bypass without a fully custom
+        // wxDataViewCtrl implementation, and even then might not be fully
+        // possible (docs are vague). To get around this, we use a one-shot
+        // timer to schedule the dialog close.
+        //
+        // See DIALOG_CHOOSE_COMPONENT::OnCloseTimer for the other end of this
+        // spaghetti noodle.
+        m_dbl_click_timer->StartOnce( DIALOG_CHOOSE_COMPONENT::DblClickDelay );
+    }
+    else
+    {
+        auto const sel = m_libraryComponentTree->GetSelection();
+
+        if( m_libraryComponentTree->IsExpanded( sel ) )
+            m_libraryComponentTree->Collapse( sel );
+        else
+            m_libraryComponentTree->Expand( sel );
+    }
+}
+
+
 static wxTreeListItem GetPrevItem( const wxTreeListCtrl& tree, const wxTreeListItem& item )
 {
     wxTreeListItem prevItem = GetPrevSibling( tree, item );

eeschema/dialogs/dialog_choose_component.h

@@ -76,11 +76,11 @@ public:
 protected:
     virtual void OnSearchBoxChange( wxCommandEvent& aEvent ) override;
     virtual void OnSearchBoxEnter( wxCommandEvent& aEvent ) override;
-    virtual void OnInterceptSearchBoxKey( wxKeyEvent& aEvent ) override;
+    virtual void OnSearchBoxKey( wxKeyEvent& aEvent ) override;
 
     virtual void OnTreeSelect( wxTreeListEvent& aEvent ) override;
-    virtual void OnDoubleClickTreeActivation( wxTreeListEvent& aEvent ) override;
+    virtual void OnTreeActivate( wxTreeListEvent& aEvent ) override;
-    virtual void OnInterceptTreeEnter( wxKeyEvent& aEvent ) override;
+    virtual void OnTreeKeyUp( wxKeyEvent& aEvent ) override;
 
     virtual void OnStartComponentBrowser( wxMouseEvent& aEvent ) override;
     virtual void OnHandlePreviewRepaint( wxPaintEvent& aRepaintEvent ) override;
@@ -90,10 +90,17 @@ protected:
 
 private:
     bool updateSelection();
-    void selectIfValid( const wxTreeListItem& aTreeId );
-    void renderPreview( LIB_PART*      aComponent, int aUnit );
-
     void updateFootprint();
+    void selectIfValid( const wxTreeListItem& aTreeId );
+    void renderPreview( LIB_PART* aComponent, int aUnit );
+
+    /**
+     * Handle the selection of an item. This is called when either the search
+     * box or the tree receive an Enter, or the tree receives a double click.
+     * If the item selected is a category, it is expanded or collapsed; if it
+     * is a component, the component is picked.
+     */
+    void HandleItemSelection();
 
     std::unique_ptr<wxTimer> m_dbl_click_timer;
     FOOTPRINT_PREVIEW_PANEL* m_footprintPreviewPanel;

eeschema/dialogs/dialog_choose_component_base.cpp

@@ -108,11 +108,11 @@ DIALOG_CHOOSE_COMPONENT_BASE::DIALOG_CHOOSE_COMPONENT_BASE( wxWindow* parent, wx
 	// Connect Events
 	this->Connect( wxEVT_IDLE, wxIdleEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnIdle ) );
 	this->Connect( wxEVT_INIT_DIALOG, wxInitDialogEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnInitDialog ) );
-	m_searchBox->Connect( wxEVT_KEY_UP, wxKeyEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnInterceptSearchBoxKey ), NULL, this );
+	m_searchBox->Connect( wxEVT_KEY_UP, wxKeyEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnSearchBoxKey ), NULL, this );
 	m_searchBox->Connect( wxEVT_COMMAND_TEXT_UPDATED, wxCommandEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnSearchBoxChange ), NULL, this );
 	m_searchBox->Connect( wxEVT_COMMAND_TEXT_ENTER, wxCommandEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnSearchBoxEnter ), NULL, this );
-	m_libraryComponentTree->Connect( wxEVT_KEY_UP, wxKeyEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnInterceptTreeEnter ), NULL, this );
+	m_libraryComponentTree->Connect( wxEVT_KEY_UP, wxKeyEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnTreeKeyUp ), NULL, this );
-	m_libraryComponentTree->Connect( wxEVT_TREELIST_ITEM_ACTIVATED, wxTreeListEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnDoubleClickTreeActivation ), NULL, this );
+	m_libraryComponentTree->Connect( wxEVT_TREELIST_ITEM_ACTIVATED, wxTreeListEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnTreeActivate ), NULL, this );
 	m_libraryComponentTree->Connect( wxEVT_TREELIST_SELECTION_CHANGED, wxTreeListEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnTreeSelect ), NULL, this );
 	m_componentDetails->Connect( wxEVT_COMMAND_HTML_LINK_CLICKED, wxHtmlLinkEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnDatasheetClick ), NULL, this );
 	m_componentView->Connect( wxEVT_LEFT_DCLICK, wxMouseEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnStartComponentBrowser ), NULL, this );
@@ -124,11 +124,11 @@ DIALOG_CHOOSE_COMPONENT_BASE::~DIALOG_CHOOSE_COMPONENT_BASE()
 	// Disconnect Events
 	this->Disconnect( wxEVT_IDLE, wxIdleEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnIdle ) );
 	this->Disconnect( wxEVT_INIT_DIALOG, wxInitDialogEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnInitDialog ) );
-	m_searchBox->Disconnect( wxEVT_KEY_UP, wxKeyEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnInterceptSearchBoxKey ), NULL, this );
+	m_searchBox->Disconnect( wxEVT_KEY_UP, wxKeyEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnSearchBoxKey ), NULL, this );
 	m_searchBox->Disconnect( wxEVT_COMMAND_TEXT_UPDATED, wxCommandEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnSearchBoxChange ), NULL, this );
 	m_searchBox->Disconnect( wxEVT_COMMAND_TEXT_ENTER, wxCommandEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnSearchBoxEnter ), NULL, this );
-	m_libraryComponentTree->Disconnect( wxEVT_KEY_UP, wxKeyEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnInterceptTreeEnter ), NULL, this );
+	m_libraryComponentTree->Disconnect( wxEVT_KEY_UP, wxKeyEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnTreeKeyUp ), NULL, this );
-	m_libraryComponentTree->Disconnect( wxEVT_TREELIST_ITEM_ACTIVATED, wxTreeListEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnDoubleClickTreeActivation ), NULL, this );
+	m_libraryComponentTree->Disconnect( wxEVT_TREELIST_ITEM_ACTIVATED, wxTreeListEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnTreeActivate ), NULL, this );
 	m_libraryComponentTree->Disconnect( wxEVT_TREELIST_SELECTION_CHANGED, wxTreeListEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnTreeSelect ), NULL, this );
 	m_componentDetails->Disconnect( wxEVT_COMMAND_HTML_LINK_CLICKED, wxHtmlLinkEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnDatasheetClick ), NULL, this );
 	m_componentView->Disconnect( wxEVT_LEFT_DCLICK, wxMouseEventHandler( DIALOG_CHOOSE_COMPONENT_BASE::OnStartComponentBrowser ), NULL, this );

eeschema/dialogs/dialog_choose_component_base.fbp

@@ -417,7 +417,7 @@
                                                     <event name="OnEnterWindow"></event>
                                                     <event name="OnEraseBackground"></event>
                                                     <event name="OnKeyDown"></event>
-                                                    <event name="OnKeyUp">OnInterceptSearchBoxKey</event>
+                                                    <event name="OnKeyUp">OnSearchBoxKey</event>
                                                     <event name="OnKillFocus"></event>
                                                     <event name="OnLeaveWindow"></event>
                                                     <event name="OnLeftDClick"></event>
@@ -504,7 +504,7 @@
                                             <event name="OnEnterWindow"></event>
                                             <event name="OnEraseBackground"></event>
                                             <event name="OnKeyDown"></event>
-                                            <event name="OnKeyUp">OnInterceptTreeEnter</event>
+                                            <event name="OnKeyUp">OnTreeKeyUp</event>
                                             <event name="OnKillFocus"></event>
                                             <event name="OnLeaveWindow"></event>
                                             <event name="OnLeftDClick"></event>
@@ -523,7 +523,7 @@
                                             <event name="OnSetFocus"></event>
                                             <event name="OnSize"></event>
                                             <event name="OnTreelistColumnSorted"></event>
-                                            <event name="OnTreelistItemActivated">OnDoubleClickTreeActivation</event>
+                                            <event name="OnTreelistItemActivated">OnTreeActivate</event>
                                             <event name="OnTreelistItemChecked"></event>
                                             <event name="OnTreelistItemContextMenu"></event>
                                             <event name="OnTreelistItemExpanded"></event>

eeschema/dialogs/dialog_choose_component_base.h

@@ -62,11 +62,11 @@ class DIALOG_CHOOSE_COMPONENT_BASE : public DIALOG_SHIM
 		// Virtual event handlers, overide them in your derived class
 		virtual void OnIdle( wxIdleEvent& event ) { event.Skip(); }
 		virtual void OnInitDialog( wxInitDialogEvent& event ) { event.Skip(); }
-		virtual void OnInterceptSearchBoxKey( wxKeyEvent& event ) { event.Skip(); }
+		virtual void OnSearchBoxKey( wxKeyEvent& event ) { event.Skip(); }
 		virtual void OnSearchBoxChange( wxCommandEvent& event ) { event.Skip(); }
 		virtual void OnSearchBoxEnter( wxCommandEvent& event ) { event.Skip(); }
-		virtual void OnInterceptTreeEnter( wxKeyEvent& event ) { event.Skip(); }
+		virtual void OnTreeKeyUp( wxKeyEvent& event ) { event.Skip(); }
-		virtual void OnDoubleClickTreeActivation( wxTreeListEvent& event ) { event.Skip(); }
+		virtual void OnTreeActivate( wxTreeListEvent& event ) { event.Skip(); }
 		virtual void OnTreeSelect( wxTreeListEvent& event ) { event.Skip(); }
 		virtual void OnDatasheetClick( wxHtmlLinkEvent& event ) { event.Skip(); }
 		virtual void OnStartComponentBrowser( wxMouseEvent& event ) { event.Skip(); }

include/class_draw_panel_gal.h

@@ -183,6 +183,24 @@ public:
      */
     virtual void OnShow() {}
 
+    /**
+     * Set whether focus is taken on certain events (mouseover, keys, etc). This should
+     * be true (and is by default) for any primary canvas, but can be false to make
+     * well-behaved preview panes and the like.
+     */
+    void SetStealsFocus( bool aStealsFocus )
+    {
+        m_stealsFocus = aStealsFocus;
+    }
+
+    /**
+     * Get whether focus is taken on certain events (see SetStealsFocus()).
+     */
+    bool GetStealsFocus() const
+    {
+        return m_stealsFocus;
+    }
+
 protected:
     void onPaint( wxPaintEvent& WXUNUSED( aEvent ) );
     void onSize( wxSizeEvent& aEvent );
@@ -240,6 +258,10 @@ protected:
     /// Flag to indicate that focus should be regained on the next mouse event. It is a workaround
     /// for cases when the panel loses keyboard focus, so it does not react to hotkeys anymore.
     bool                     m_lostFocus;
+
+    /// Flag to indicate whether the panel should take focus at certain times (when moused over,
+    /// and on various mouse/key events)
+    bool                     m_stealsFocus;
 };
 
 #endif

pcbnew/footprint_preview_panel.cpp

@@ -162,6 +162,7 @@ FOOTPRINT_PREVIEW_PANEL::FOOTPRINT_PREVIEW_PANEL(
     m_loader = std::make_unique<LOADER_THREAD>( this );
     m_loader->Run();
 
+    SetStealsFocus( false );
     ShowScrollbars( wxSHOW_SB_NEVER, wxSHOW_SB_NEVER );
     EnableScrolling( false, false );    // otherwise Zoom Auto disables GAL canvas