QMenu items checkable/not-checkable not text aligned?

Christian Ehrlicher

There are some bugs about this. What Qt version do you use and how do your style this?

quiret

Code lines related to the issue:

        viewMenu = menu->addMenu("");

        QAction *actionShowFullImage = CreateMnemonicActionL( "Main.View.FullImg", this);
        // actionBackground->setShortcut(Qt::Key_F4);
        actionShowFullImage->setCheckable(true);
        viewMenu->addAction(actionShowFullImage);

        connect(actionShowFullImage, &QAction::triggered, this, &mainWindowMenuEnv::onToggleShowFullImage);

        QAction *actionBackground = CreateMnemonicActionL( "Main.View.Backgr", this);
        actionBackground->setShortcut( Qt::Key_F5 );
        actionBackground->setCheckable(true);
        viewMenu->addAction(actionBackground);

        connect(actionBackground, &QAction::triggered, this, &mainWindowMenuEnv::onToggleShowBackground);

        QAction *actionShowMipLevels = CreateMnemonicActionL( "Main.View.DispML", this );
        actionShowMipLevels->setShortcut( Qt::Key_F6 );
        actionShowMipLevels->setCheckable(true);
        viewMenu->addAction(actionShowMipLevels);

        connect( actionShowMipLevels, &QAction::triggered, this, &mainWindowMenuEnv::onToggleShowMipmapLayers );

        QAction *actionShowLog = CreateMnemonicActionL( "Main.View.ShowLog", this );
        actionShowLog->setShortcut( Qt::Key_F7 );
        viewMenu->addAction(actionShowLog);

        connect( actionShowLog, &QAction::triggered, this, &mainWindowMenuEnv::onToggleShowLog );

        viewMenu->addSeparator();

        this->actionThemeDark = CreateMnemonicActionL( "Main.View.DarkThm", this );
        this->actionThemeDark->setCheckable(true);
        this->actionThemeLight = CreateMnemonicActionL( "Main.View.LightTm", this );
        this->actionThemeLight->setCheckable(true);

        // enable needed theme in menu before connecting a slot
        actionThemeDark->setChecked(true);

        connect(this->actionThemeDark, &QAction::triggered, this, &mainWindowMenuEnv::onToogleDarkTheme);
        connect(this->actionThemeLight, &QAction::triggered, this, &mainWindowMenuEnv::onToogleLightTheme);

        viewMenu->addAction(this->actionThemeDark);
        viewMenu->addAction(this->actionThemeLight);

        actionQuit->setShortcut(QKeySequence::Quit);
        connect(actionQuit, &QAction::triggered, mainWnd, &MainWindow::close);

        infoMenu = menu->addMenu("");

(https://osdn.net/projects/magic-txd/scm/svn/blobs/head/src/mainwindow.menu.cpp at line 338 revision 70)

Related stylesheet lines:

QMenu
{
    background-color: #181818;
    border: none;
    font-family:"Segoe UI Light";
    font-size: 22px;
    color: #ebebeb;
}

QMenu::item
{
    background-color: transparent;
    padding: 2px 20px;
}

QMenu::item:disabled
{
    background-color: #080808;
    color: #858585;
}

QMenu::item:selected:!disabled
{
    background-color: #383838;
}

QMenu::separator
{
    height: 1px;
    background-color: #383838;
    margin-left: 0px;
    margin-right: 0px;
}

(https://osdn.net/projects/magic-txd/svn/view/resources/dark.shell?view=markup&revision=74&root=magic-txd)

Any help is appreciated! I understand that potential code irregularities could have caused this. I am willing to be patient or help with testing of newer Qt builds.

Christian Ehrlicher

@quiret said in QMenu items checkable/not-checkable not text aligned?:

QMenu::item
{
background-color: transparent;
padding: 2px 20px;
}

The padding is calculated from the beginning and is increased when it's to small (e.g. because there is a checkbox). Try to set a higher value here.

quiret

@Christian-Ehrlicher

New stylesheet content:

QMenu::item
{
    background-color: transparent;
    padding: 2px 40px;
}

Visual Result:

As you can see the increased horizontal padding has worsened the issue. Please help!

Christian Ehrlicher

I think it was introduced with https://bugreports.qt.io/browse/QTBUG-74655 - as a quick check please provide a (maybe even transparent) icon for the 'Show log' menu entry.
And as always - it would be nice to get a very simple example which only triggers the bad behavior without the rest (esp. the other css stylings) - it makes debugging much easier.

quiret

@Christian-Ehrlicher Thank you very much for your guidance! Due to the size of Qt I was unsure whether the issue was on my behalf or Qt's. To further improve our investigation I will create a small sample project that will be easier to debug. I will try to get it done by today. My intention was not to have you debug my large Qt application - I just wanted to get feedback with little effort by my side. But I have realized that this issue is worth investigating so stand by...

quiret

@Christian-Ehrlicher Here is a sample project. Since I am unable to upload any files, here is the text content of the most relevant source files:

main.cpp:

#include <QtWidgets/qapplication.h>
#include <QtWidgets/qmainwindow.h>
#include <QtWidgets/qmenu.h>
#include <QtWidgets/qmenubar.h>
#include <QtCore/qfile.h>

#include <QtCore/QtPlugin>

Q_IMPORT_PLUGIN(QWindowsIntegrationPlugin)

int main( int argc, char *argv[] )
{
    QApplication coreapp( argc, argv );

    QMainWindow *mainWnd = new QMainWindow( nullptr );
    {
        QFile sheetfile( "style.shell" );

        sheetfile.open( QIODevice::ReadOnly );

        auto content = sheetfile.readAll();

        QString sheetcontent = QString::fromLatin1( content );

        mainWnd->setStyleSheet( sheetcontent );

        QMenuBar *testMenuBar = new QMenuBar( mainWnd );

        QMenu *testMenu = testMenuBar->addMenu( "File" );

        QAction *uncheckableAction = new QAction( "Test u" );
        uncheckableAction->setCheckable( false );

        QAction *checkableAction = new QAction( "Test c" );
        checkableAction->setCheckable( true );

        testMenu->addAction( uncheckableAction );
        testMenu->addAction( checkableAction );

        mainWnd->setMenuBar( testMenuBar );

        mainWnd->show();
    }
    coreapp.exec();

    delete mainWnd;

    return 0;
}

style.shell:

QMenuBar
{
    min-height: 52px;
    font-family:"Segoe UI Light";
    font-size: 26px;
    color: #000000;
    border: none;
}

QMenuBar::item
{
    spacing: 0px; /* spacing between menu bar items */
    padding: 1px 20px;
}

QMenuBar::item:selected
{
    background: #383838;
}

QMenuBar::item:pressed
{
    background: #383838;
}

QMenu
{
    background-color: #181818;
    border: none;
    font-family:"Segoe UI Light";
    font-size: 22px;
    color: #ebebeb;
}

QMenu::item
{
    background-color: transparent;
    padding: 2px 40px;
}

QMenu::item:disabled
{
    background-color: #080808;
    color: #858585;
}

QMenu::item:selected:!disabled
{
    background-color: #383838;
}

QMenu::separator
{
    height: 1px;
    background-color: #383838;
    margin-left: 0px;
    margin-right: 0px;
}

This sample contains a menu bar with one menu called "File". Inside of the menu are two items, one checkable and the other not checkable. You can see that even with a padding applied to both items the text is not aligned. This is contrary to Qt5.11 behaviour where the padding made the text aligned.

Please continue helping me resolve this issue further down the line! :)

Christian Ehrlicher

I simplified it even a little bit more

#include <QtWidgets>

int main( int argc, char *argv[] )
{
  QApplication coreapp( argc, argv );

  QMainWindow mainWnd;
  mainWnd.setStyleSheet(R"(QMenu::item
                         {
                         background-color: transparent;
                         padding: 2px 40px;
                         })" );
  QMenu *testMenu = mainWnd.menuBar()->addMenu( "File" );
  QAction *uncheckableAction = new QAction( "Test u" );
  uncheckableAction->setCheckable( false );
  QAction *checkableAction = new QAction( "Test c" );
  checkableAction->setCheckable( true );
  testMenu->addAction( uncheckableAction );
  testMenu->addAction( checkableAction );
  mainWnd.show();
  return coreapp.exec();
}

Christian Ehrlicher

As expected - add a transparent QIcon to one of your menu actions and all is fine. Once I tried to fix it but got a lot of other regressions so I had to revert it. And I'm currently not interested in trying it again - sorry. But I think the workaround should be fine for you.
The problem is that the padding defines the padding of the content rect which includes the decoration and checkbox so we can't 'misuse' this. And when drawing a single element one does not know that other elements have a checkbox so adding an offset there also does not work. Maybe QStyleOptionMenuItem.maxIconWidth should also be initialized with a proper value when only checkable and non-checkable (but no icon) menu items are available... But only for Qt6 I would guess.

quiret

@Christian-Ehrlicher OK. I will note that as the mandated work-around. Thank you for your time.

I heard you teasing Qt6 in praise. Will it improve a lot of shortcomings of Qt? :)

Christian Ehrlicher

@quiret said in QMenu items checkable/not-checkable not text aligned?:

Will it improve a lot of shortcomings of Qt?

Hard to tell - a lot of internal cleanup but not much new on the widgets front. Can't say anything to QML though.

Christian Ehrlicher

I think I found a simple solution but don't know why I did not saw this during my fixes in 5.12.x... do you compile Qt by your own?

quiret

@Christian-Ehrlicher Yes. Why?

Christian Ehrlicher

@quiret Then please try this fix:

diff --git a/src/widgets/styles/qstylesheetstyle.cpp b/src/widgets/styles/qstylesheetstyle.cpp
index 72f7ad7455..dd31d67728 100644
--- a/src/widgets/styles/qstylesheetstyle.cpp
+++ b/src/widgets/styles/qstylesheetstyle.cpp
@@ -3774,6 +3774,12 @@ void QStyleSheetStyle::drawControl(ControlElement ce, const QStyleOption *opt, Q
                     }
                     textRectOffset = std::max(textRectOffset, cmRect.width());
                 }
+                // make sure all items are aligned, no matter if they have an icon, checkbox or nothing
+                if (m->menuHasCheckableItems) {
+                    QRenderRule subSubRule = renderRule(w, opt, PseudoElement_MenuCheckMark);
+                    const QRect cmRect = positionRect(w, subRule, subSubRule, PseudoElement_MenuCheckMark, opt->rect, opt->direction);
+                    textRectOffset = std::max(textRectOffset, cmRect.width());
+                }
 
                 QRect textRect = subRule.contentsRect(opt->rect);
                 textRect.setLeft(textRect.left() + textRectOffset);

/edit: updated the patch a little bit to make sure all items are aligned, no matter if they have an icon, checkbox or nothing. Then the alignment should also work when there is an icon and a checkbox entry and the checkbox is larger than the icon.

quiret

@Christian-Ehrlicher Sorry about the delay. My external HDD was acting up, but I have GOOD NEWS!

This looks dang right. Thank you very much for your help! Do you think this patch can make it into Qt?

Christian Ehrlicher

@quiret said in QMenu items checkable/not-checkable not text aligned?:

Do you think this patch can make it into Qt?

I think yes. Need some more tests to not screw up other stuff - sadly the css styling is very fragile. But I think we don't get it into 5.x because there will be no opensource version of 5.x anymore for the next 12 months afaiu.

quiret

@Christian-Ehrlicher Oh! Now I understand why you made me go through this tiresome process of compiling Qt from source! That's fine, it takes ages to compile Qt. I really wanted to give you valuable feedback too. But if there is no more update to the git tracker in terms of versioning then I could have been left in the dust. I can assure you that users of my tool will be grateful for your contribution!

I realize the Qt team has two concepts: git tracker and website releases. With "no opensource version of 5.x" do you mean no further branching on the git tracker?

Christian Ehrlicher

@quiret said in QMenu items checkable/not-checkable not text aligned?:

With "no opensource version of 5.x" do you mean no further branching on the git tracker?

I don't know exactly how TQtC will handle it. They said they will the delay the open source version as long as possible (which is a max. of 1 year according the KDE Free Qt Foundation). How they will hide the releases is unknown and there was no real communication from TQtC how they will handle it. Therefore I decided by myself to not port anything back to Qt5 (or Qt6.x-1) because I as a user of the opensource version don't have any advantage out of it.

Christian Ehrlicher

Review: https://codereview.qt-project.org/c/qt/qtbase/+/327618