[Differential] [Changed Subscribers] D2089: Rewrite Window List plasmoid.

[davidedmundson (David Edmundson)]

davidedmundson added inline comments.

INLINE COMMENTS

> main.qml:70
>  
> -    function setOnDesktop(id, desktop) {
> -        var service = tasksSource.serviceForSource("tasks");
> -        var operation = service.operationDescription("toDesktop");
> -operation.Id = id;
> -        operation.desktop = desktop;
> -        service.startOperationCall(operation);
> +    PlasmaCore.FrameSvgItem {
> +        id : listItemSvg

You're using the margins from this frame, but you're not even rendering this frame anywhere?
Using the margins from something we're not rendering doesn't make really make sense semantically.

Then later we later use the left/right margins from the highlight which makes even less sense. No-one else does that.

We have a consistency problem with list view delegates over Plasma generally, so I this probably isn't any worse, but it's not right.

There is a PlasmaComponents.ListItem which does a fairly good job of handling some of this vaguely consistently. Though that's currently only used by 4 things.

> main.qml:157
> +                height: root.itemHeight
> +                width: windowListView.overflowing ? ListView.view.width - units.smallSpacing : ListView.view.width
> +

Not tested, but will this work with RTL?

Note if not, you just need an anchors.left: parent.left   and it will

> main.qml:203
> +
> +                        anchors.verticalCenter: parent.verticalCenter
> +

it's better to do

height: parent.height
you already have the centering of the text in the text alignment

REPOSITORY
  rPLASMADESKTOP Plasma Desktop

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: hein, #plasma
Cc: davidedmundson, broulik, plasma-devel, jensreuterberg, abetts, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160710/d37ebe88/attachment.html>