davidedmundson added a comment.
Generally awesome.
One major bug, rest are tiny comments.
Will give it a second review later, as it's a bit patch and I got tired :)
INLINE COMMENTS
> main.qml:175
>
> - mainText: desktopName
> - // our ToolTip has maximumLineCount of 8 which doesn't fit but
> QML doesn't
> - // respect that in RichText so we effectively can put in as much
> as we like :)
> - // it also gives us more flexibility when it comes to styling
> the <li>
> - textFormat: Text.RichText
> + property int effectiveRows: {
> + var columns = Math.floor(repeater.count / pagerModel.layoutRows);
add some readonly's over here.
It's apparently marginally faster; plus it can make things a bit more readable
> main.qml:402
> +
> + pagerModel.refresh();
> + } else {
why?
> pagermodel.cpp:76
> +int PagerModel::Private::instanceCount = 0;
> +ActivityInfo *PagerModel::Private::activityInfo = new ActivityInfo();
> +VirtualDesktopInfo *PagerModel::Private::virtualDesktopInfo = new
> VirtualDesktopInfo();
you delete them in
if (!instanceCount) in the dstror,
but you're creating them statically.
that will crash on create/remove/create and it's a bit weird.
> pagermodel.cpp:84
> +
> + QObject::connect(activityInfo, &ActivityInfo::currentActivityChanged, q,
> + [this]() {
this is effectively duped in refreshDatasSource()
> pagermodel.cpp:96
> + [this]() {
> + desktopDown = false;
> + }
I'm a bit confused here.
If VirtualDesktop changes you're resetting your own knowledge of if the desktop
is shown or not.
Why?
> pagermodel.cpp:250
> + d->windowModels.clear();
> +
> + endResetModel();
emit countChanged in both places
> pagermodel.cpp:429
> + // Toggle the desktop.
> + if (d->showDesktop) {
> + NETRootInfo info(QX11Info::connection(), 0);
this is a new feature?
It doesn't work in the current version anyway.
Note there is a DBus verison of showDashboard which is maybe sensible to use?
It cuts out some of the X specific code.
REPOSITORY
rPLASMADESKTOP Plasma Desktop
REVISION DETAIL
https://phabricator.kde.org/D2676
EMAIL PREFERENCES
https://phabricator.kde.org/settings/panel/emailpreferences/
To: hein, #plasma, davidedmundson
Cc: plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas
