Empty QskLabelData is interpreted as separator now. Not sure how much

of an improvement this is as it adds the separators to the list of
options. But at least this allows to implement a wrapper like
QskMenuButton by copying options only.
Definitely not the final word on this API.

examples/gallery/main.cpp

@@ -187,7 +187,7 @@ namespace
 
             // see https://github.com/uwerat/qskinny/issues/192
             connect( menu, &QskMenu::triggered,
-                []( int index ) { if ( index == 3 ) qApp->quit(); } );
+                []( int index ) { if ( index == 4 ) qApp->quit(); } );
         }
     };
 
@@ -224,7 +224,7 @@ namespace
                 drawer->setEdge( Qt::RightEdge );
 
                 auto burger = new QskPushButton( "≡", this );
-		        burger->setEmphasis( QskPushButton::LowEmphasis );
+                burger->setEmphasis( QskPushButton::LowEmphasis );
 
                 connect( burger, &QskPushButton::clicked,
                     drawer, &QskPopup::open );

skins/squiek/QskSquiekSkin.cpp

@@ -428,6 +428,7 @@ void Editor::setupMenu()
 
     setMetric( Q::Separator | A::Size, 2_dp );
     setSeparator( Q::Separator );
+    setMargin( Q::Separator, 2 );
 
     setPadding( Q::Segment, QskMargins( 2, 10, 2, 10 ) );
     setSpacing( Q::Segment, 5 );

src/controls/QskMenu.cpp

@@ -28,14 +28,24 @@ QSK_SUBCONTROL( QskMenu, Separator )
 
 QSK_SYSTEM_STATE( QskMenu, Selected, QskAspect::FirstSystemState << 2 )
 
+static inline int qskActionIndex( const QVector< int >& actions, int index )
+{
+    if ( index < 0 )
+        return -1;
+
+    auto it = std::lower_bound( actions.constBegin(), actions.constEnd(), index );
+    return it - actions.constBegin();
+}
+
 class QskMenu::PrivateData
 {
   public:
     QPointF origin;
 
     QVector< QskLabelData > options;
-    // QVector< bool > enabled;
+
     QVector< int > separators;
+    QVector< int > actions;
 
     int triggeredIndex = -1;
     int currentIndex = -1;
@@ -126,15 +136,22 @@ int QskMenu::addOption( const QUrl& graphicSource, const QString& text )
 
 int QskMenu::addOption( const QskLabelData& option )
 {
+    const int index = m_data->options.count();
+
     m_data->options += option;
 
+    if ( option.isEmpty() )
+        m_data->separators += index;
+    else
+        m_data->actions += index;
+
     resetImplicitSize();
     update();
 
     if ( isComponentComplete() )
         Q_EMIT optionsChanged();
 
-    return count() - 1;
+    return index;
 }
 
 void QskMenu::setOptions( const QStringList& options )
@@ -146,6 +163,14 @@ void QskMenu::setOptions( const QVector< QskLabelData >& options )
 {
     m_data->options = options;
 
+    for ( int i = 0; i < options.count(); i++ )
+    {
+        if ( options[i].isEmpty() )
+            m_data->separators += i;
+        else
+            m_data->actions += i;
+    }
+
     if ( m_data->currentIndex >= 0 )
     {
         m_data->currentIndex = -1;
@@ -163,7 +188,6 @@ void QskMenu::setOptions( const QVector< QskLabelData >& options )
 
 void QskMenu::clear()
 {
-    m_data->separators.clear();
     setOptions( QVector< QskLabelData >() );
 }
 
@@ -177,27 +201,19 @@ QskLabelData QskMenu::optionAt( int index ) const
     return m_data->options.value( index );
 }
 
-int QskMenu::count() const
-{
-    return m_data->options.count();
-}
-
 void QskMenu::addSeparator()
 {
-    m_data->separators += m_data->options.count();
+    addOption( QskLabelData() );
-
-    resetImplicitSize();
-    update();
 }
 
-int QskMenu::separatorPosition( int index ) const
+QVector< int > QskMenu::separators() const
 {
-    return m_data->separators.value( index, -1 );
+    return m_data->separators;
 }
 
-int QskMenu::separatorCount() const
+QVector< int > QskMenu::actions() const
 {
-    return m_data->separators.count();
+    return m_data->actions;
 }
 
 int QskMenu::currentIndex() const
@@ -207,8 +223,15 @@ int QskMenu::currentIndex() const
 
 void QskMenu::setCurrentIndex( int index )
 {
-    if( index < 0 || index >= count() )
+    if( index < 0 || index >= m_data->options.count() )
+    {
         index = -1;
+    }
+    else
+    {
+        if ( m_data->options[index].isEmpty() ) // a seperator
+            index = -1;
+    }
 
     if( index != m_data->currentIndex )
     {
@@ -308,26 +331,34 @@ void QskMenu::wheelEvent( QWheelEvent* event )
 
 void QskMenu::traverse( int steps )
 {
-    if ( count() == 0 || ( steps % count() == 0 ) )
+    const auto& actions = m_data->actions;
+    const auto count = actions.count();
+
+    // -1 -> only one entry ?
+    if ( actions.isEmpty() || ( steps % count == 0 ) )
         return;
 
-    auto index = m_data->currentIndex + steps;
+    int action1 = qskActionIndex( actions, m_data->currentIndex );
+    int action2 = action1 + steps;
 
-    auto newIndex = index % count();
+    // when cycling we want to slide in
-    if ( newIndex < 0 )
+    int index1;
-        newIndex += count();
 
-    // when cycling we want slide in
+    if ( action2 < 0 )
+        index1 = m_data->options.count();
+    else if ( action2 >= count )
+        index1 = -1;
+    else
+        index1 = actions[ action1 ];
 
-    int startIndex = m_data->currentIndex;
+    action2 %= count;
+    if ( action2 < 0 )
+        action2 += count;
 
-    if ( index < 0 )
+    const auto index2 = actions[ action2 ];
-        startIndex = count();
-    else if ( index >= count() )
-        startIndex = -1;
 
-    movePositionHint( Cursor, startIndex, newIndex );
+    movePositionHint( Cursor, index1, index2 );
-    setCurrentIndex( newIndex );
+    setCurrentIndex( index2 );
 }
 
 void QskMenu::mousePressEvent( QMouseEvent* event )
@@ -384,7 +415,10 @@ void QskMenu::aboutToShow()
     setGeometry( QRectF( m_data->origin, sizeConstraint() ) );
 
     if ( m_data->currentIndex < 0 )
-        setCurrentIndex( 0 );
+    {
+        if ( !m_data->actions.isEmpty() )
+            setCurrentIndex( m_data->actions.first() );
+    }
 
     Inherited::aboutToShow();
 }
@@ -396,8 +430,10 @@ QRectF QskMenu::focusIndicatorRect() const
 
     if( currentIndex() >= 0 )
     {
+        auto actionIndex = qskActionIndex( m_data->actions, currentIndex() );
+
         return effectiveSkinlet()->sampleRect( this,
-            contentsRect(), Segment, currentIndex() );
+            contentsRect(), Segment, actionIndex );
     }
 
     return Inherited::focusIndicatorRect();
@@ -405,19 +441,23 @@ QRectF QskMenu::focusIndicatorRect() const
 
 QRectF QskMenu::cellRect( int index ) const
 {
+    const auto actionIndex = qskActionIndex( m_data->actions, index );
+
     return effectiveSkinlet()->sampleRect(
-        this, contentsRect(), QskMenu::Segment, index );
+        this, contentsRect(), QskMenu::Segment, actionIndex );
 }
 
 int QskMenu::indexAtPosition( const QPointF& pos ) const
 {
-    return effectiveSkinlet()->sampleIndexAt(
+    const auto index = effectiveSkinlet()->sampleIndexAt(
         this, contentsRect(), QskMenu::Segment, pos );
+
+    return m_data->actions.value( index, -1 );
 }
 
 void QskMenu::trigger( int index )
 {
-    if ( index >= 0 && index < m_data->options.count()  )
+    if ( index >= 0 && index < m_data->options.count() )
     {
         m_data->triggeredIndex = index;
         Q_EMIT triggered( index );

src/controls/QskMenu.h

@@ -26,8 +26,6 @@ class QSK_EXPORT QskMenu : public QskPopup
     Q_PROPERTY( QVector< QskLabelData > options READ options
         WRITE setOptions NOTIFY optionsChanged )
 
-    Q_PROPERTY( int count READ count )
-
     Q_PROPERTY( int currentIndex READ currentIndex
         WRITE setCurrentIndex NOTIFY currentIndexChanged )
 
@@ -58,6 +56,7 @@ class QSK_EXPORT QskMenu : public QskPopup
     int addOption( const QString&, const QString& );
     int addOption( const QUrl&, const QString& );
     int addOption( const QskLabelData& );
+    void addSeparator();
 
     void setOptions( const QVector< QskLabelData >& );
     void setOptions( const QStringList& );
@@ -65,14 +64,8 @@ class QSK_EXPORT QskMenu : public QskPopup
     QVector< QskLabelData > options() const;
     QskLabelData optionAt( int ) const;
 
-    int count() const;
+    QVector< int > separators() const;
-
+    QVector< int > actions() const;
-    void addSeparator();
-
-    int separatorPosition( int ) const;
-    int separatorCount() const;
-
-    void clear();
 
     int currentIndex() const;
     QString currentText() const;
@@ -98,6 +91,7 @@ class QSK_EXPORT QskMenu : public QskPopup
 
   public Q_SLOTS:
     void setCurrentIndex( int );
+    void clear();
 
   protected:
     void keyPressEvent( QKeyEvent* ) override;

src/controls/QskMenuSkinlet.cpp

@@ -18,6 +18,29 @@
 #include <qfontmetrics.h>
 #include <qmath.h>
 
+static inline int qskActionIndex( const QskMenu* menu, int optionIndex )
+{
+    if ( optionIndex < 0 )
+        return -1;
+
+    const auto& actions = menu->actions();
+
+    auto it = std::lower_bound(
+        actions.constBegin(), actions.constEnd(), optionIndex );
+
+    return it - actions.constBegin();
+}
+
+static inline qreal qskPaddedSeparatorHeight( const QskMenu* menu )
+{
+    using Q = QskMenu;
+
+    const auto margins = menu->marginHint( Q::Separator );
+
+    return menu->metric( Q::Separator | QskAspect::Size )
+        + margins.top() + margins.bottom();
+}
+
 class QskMenuSkinlet::PrivateData
 {
   public:
@@ -45,18 +68,6 @@ class QskMenuSkinlet::PrivateData
         m_segmentHeight = m_segmentWidth = m_graphicWidth = m_textWidth = -1.0;
     }
 
-    inline int separatorsBefore( const QskMenu* menu, int index ) const
-    {
-        int i = 0;
-        for ( ; i < menu->separatorCount(); i++ )
-        {
-            if ( menu->separatorPosition( i ) > index )
-                break;
-        }
-
-        return i;
-    }
-
     inline qreal graphicWidth( const QskMenu* menu ) const
     {
         if ( m_isCaching )
@@ -112,8 +123,6 @@ class QskMenuSkinlet::PrivateData
   private:
     qreal graphicWidthInternal( const QskMenu* menu ) const
     {
-        const auto skinlet = menu->effectiveSkinlet();
-
         const auto hint = menu->strutSizeHint( QskMenu::Icon );
         const qreal textHeight = menu->effectiveFontHeight( QskMenu::Text );
 
@@ -138,8 +147,6 @@ class QskMenuSkinlet::PrivateData
 
     qreal textWidthInternal( const QskMenu* menu ) const
     {
-        const auto skinlet = menu->effectiveSkinlet();
-
         const QFontMetricsF fm( menu->effectiveFont( QskMenu::Text ) );
 
         auto maxWidth = 0.0;
@@ -210,16 +217,29 @@ QskMenuSkinlet::~QskMenuSkinlet() = default;
 QRectF QskMenuSkinlet::cursorRect(
     const QskSkinnable* skinnable, const QRectF& contentsRect, int index ) const
 {
-    const auto count = sampleCount( skinnable, QskMenu::Segment );
+    using Q = QskMenu;
 
-    auto rect = sampleRect( skinnable, contentsRect,
+    const auto menu = static_cast< const QskMenu* >( skinnable );
-        QskMenu::Segment, qBound( 0, index, count ) );
+    const auto actions = menu->actions();
+
+    index = qskActionIndex( menu, index );
+
+    QRectF rect;
 
     if ( index < 0 )
+    {
+        rect = sampleRect( skinnable, contentsRect, Q::Segment, 0 );
         rect.setBottom( rect.top() );
-
+    }
-    if ( index >= count )
+    else if ( index >= actions.count() )
+    {
+        rect = sampleRect( skinnable, contentsRect, Q::Segment, actions.count() - 1 );
         rect.setTop( rect.bottom() );
+    }
+    else
+    {
+        rect = sampleRect( skinnable, contentsRect, Q::Segment, index );
+    }
 
     return rect;
 }
@@ -273,19 +293,15 @@ QRectF QskMenuSkinlet::sampleRect(
 
     if ( subControl == Q::Segment )
     {
+        const auto h = m_data->segmentHeight( menu );
+
+        auto dy = index * h;
+
+        if ( const auto n = menu->actions()[ index ] - index )
+            dy += n * qskPaddedSeparatorHeight( menu );
+
         const auto r = menu->subControlContentsRect( Q::Panel );
-
+        return QRectF( r.x(), r.y() + dy, r.width(), h );
-        auto h = m_data->segmentHeight( menu );
-
-        if ( int n = m_data->separatorsBefore( menu, index ) )
-        {
-            // spacing ???
-
-            const qreal separatorH = menu->metric( Q::Separator | QskAspect::Size );
-            h += n * separatorH;
-        }
-
-        return QRectF( r.x(), r.y() + index * h, r.width(), h );
     }
 
     if ( subControl == QskMenu::Icon || subControl == QskMenu::Text )
@@ -318,22 +334,19 @@ QRectF QskMenuSkinlet::sampleRect(
 
     if ( subControl == QskMenu::Separator )
     {
-        const int pos = menu->separatorPosition( index );
+        const auto separators = menu->separators();
-        if ( pos < 0 )
+        if ( index >= separators.count() )
             return QRectF();
 
-        QRectF r = menu->subControlContentsRect( Q::Panel );
+        const auto h = qskPaddedSeparatorHeight( menu );
 
-        if ( pos < menu->count() )
+        auto y = index * h;
-        {
-            const auto segmentRect = sampleRect( skinnable, contentsRect, Q::Segment, pos );
-            r.setBottom( segmentRect.top() ); // spacing ???
-        }
 
-        const qreal h = menu->metric( Q::Separator | QskAspect::Size );
+        if ( const auto n = qskActionIndex( menu, separators[ index ] ) )
-        r.setTop( r.bottom() - h );
+            y += n * m_data->segmentHeight( menu );
 
-        return r;
+        const auto r = menu->subControlContentsRect( Q::Panel );
+        return QRectF( r.left(), y, r.width(), h );
     }
 
     return Inherited::sampleRect(
@@ -356,13 +369,13 @@ int QskMenuSkinlet::sampleCount(
     if ( subControl == Q::Segment || subControl == Q::Icon || subControl == Q::Text )
     {
         const auto menu = static_cast< const QskMenu* >( skinnable );
-        return menu->count();
+        return menu->actions().count();
     }
 
     if ( subControl == Q::Separator )
     {
         const auto menu = static_cast< const QskMenu* >( skinnable );
-        return menu->separatorCount();
+        return menu->separators().count();
     }
 
     return Inherited::sampleCount( skinnable, subControl );
@@ -378,7 +391,8 @@ QskAspect::States QskMenuSkinlet::sampleStates(
     if ( subControl == Q::Segment || subControl == Q::Icon || subControl == Q::Text )
     {
         const auto menu = static_cast< const QskMenu* >( skinnable );
-        if ( menu->currentIndex() == index )
+
+        if ( menu->currentIndex() == menu->actions()[ index ] )
             states |= QskMenu::Selected;
     }
 
@@ -483,6 +497,8 @@ QSGNode* QskMenuSkinlet::updateSampleNode( const QskSkinnable* skinnable,
 
     if ( subControl == Q::Icon )
     {
+        index = menu->actions()[ index ];
+
         const auto graphic = menu->optionAt( index ).icon().graphic();
         if ( graphic.isNull() )
             return nullptr;
@@ -496,6 +512,8 @@ QSGNode* QskMenuSkinlet::updateSampleNode( const QskSkinnable* skinnable,
 
     if ( subControl == Q::Text )
     {
+        index = menu->actions()[ index ];
+
         const auto text = menu->optionAt( index ).text();
         if ( text.isEmpty() )
             return nullptr;
@@ -541,7 +559,7 @@ QSizeF QskMenuSkinlet::sizeHint( const QskSkinnable* skinnable,
 
     if ( const auto count = sampleCount( skinnable, Q::Separator ) )
     {
-        h += count * menu->metric( Q::Separator | QskAspect::Size );
+        h += count * qskPaddedSeparatorHeight( menu );
     }
 
     auto hint = skinnable->outerBoxSize( QskMenu::Panel, QSizeF( w, h ) );