D28146: [applets/appmenu] Use libtaskmanager for appmenus

[Kai Uwe Broulik]

broulik added a comment.


  I love patches with a lot of red!
  
  When filtering by screen, this now means we get appmenu only when the active window is on the same screen? Not sure if this is what we want?
  (Perhaps we could do some clever "last window with menu" tracking so we can have per-screen global menu? :D)

INLINE COMMENTS

> CMakeLists.txt:15
> +                      KF5::WindowSystem
> +                      PW::LibTaskManager)
>  

Unused

> appmenumodel.cpp:69
>  
> -    connect(this, &AppMenuModel::screenGeometryChanged, this, [this] {
> -        onWindowChanged(m_currentWindowId);
> -    });
> +    connect(this, &AppMenuModel::screenGeometryChanged, this, &AppMenuModel::onActiveWindowChanged);
>  

Is this needed? I would think if screen geometry changes, task manager updates and filters and signals the active task having changed?

> appmenumodel.cpp:147
>  
> +#include <QDebug>
>  

Remove

> appmenumodel.cpp:151
>  {
> -    qApp->removeNativeEventFilter(this);
> +    auto objectPath = m_tasksModel->data(m_tasksModel->activeTask(), TaskManager::AbstractTasksModel::ApplicationMenuObjectPath).value<QString>();
> +    auto serviceName = m_tasksModel->data(m_tasksModel->activeTask(), TaskManager::AbstractTasksModel::ApplicationMenuServiceName).value<QString>();

Please write as

  const QModelIndex activeTaskIdx = m_tasksMode->activeTask();
  const QString objectPath = m_tasksModel->data(activeTaskIdx, TaskManager::AbstractTasmsModel::ApplicationMenuObjectPath).toString();
  const QString serviceName = ....;

(perhaps you could drop a `using namespace TaskManager` at the top of the cpp file)

> appmenumodel.cpp:154
>  
> -    if (!id) {
> -        setMenuAvailable(false);
> +    if (objectPath != QString() && serviceName != QString()) {
> +        setMenuAvailable(true);

`!objectPath.isEmpty()`

> appmenumodel.h:88
>  
>      QRect m_screenGeometry;
>  

I think you can remove this member now and just forward to tasksmodel

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D28146

To: cblack, #plasma
Cc: broulik, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200319/304d33e6/attachment-0001.html>