broulik added inline comments. INLINE COMMENTS
> SidebarMode.cpp:402 > +{ > + QMenu *menu = new QMenu(); > + QStringList actionList { QStringLiteral("configure"), > QStringLiteral("help_contents"), QStringLiteral("help_about_app"), > QStringLiteral("help_about_kde") }; This leaks. `menu->setAttribute(Qt::WA_DeleteOnClose);` > SidebarMode.cpp:403 > + QMenu *menu = new QMenu(); > + QStringList actionList { QStringLiteral("configure"), > QStringLiteral("help_contents"), QStringLiteral("help_about_app"), > QStringLiteral("help_about_kde") }; > + for (QAction *a : d->collection->actions()) { `const` > SidebarMode.cpp:404 > + QStringList actionList { QStringLiteral("configure"), > QStringLiteral("help_contents"), QStringLiteral("help_about_app"), > QStringLiteral("help_about_kde") }; > + for (QAction *a : d->collection->actions()) { > + if (actionList.contains(a->objectName())) { I think it's better to iterate the list of actions and then get them from the collection. This way the order is also preserved correctly: for (const QString &actionName : actionList) { menu->addAction(d->collection->action(actionName); } > SidebarMode.cpp:410 > + connect(menu, &QMenu::aboutToHide, this, [this] () { > QMetaObject::invokeMethod(d->quickWidget->rootObject(), "closeMenu"); } ); > + menu->exec(position); > +} Don't `exec()` in conjunction with QML, this is just asking for trouble. Use `popup()` instead > main.qml:44 > + function closeMenu() { > + mainColumn.actionMenuButton.checked = false; > + } Can you instead do a `Q_PROPERTY(bool actionMenuVisible ...)` in `systemsettings` which you set `true` before the menu opens and set `false` in `aboutToHide`. Then bind the `checked` of the button to it. This way everything is in a predictable space and not called all over the place. REPOSITORY R124 System Settings REVISION DETAIL https://phabricator.kde.org/D22896 To: GB_2, #plasma, #vdg, ngraham, mart Cc: mart, filipf, ngraham, broulik, #vdg, plasma-devel, #plasma, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol