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

[broulik (Kai Uwe Broulik)]

broulik added inline comments.

INLINE COMMENTS

> main.qml:94
> +
> +        onFocusChanged: {
> +            if (!focus) {

Does this ever happen? focus != activeFocus

> main.qml:113
> +            highlight: PlasmaComponents.Highlight {
> +                anchors.fill: windowListView.currentItem != null ? windowListView.currentItem : undefined
> +

Doesn't ListView take care of highlight placement and visibility?

> main.qml:121
> +            section.delegate: PlasmaComponents.Label {
> +                id: sectionLabel
> +

Unused id

> main.qml:142
> +
> +            delegate: Item {
> +                id: item

Just make this the MouseArea, saves you one Item per delegate

> main.qml:157
> +                    hoverEnabled: true
> +                    acceptedButtons: Qt.LeftButton
> +

This is the default

> main.qml:186
> +
> +                        width: visible ? units.iconSizes.small : 0
> +                        height: width

Layout.preferredWidth? I've seen wonkiness with IconItem and Layouts as it has an implicitWidth/height and that has priority over width/height as far as Layout is concerned

> main.qml:199
> +
> +                        width: (parent.width - icon.width - parent.spacing)
> +

Use RowLayout instead of Row and then Layout.fillWidth?

> main.qml:211
> +
> +                Keys.onTabPressed: unclutterButton.focus = true
> +                Keys.onBacktabPressed: windowPin.focus = true

There's a KeyNavigation attached property where you can tell it which the next tab/backtab/up/down item is

> main.qml:218
> +                Keys.onPressed: {
> +                    event.accepted = true;
> +

event.accepted twice, inside and outside the if

> main.qml:309
> +
> +            Accessible.role: Accessible.Button
> +            Accessible.name: text

QtQuick Controls Button from which PlasmaComponents Button inherits automatically handles Accessible automatically

> main.qml:388
> +    Component.onCompleted: {
> +        var toolTipData = new Object;
> +        toolTipData["image"] = "preferences-system-window";

Is that API even supported still? We have Plasmoid.toolTipMainText, Plasmoid.toolTipSubText etc

REPOSITORY
  rPLASMADESKTOP Plasma Desktop

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

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

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