broulik added a comment.

  Neat!
  
  I do get a bunch of warnings on startup, though:
  
    
    
file:///usr/share/plasma/plasmoids/org.kde.desktopcontainment/contents/ui/FolderItemDelegate.qml:244:17:
 QML Text: Binding loop detected for property "width"
  
  Spring-loading when dragging a file ontop of a folder no longer works (didn't 
see any obvious warnings on console)

INLINE COMMENTS

> FolderItemDelegate.qml:55
>  
> +        visible: status == Loader.Ready
> +

`===`

> FolderItemDelegate.qml:61
> +
> +        asynchronous: true
>      }

I ran into quite some trouble (layouts, crashes, glitches) with async loaders 
in an item view, you said it didn't help anyways, so perhaps we can try without?

> FolderItemDelegate.qml:79
>              property Item label: label
> -            property Item labelArea: textShadow
> +            property Item labelArea: frameLoader.textShadow ? 
> frameLoader.textShadow : label
>              property Item actionsOverlay: actions

Could be simplified to

  frameLoader.textShadow || label

> FolderItemDelegate.qml:89
> +                    frameLoader.grabToImage(function(result) {
> +                        dir.addItemDragImage(positioner.map(index), main.x + 
> frameLoaderx, main.y + frameLoader.y, frameLoader.width, frameLoader.height, 
> result.image);
>                      });

typo `frameLoader.x`

That's why the drag pixmap doesn't work

> FolderItemDelegate.qml:120
> +                        selectionButton = null;
> +                    }
> +

QML nulls properties automatically if the item referenced is destroyed but I 
understand you want this explicitly

> FolderItemDelegate.qml:177
> +                sourceComponent: frameComponent
> +                active: state != ""
> +                asynchronous: true

`!==`

> FolderItemDelegate.qml:292
>  
> -                        color: textShadow.visible ? "white" : 
> PlasmaCore.ColorScope.textColor
> +                    color: (frameLoader.textShadow && 
> frameLoader.textShadow.visible
> +                        ? "white" : PlasmaCore.ColorScope.textColor)

Careful with binding things to `visible`, it updates recursively and may cause 
unexpected re-evaluations or glitches when the applet popup opens/closes (like 
the folderview title bug)

> FolderItemDelegate.qml:293
> +                    color: (frameLoader.textShadow && 
> frameLoader.textShadow.visible
> +                        ? "white" : PlasmaCore.ColorScope.textColor)
>  

Since we're all about speed here, `"#fff"` is ~6% faster than `"white"` :D

REPOSITORY
  R119 Plasma Desktop

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

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

To: hein, #plasma
Cc: broulik, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas

Reply via email to