Another try at getting the Link combobox working on MSW.

3d-viewer/3d_viewer/3d_toolbar.cpp

@@ -60,7 +60,7 @@ void EDA_3D_VIEWER_FRAME::ReCreateMainToolbar()
     for( std::pair<const wxString, VIEWPORT3D>& pair : m_viewports )
         m_cbViewports->Append( pair.first, static_cast<void*>( &pair.second ) );
 
-    m_cbViewports->Append( wxT( "-----" ) );
+    m_cbViewports->Append( wxT( "---" ) );
     m_cbViewports->Append( _( "Save viewport..." ) );
     m_cbViewports->Append( _( "Delete viewport..." ) );
 

common/CMakeLists.txt

@@ -200,6 +200,7 @@ set( COMMON_WIDGET_SRCS
     widgets/wx_aui_art_providers.cpp
     widgets/wx_aui_utils.cpp
     widgets/wx_busy_indicator.cpp
+    widgets/wx_combobox.cpp
     widgets/wx_ellipsized_static_text.cpp
     widgets/wx_grid.cpp
     widgets/wx_listbox.cpp

common/widgets/wx_combobox.cpp

@@ -0,0 +1,185 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2022 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation, either version 3 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <functional>
+#include <widgets/wx_combobox.h>
+#include <wx/dc.h>
+#include <wx/pen.h>
+#include <wx/settings.h>
+
+
+#define SEPARATOR wxT( "---" )
+
+
+WX_COMBOBOX::WX_COMBOBOX( wxWindow* aParent, int aId, const wxString& aValue, const wxPoint& aPos,
+                          const wxSize& aSize, int n, const wxString choices[], long style ) :
+        wxOwnerDrawnComboBox( aParent, aId, aValue, aPos, aSize, n, choices, style ),
+        m_lastSelection( 0 )
+{
+}
+
+
+WX_COMBOBOX::~WX_COMBOBOX()
+{
+}
+
+
+void WX_COMBOBOX::DoSetPopupControl( wxComboPopup* aPopup )
+{
+    using namespace std::placeholders;
+    wxOwnerDrawnComboBox::DoSetPopupControl( aPopup );
+
+    // Bind events to intercept selections, so the separator can be made nonselectable.
+
+    GetVListBoxComboPopup()->Bind( wxEVT_MOTION, &WX_COMBOBOX::TryVetoMouse, this );
+    GetVListBoxComboPopup()->Bind( wxEVT_LEFT_DOWN, &WX_COMBOBOX::TryVetoMouse, this );
+    GetVListBoxComboPopup()->Bind( wxEVT_LEFT_UP, &WX_COMBOBOX::TryVetoMouse, this );
+    GetVListBoxComboPopup()->Bind( wxEVT_LEFT_DCLICK, &WX_COMBOBOX::TryVetoMouse, this );
+    GetVListBoxComboPopup()->Bind( wxEVT_LISTBOX, std::bind( &WX_COMBOBOX::TryVetoSelect,
+                                                             this, _1, true ) );
+
+    Bind( wxEVT_COMBOBOX, std::bind( &WX_COMBOBOX::TryVetoSelect, this, _1, false ) );
+}
+
+
+void WX_COMBOBOX::OnDrawItem( wxDC& aDC, wxRect const& aRect, int aItem, int aFlags ) const
+{
+    wxString text = GetMenuText( aItem );
+
+    if( text == SEPARATOR )
+    {
+        wxPen pen( wxSystemSettings::GetColour( wxSYS_COLOUR_GRAYTEXT ), 1 );
+
+        aDC.SetPen( pen );
+        aDC.DrawLine( aRect.x, aRect.y + aRect.height / 2, aRect.x + aRect.width,
+                      aRect.y + aRect.height / 2 );
+    }
+    else
+    {
+        wxCoord x, y;
+
+        if( aFlags & wxODCB_PAINTING_CONTROL )
+        {
+            x = aRect.x + GetMargins().x;
+            y = ( aRect.height - aDC.GetCharHeight() ) / 2 + aRect.y;
+        }
+        else
+        {
+            x = aRect.x + 2;
+            y = aRect.y;
+        }
+
+        aDC.DrawText( text, x, y );
+    }
+}
+
+
+int WX_COMBOBOX::GetCharHeight() const
+{
+    return wxOwnerDrawnComboBox::GetCharHeight() + 2;
+}
+
+
+wxCoord WX_COMBOBOX::OnMeasureItem( size_t aItem ) const
+{
+    if( GetMenuText( aItem ) == SEPARATOR )
+        return 11;
+    else
+        return wxOwnerDrawnComboBox::OnMeasureItem( aItem );
+}
+
+
+wxCoord WX_COMBOBOX::OnMeasureItemWidth( size_t aItem ) const
+{
+    if( GetMenuText( aItem ) == SEPARATOR )
+        return GetTextRect().GetWidth() - 2;
+    else
+        return wxOwnerDrawnComboBox::OnMeasureItemWidth( aItem );
+}
+
+
+void WX_COMBOBOX::TryVetoMouse( wxMouseEvent& aEvent )
+{
+    int item = GetVListBoxComboPopup()->VirtualHitTest( aEvent.GetPosition().y );
+
+    if( GetMenuText( item ) != SEPARATOR )
+        aEvent.Skip();
+}
+
+
+void WX_COMBOBOX::TryVetoSelect( wxCommandEvent& aEvent, bool aInner )
+{
+    int sel = GetSelectionEither( aInner );
+
+    if( sel >= 0 && sel < (int) GetCount() )
+    {
+        wxString text = GetMenuText( sel );
+
+        if( text == SEPARATOR )
+        {
+            SetSelectionEither( aInner, m_lastSelection );
+        }
+        else
+        {
+            m_lastSelection = sel;
+            aEvent.Skip();
+        }
+    }
+}
+
+
+void WX_COMBOBOX::Append( const wxString& aText, const wxString& aMenuText )
+{
+    if( !aMenuText.IsEmpty() )
+        m_menuText[ GetCount() ] = aMenuText;
+
+    wxOwnerDrawnComboBox::Append( aText );
+}
+
+
+wxString WX_COMBOBOX::GetMenuText( int aItem ) const
+{
+    if( m_menuText.count( aItem ) )
+        return m_menuText.at( aItem );
+    else if( aItem >= 0 && aItem < (int) GetCount() )
+        return GetVListBoxComboPopup()->GetString( aItem );
+    else
+        return wxEmptyString;
+}
+
+
+int WX_COMBOBOX::GetSelectionEither( bool aInner ) const
+{
+    if( aInner )
+        return GetVListBoxComboPopup()->wxVListBox::GetSelection();
+    else
+        return GetSelection();
+}
+
+
+void WX_COMBOBOX::SetSelectionEither( bool aInner, int aSel )
+{
+    if( aSel >= 0 && aSel < (int) GetCount() )
+    {
+        if( aInner )
+            return GetVListBoxComboPopup()->wxVListBox::SetSelection( aSel );
+        else
+            return SetSelection( aSel );
+    }
+}

eeschema/dialogs/dialog_text_properties.cpp

@@ -27,6 +27,7 @@
 #include <widgets/bitmap_button.h>
 #include <widgets/font_choice.h>
 #include <widgets/color_swatch.h>
+#include <widgets/wx_combobox.h>
 #include <base_units.h>
 #include <settings/color_settings.h>
 #include <tool/tool_manager.h>
@@ -134,14 +135,15 @@ DIALOG_TEXT_PROPERTIES::DIALOG_TEXT_PROPERTIES( SCH_EDIT_FRAME* aParent, SCH_ITE
         wxString sheetPageNum = sheet.GetPageNumber();
         wxString sheetName = sheet.size() == 1 ? _( "<root sheet>" ) : sheet.Last()->GetName();
 
-        m_hyperlinkCombo->Append( wxString::Format( _( "Page %s (%s)" ), sheetPageNum, sheetName ) );
+        m_hyperlinkCombo->Append( wxT( "#" ) + sheetPageNum,
+                                  wxString::Format( _( "Page %s (%s)" ), sheetPageNum, sheetName ) );
         m_pageNumbers.push_back( sheetPageNum );
     }
 
     m_hyperlinkCombo->Append( wxT( "---" ) );
-    m_hyperlinkCombo->Append( wxT( "file://..." ), KiBitmap( BITMAPS::small_folder ) );
-    m_hyperlinkCombo->Append( wxT( "http://..." ), KiBitmap( BITMAPS::www ) );
-    m_hyperlinkCombo->Append( wxT( "https://..." ), KiBitmap( BITMAPS::www ) );
+    m_hyperlinkCombo->Append( wxT( "file://" ), wxT( "file://..." ) );
+    m_hyperlinkCombo->Append( wxT( "http://" ), wxT( "http://..." ) );
+    m_hyperlinkCombo->Append( wxT( "https://" ), wxT( "https://..." ) );
 
     SetupStandardButtons();
     Layout();
@@ -305,42 +307,11 @@ void DIALOG_TEXT_PROPERTIES::onHyperlinkText( wxCommandEvent& event )
 
 void DIALOG_TEXT_PROPERTIES::onHyperlinkCombo( wxCommandEvent& aEvent )
 {
-    size_t sel = aEvent.GetSelection();
-
-    if( sel < 0 )
-    {
-        // user clicked outside dropdown; leave current value
-    }
-    else if( sel == m_hyperlinkCombo->GetCount() - 4 )
-    {
-        // separator
-    }
-    else if( sel == m_hyperlinkCombo->GetCount() - 3 )
-    {
-        static wxString helper = wxT( "file://" );
-
-        m_hyperlinkCombo->ChangeValue( helper );
-        m_hyperlinkCombo->SetInsertionPointEnd();
-    }
-    else if( sel == m_hyperlinkCombo->GetCount() - 2 )
+    if( aEvent.GetSelection() >= 0 )
     {
-        static wxString helper = wxT( "http://" );
-
-        m_hyperlinkCombo->ChangeValue( helper );
-        m_hyperlinkCombo->SetInsertionPointEnd();
-    }
-    else if( sel == m_hyperlinkCombo->GetCount() - 1 )
-    {
-        static wxString helper = wxT( "https://" );
-
-        m_hyperlinkCombo->ChangeValue( helper );
+        m_hyperlinkCb->SetValue( true );
         m_hyperlinkCombo->SetInsertionPointEnd();
     }
-    else
-    {
-        m_hyperlinkCombo->ChangeValue( wxT( "#" ) + m_pageNumbers[ sel ] );
-        m_hyperlinkCombo->SelectAll();
-    }
 }
 
 

eeschema/dialogs/dialog_text_properties_base.cpp

@@ -9,6 +9,7 @@
 #include "widgets/color_swatch.h"
 #include "widgets/font_choice.h"
 #include "widgets/infobar.h"
+#include "widgets/wx_combobox.h"
 
 #include "dialog_text_properties_base.h"
 
@@ -273,7 +274,7 @@ DIALOG_TEXT_PROPERTIES_BASE::DIALOG_TEXT_PROPERTIES_BASE( wxWindow* parent, wxWi
 	wxBoxSizer* bSizer11;
 	bSizer11 = new wxBoxSizer( wxHORIZONTAL );
 
-	m_hyperlinkCombo = new wxBitmapComboBox( this, wxID_ANY, wxEmptyString, wxDefaultPosition, wxDefaultSize, 0, NULL, 0 );
+	m_hyperlinkCombo = new WX_COMBOBOX( this, wxID_ANY, wxEmptyString, wxDefaultPosition, wxDefaultSize, 0, NULL, 0 );
 	bSizer11->Add( m_hyperlinkCombo, 1, wxALIGN_CENTER_VERTICAL, 5 );
 
 

eeschema/dialogs/dialog_text_properties_base.fbp

@@ -2695,7 +2695,7 @@
                                         <property name="show">1</property>
                                         <property name="size"></property>
                                         <property name="style"></property>
-                                        <property name="subclass">; ; forward_declare</property>
+                                        <property name="subclass">WX_COMBOBOX; widgets/wx_combobox.h; forward_declare</property>
                                         <property name="toolbar_pane">0</property>
                                         <property name="tooltip"></property>
                                         <property name="validator_data_type"></property>

eeschema/dialogs/dialog_text_properties_base.h

@@ -13,6 +13,7 @@
 class BITMAP_BUTTON;
 class COLOR_SWATCH;
 class FONT_CHOICE;
+class WX_COMBOBOX;
 class WX_INFOBAR;
 
 #include "dialog_shim.h"
@@ -88,7 +89,7 @@ class DIALOG_TEXT_PROPERTIES_BASE : public DIALOG_SHIM
 		wxPanel* m_panelFillColor;
 		COLOR_SWATCH* m_fillColorSwatch;
 		wxCheckBox* m_hyperlinkCb;
-		wxBitmapComboBox* m_hyperlinkCombo;
+		WX_COMBOBOX* m_hyperlinkCombo;
 		wxStaticLine* m_staticline;
 		wxStdDialogButtonSizer* m_sdbSizer1;
 		wxButton* m_sdbSizer1OK;

include/widgets/wx_combobox.h

@@ -0,0 +1,83 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2022 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation, either version 3 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef WX_COMBOBOX_H
+#define WX_COMBOBOX_H
+
+#include <map>
+#include <wx/odcombo.h>
+
+/**
+ * Fix some issues with wxCombobox:
+ *  - setting the value of a non-read-only combobox doesn't work on MSW
+ *  - rollover highlighting in the dropdown doesn't work on OSX
+ *  - separators don't work anywhere
+ */
+class WX_COMBOBOX : public wxOwnerDrawnComboBox
+{
+public:
+    WX_COMBOBOX( wxWindow* aParent, int aId = wxID_ANY, const wxString& aValue = wxEmptyString,
+                 const wxPoint& aPos = wxDefaultPosition, const wxSize& aSize = wxDefaultSize,
+                 int n = 0, const wxString choices[] = nullptr, long style = 0 );
+
+    virtual ~WX_COMBOBOX();
+
+    void Append( const wxString& aText, const wxString& aMenuText = wxEmptyString );
+
+    int GetCharHeight() const override;
+
+protected:
+    virtual void DoSetPopupControl( wxComboPopup* aPopup ) override;
+    virtual void OnDrawItem( wxDC& aDC, const wxRect& aRect, int aItem, int aFlags ) const override;
+    virtual wxCoord OnMeasureItem( size_t aItem ) const override;
+    virtual wxCoord OnMeasureItemWidth( size_t aItem ) const override;
+
+    /// Veto a mouseover event if in the separator
+    void TryVetoMouse( wxMouseEvent& aEvent );
+
+    /**
+     * Veto a select event for the separator
+     *
+     * @param aEvent - the wxCommandEvent caller
+     * @param aInner - true if event was called for the inner list (ie the popup)
+     */
+    void TryVetoSelect( wxCommandEvent& aEvent, bool aInner );
+
+    /**
+     * Safely get a string for an item, returning wxEmptyString if the item doesn't exist.
+     */
+    wxString GetMenuText( int aItem ) const;
+
+    /**
+     * Get selection from either the outer (combo box) or inner (popup) list.
+     */
+    int GetSelectionEither( bool aInner ) const;
+
+    /**
+     * Safely set selection for either the outer (combo box) or inner (popup) list, doing nothing
+     * for invalid selections.
+     */
+    void SetSelectionEither( bool aInner, int aSel );
+
+private:
+    std::map<int, wxString> m_menuText;
+    int                     m_lastSelection;
+};
+
+#endif // WX_COMBOBOX_H

pcbnew/widgets/appearance_controls.cpp

@@ -2438,7 +2438,7 @@ void APPEARANCE_CONTROLS::rebuildLayerPresetsWidget()
         idx++;
     }
 
-    m_cbLayerPresets->Append( wxT( "-----" ) );
+    m_cbLayerPresets->Append( wxT( "---" ) );
     m_cbLayerPresets->Append( _( "Save preset..." ) );
     m_cbLayerPresets->Append( _( "Delete preset..." ) );
 
@@ -2677,7 +2677,7 @@ void APPEARANCE_CONTROLS::rebuildViewportsWidget()
     for( std::pair<const wxString, VIEWPORT>& pair : m_viewports )
         m_cbViewports->Append( pair.first, static_cast<void*>( &pair.second ) );
 
-    m_cbViewports->Append( wxT( "-----" ) );
+    m_cbViewports->Append( wxT( "---" ) );
     m_cbViewports->Append( _( "Save viewport..." ) );
     m_cbViewports->Append( _( "Delete viewport..." ) );