From a053373d575b531d791ed58c79b9be5bd3b22f07 Mon Sep 17 00:00:00 2001 From: outfoxxed Date: Fri, 13 Dec 2024 01:30:11 -0800 Subject: [PATCH] core/qsmenu!: improve menu layout change UX Exposes QsMenuOpener.children as an ObjectModel instead of a list to allow smoother layout change handling in custom menu renderers. Fixes QsMenuAnchor/platform menus closing whenever menu content changes. --- src/core/model.cpp | 16 +++++++++++++ src/core/model.hpp | 8 +++++++ src/core/platformmenu.cpp | 8 ++++--- src/core/qsmenu.cpp | 33 +++++---------------------- src/core/qsmenu.hpp | 14 +++++------- src/dbus/dbusmenu/dbusmenu.cpp | 41 +++++++++++++++------------------- src/dbus/dbusmenu/dbusmenu.hpp | 8 +++---- 7 files changed, 61 insertions(+), 67 deletions(-) diff --git a/src/core/model.cpp b/src/core/model.cpp index bcba6a15..30ded8f0 100644 --- a/src/core/model.cpp +++ b/src/core/model.cpp @@ -71,6 +71,22 @@ bool UntypedObjectModel::removeObject(const QObject* object) { return true; } +void UntypedObjectModel::diffUpdate(const QVector& newValues) { + for (qsizetype i = 0; i < this->valuesList.length();) { + if (newValues.contains(this->valuesList.at(i))) i++; + else this->removeAt(i); + } + + qsizetype oi = 0; + for (auto* object: newValues) { + if (this->valuesList.length() == oi || this->valuesList.at(oi) != object) { + this->insertObject(object, oi); + } + + oi++; + } +} + qsizetype UntypedObjectModel::indexOf(QObject* object) { return this->valuesList.indexOf(object); } UntypedObjectModel* UntypedObjectModel::emptyInstance() { diff --git a/src/core/model.hpp b/src/core/model.hpp index 9bfe406d..56297bfa 100644 --- a/src/core/model.hpp +++ b/src/core/model.hpp @@ -73,6 +73,9 @@ protected: void insertObject(QObject* object, qsizetype index = -1); bool removeObject(const QObject* object); + // Assumes only one instance of a specific value + void diffUpdate(const QVector& newValues); + QVector valuesList; private: @@ -97,6 +100,11 @@ public: void removeObject(const T* object) { this->UntypedObjectModel::removeObject(object); } + // Assumes only one instance of a specific value + void diffUpdate(const QVector& newValues) { + this->UntypedObjectModel::diffUpdate(*std::bit_cast*>(&newValues)); + } + static ObjectModel* emptyInstance() { return static_cast*>(UntypedObjectModel::emptyInstance()); } diff --git a/src/core/platformmenu.cpp b/src/core/platformmenu.cpp index a06575ea..427dde08 100644 --- a/src/core/platformmenu.cpp +++ b/src/core/platformmenu.cpp @@ -20,6 +20,7 @@ #include "../window/proxywindow.hpp" #include "../window/windowinterface.hpp" #include "iconprovider.hpp" +#include "model.hpp" #include "platformmenu_p.hpp" #include "popupanchor.hpp" #include "qsmenu.hpp" @@ -61,6 +62,7 @@ PlatformMenuEntry::PlatformMenuEntry(QsMenuEntry* menu): QObject(menu), menu(men QObject::connect(menu, &QsMenuEntry::buttonTypeChanged, this, &PlatformMenuEntry::onButtonTypeChanged); QObject::connect(menu, &QsMenuEntry::checkStateChanged, this, &PlatformMenuEntry::onCheckStateChanged); QObject::connect(menu, &QsMenuEntry::hasChildrenChanged, this, &PlatformMenuEntry::relayoutParent); + QObject::connect(menu->children(), &UntypedObjectModel::valuesChanged, this, &PlatformMenuEntry::relayout); // clang-format on } @@ -178,10 +180,10 @@ void PlatformMenuEntry::relayout() { this->qmenu->setIcon(getCurrentEngineImageAsIcon(icon)); } - auto children = this->menu->children(); - auto len = children.count(&children); + const auto& children = this->menu->children()->valueList(); + auto len = children.count(); for (auto i = 0; i < len; i++) { - auto* child = children.at(&children, i); + auto* child = children.at(i); auto* instance = new PlatformMenuEntry(child); QObject::connect(instance, &QObject::destroyed, this, &PlatformMenuEntry::onChildDestroyed); diff --git a/src/core/qsmenu.cpp b/src/core/qsmenu.cpp index de9ed6f6..760f7e76 100644 --- a/src/core/qsmenu.cpp +++ b/src/core/qsmenu.cpp @@ -4,8 +4,8 @@ #include #include #include -#include +#include "model.hpp" #include "platformmenu.hpp" using namespace qs::menu::platform; @@ -34,15 +34,6 @@ void QsMenuEntry::display(QObject* parentWindow, int relativeX, int relativeY) { if (!success) delete platform; } -QQmlListProperty QsMenuEntry::emptyChildren(QObject* parent) { - return QQmlListProperty( - parent, - nullptr, - &QsMenuEntry::childCount, - &QsMenuEntry::childAt - ); -} - void QsMenuEntry::ref() { this->refcount++; if (this->refcount == 1) emit this->opened(); @@ -53,7 +44,9 @@ void QsMenuEntry::unref() { if (this->refcount == 0) emit this->closed(); } -QQmlListProperty QsMenuEntry::children() { return QsMenuEntry::emptyChildren(this); } +ObjectModel* QsMenuEntry::children() { + return ObjectModel::emptyInstance(); +} QsMenuOpener::~QsMenuOpener() { if (this->mMenu) { @@ -83,13 +76,6 @@ void QsMenuOpener::setMenu(QsMenuHandle* menu) { if (menu != nullptr) { auto onMenuChanged = [this, menu]() { if (menu->menu()) { - QObject::connect( - menu->menu(), - &QsMenuEntry::childrenChanged, - this, - &QsMenuOpener::childrenChanged - ); - menu->menu()->ref(); } @@ -113,19 +99,12 @@ void QsMenuOpener::onMenuDestroyed() { emit this->childrenChanged(); } -QQmlListProperty QsMenuOpener::children() { +ObjectModel* QsMenuOpener::children() { if (this->mMenu && this->mMenu->menu()) { return this->mMenu->menu()->children(); } else { - return QsMenuEntry::emptyChildren(this); + return ObjectModel::emptyInstance(); } } -qsizetype QsMenuEntry::childCount(QQmlListProperty* /*property*/) { return 0; } - -QsMenuEntry* -QsMenuEntry::childAt(QQmlListProperty* /*property*/, qsizetype /*index*/) { - return nullptr; -} - } // namespace qs::menu diff --git a/src/core/qsmenu.hpp b/src/core/qsmenu.hpp index 932bdb2a..6684c686 100644 --- a/src/core/qsmenu.hpp +++ b/src/core/qsmenu.hpp @@ -9,6 +9,7 @@ #include #include "doc.hpp" +#include "model.hpp" namespace qs::menu { @@ -107,9 +108,7 @@ public: void ref(); void unref(); - [[nodiscard]] virtual QQmlListProperty children(); - - static QQmlListProperty emptyChildren(QObject* parent); + [[nodiscard]] virtual ObjectModel* children(); signals: /// Send a trigger/click signal to the menu entry. @@ -125,12 +124,8 @@ signals: void buttonTypeChanged(); void checkStateChanged(); void hasChildrenChanged(); - QSDOC_HIDE void childrenChanged(); private: - static qsizetype childCount(QQmlListProperty* property); - static QsMenuEntry* childAt(QQmlListProperty* property, qsizetype index); - qsizetype refcount = 0; }; @@ -140,7 +135,8 @@ class QsMenuOpener: public QObject { /// The menu to retrieve children from. Q_PROPERTY(qs::menu::QsMenuHandle* menu READ menu WRITE setMenu NOTIFY menuChanged); /// The children of the given menu. - Q_PROPERTY(QQmlListProperty children READ children NOTIFY childrenChanged); + QSDOC_TYPE_OVERRIDE(ObjectModel*); + Q_PROPERTY(UntypedObjectModel* children READ children NOTIFY childrenChanged); QML_ELEMENT; public: @@ -151,7 +147,7 @@ public: [[nodiscard]] QsMenuHandle* menu() const; void setMenu(QsMenuHandle* menu); - [[nodiscard]] QQmlListProperty children(); + [[nodiscard]] ObjectModel* children(); signals: void menuChanged(); diff --git a/src/dbus/dbusmenu/dbusmenu.cpp b/src/dbus/dbusmenu/dbusmenu.cpp index 653ec8cb..e86b580d 100644 --- a/src/dbus/dbusmenu/dbusmenu.cpp +++ b/src/dbus/dbusmenu/dbusmenu.cpp @@ -21,6 +21,7 @@ #include #include "../../core/iconimageprovider.hpp" +#include "../../core/model.hpp" #include "../../core/qsmenu.hpp" #include "../../dbus/properties.hpp" #include "dbus_menu.h" @@ -95,22 +96,8 @@ void DBusMenuItem::updateLayout() const { bool DBusMenuItem::hasChildren() const { return this->displayChildren || this->id == 0; } -QQmlListProperty DBusMenuItem::children() { - return QQmlListProperty( - this, - nullptr, - &DBusMenuItem::childrenCount, - &DBusMenuItem::childAt - ); -} - -qsizetype DBusMenuItem::childrenCount(QQmlListProperty* property) { - return reinterpret_cast(property->object)->enabledChildren.count(); -} - -QsMenuEntry* DBusMenuItem::childAt(QQmlListProperty* property, qsizetype index) { - auto* item = reinterpret_cast(property->object); - return item->menu->items.value(item->enabledChildren.at(index)); +ObjectModel* DBusMenuItem::children() { + return reinterpret_cast*>(&this->enabledChildren); } void DBusMenuItem::updateProperties(const QVariantMap& properties, const QStringList& removed) { @@ -270,14 +257,13 @@ void DBusMenuItem::updateProperties(const QVariantMap& properties, const QString } void DBusMenuItem::onChildrenUpdated() { - this->enabledChildren.clear(); - + QVector children; for (auto child: this->mChildren) { auto* item = this->menu->items.value(child); - if (item->visible) this->enabledChildren.push_back(child); + if (item->visible) children.append(item); } - emit this->childrenChanged(); + this->enabledChildren.diffUpdate(children); } QDebug operator<<(QDebug debug, DBusMenuItem* item) { @@ -388,7 +374,7 @@ void DBusMenu::updateLayoutRecursive( [&](const DBusMenuLayout& layout) { return layout.id == *iter; } ); - if (existing == layout.children.end()) { + if (!item->mShowChildren || existing == layout.children.end()) { qCDebug(logDbusMenu) << "Removing missing layout item" << this->items.value(*iter) << "from" << item; this->removeRecursive(*iter); @@ -402,7 +388,7 @@ void DBusMenu::updateLayoutRecursive( for (const auto& child: layout.children) { if (item->mShowChildren && !item->mChildren.contains(child.id)) { qCDebug(logDbusMenu) << "Creating new layout item" << child.id << "in" << item; - item->mChildren.push_back(child.id); + // item->mChildren.push_back(child.id); this->items.insert(child.id, nullptr); childrenChanged = true; } @@ -410,7 +396,15 @@ void DBusMenu::updateLayoutRecursive( this->updateLayoutRecursive(child, item, depth - 1); } - if (childrenChanged) item->onChildrenUpdated(); + if (childrenChanged) { + // reset to preserve order + item->mChildren.clear(); + for (const auto& child: layout.children) { + item->mChildren.push_back(child.id); + } + + item->onChildrenUpdated(); + } } if (item->mShowChildren && !item->childrenLoaded) { @@ -554,6 +548,7 @@ void DBusMenuHandle::onMenuPathChanged() { this->mMenu->setParent(this); QObject::connect(&this->mMenu->rootItem, &DBusMenuItem::layoutUpdated, this, [this]() { + QObject::disconnect(&this->mMenu->rootItem, &DBusMenuItem::layoutUpdated, this, nullptr); this->loaded = true; emit this->menuChanged(); }); diff --git a/src/dbus/dbusmenu/dbusmenu.hpp b/src/dbus/dbusmenu/dbusmenu.hpp index 75d2c26a..35afa98e 100644 --- a/src/dbus/dbusmenu/dbusmenu.hpp +++ b/src/dbus/dbusmenu/dbusmenu.hpp @@ -15,6 +15,7 @@ #include "../../core/doc.hpp" #include "../../core/imageprovider.hpp" +#include "../../core/model.hpp" #include "../../core/qsmenu.hpp" #include "../properties.hpp" #include "dbus_menu_types.hpp" @@ -65,7 +66,7 @@ public: [[nodiscard]] bool isShowingChildren() const; void setShowChildrenRecursive(bool showChildren); - [[nodiscard]] QQmlListProperty children() override; + [[nodiscard]] ObjectModel* children() override; void updateProperties(const QVariantMap& properties, const QStringList& removed = {}); void onChildrenUpdated(); @@ -96,11 +97,8 @@ private: menu::QsMenuButtonType::Enum mButtonType = menu::QsMenuButtonType::None; Qt::CheckState mCheckState = Qt::Unchecked; bool displayChildren = false; - QVector enabledChildren; + ObjectModel enabledChildren {this}; DBusMenuItem* parentMenu = nullptr; - - static qsizetype childrenCount(QQmlListProperty* property); - static menu::QsMenuEntry* childAt(QQmlListProperty* property, qsizetype index); }; QDebug operator<<(QDebug debug, DBusMenuItem* item);