[FolderView] Don't calculate extra spacing when we don't need to.
ClosedPublic

Authored by Zren on Jun 21 2017, 6:56 PM.

Details

Summary

Wait until the ScrollView component has completed before attempting to
dynamically size the icons based on the viewport width/height.

A larger write up is in D6188, but essentially, variables are recalculated like this before the panels are added:

qml: cellHeight iconHeight 108
qml: onCellHeightChanged 108
qml: cellWidth iconWidth 92
qml: onCellWidthChanged 92
qml: cellHeight iconHeight 108
qml: cellHeight iconHeight 108
qml: cellWidth iconWidth 92
qml: scrollArea.onCompleted
qml: cellWidth iconWidth 92
qml: cellHeight iconHeight 108
qml: cellHeight iconHeight 108
qml: cellWidth iconWidth 92
qml:     availableColumns 20.869565217391305  = containerSize / cellSize 1920 92
qml:     availableColumns 20  floored
qml:     allColumnSize 1840
qml:     extraSpace 80
qml:     extraSpacing 4
qml: cellWidth extraWidth 4
qml: onCellWidthChanged 96
qml: cellHeight iconHeight 108
qml:     availableColumns 10  = containerSize / cellSize 1080 108
qml:     availableColumns 10  floored
qml:     allColumnSize 1080
qml:     extraSpace 0
qml:     extraSpacing 0                                                                                                                                                                                                                                                        
qml: cellHeight extraHeight 0

After the panels are added, it will do:

file:///home/chris/.local/share/plasma/plasmoids/org.kde.desktopcontainment/contents/ui/FolderItemDelegate.qml:282:17: QML Text: Binding loop detected for property "width"
file:///home/chris/.local/share/plasma/plasmoids/org.kde.desktopcontainment/contents/ui/FolderItemDelegate.qml:282:17: QML Text: Binding loop detected for property "width"
qml: cellHeight iconHeight 108
qml:     availableColumns 9.99074074074074  = containerSize / cellSize 1079 108
qml:     availableColumns 9  floored
qml:     allColumnSize 972
qml:     extraSpace 107
qml:     extraSpacing 11.88888888888889
qml: cellHeight extraHeight 11.88888888888889
qml: onCellHeightChanged 119.88888888888889
file:///home/chris/.local/share/plasma/plasmoids/org.kde.desktopcontainment/contents/ui/FolderItemDelegate.qml:282:17: QML Text: Binding loop detected for property "width"
file:///home/chris/.local/share/plasma/plasmoids/org.kde.desktopcontainment/contents/ui/FolderItemDelegate.qml:282:17: QML Text: Binding loop detected for property "width"
qml: timeLeft 0 1498071212427
qml: cellHeight iconHeight 108
qml:     availableColumns 9.722222222222221  = containerSize / cellSize 1050 108
qml:     availableColumns 9  floored
qml:     allColumnSize 972
qml:     extraSpace 78
qml:     extraSpacing 8.666666666666666
qml: cellHeight extraHeight 8.666666666666666
qml: onCellHeightChanged 116.66666666666667
file:///home/chris/.local/share/plasma/plasmoids/org.kde.desktopcontainment/contents/ui/FolderItemDelegate.qml:282:17: QML Text: Binding loop detected for property "width"
file:///home/chris/.local/share/plasma/plasmoids/org.kde.desktopcontainment/contents/ui/FolderItemDelegate.qml:282:17: QML Text: Binding loop detected for property "width"

It may sometimes skip the plasmoid.screenGeomertry.height = 1079 and only calculate it at 1050px.

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
Zren created this revision.Jun 21 2017, 6:56 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJun 21 2017, 6:56 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
hein requested changes to this revision.Jun 21 2017, 7:19 PM
hein added inline comments.
containments/desktop/package/contents/ui/FolderView.qml
492

Missing semicolon, also ready is defined nowhere.

This revision now requires changes to proceed.Jun 21 2017, 7:19 PM
Zren added inline comments.Jun 21 2017, 7:37 PM
containments/desktop/package/contents/ui/FolderView.qml
492

Arg, curses.

ready is defined 2 lines up. Also, do you want me to put scrollArea.ready so it's not confused with folderViewLayer.ready defined in main.qml?

https://github.com/KDE/plasma-desktop/blob/master/containments/desktop/package/contents/ui/main.qml#L405

hein added inline comments.Jun 21 2017, 7:55 PM
containments/desktop/package/contents/ui/FolderView.qml
492

Maybe I'm blind, but you're linking to a definition of a 'ready' prop in a different file. FolderView.qml can't depend on a property in main.qml, this will e.g. break in FolderViewDialog. And yeah, disambiguification is good ...

Zren added inline comments.Jun 21 2017, 8:04 PM
containments/desktop/package/contents/ui/FolderView.qml
492

Ready is defined right here https://i.imgur.com/fvRe3CI.png
The definition, assignment, and usage are all within 10 lines from each other so I didn't think I needed the scrollView. prefix but I might as well in case stuff is moved around later.

Zren updated this revision to Diff 15709.Jun 21 2017, 8:07 PM
Zren edited edge metadata.

+semicolon and specify it's scrollArea.ready

Zren marked an inline comment as done.Jun 21 2017, 8:09 PM
hein accepted this revision.EditedJun 21 2017, 8:27 PM

I see the definition now with the updated diff - maybe the Phab diff viewer was acting up or something (I think I remember Ctrl+F'ing even), or I really am blind ...

This revision is now accepted and ready to land.Jun 21 2017, 8:27 PM
This revision was automatically updated to reflect the committed changes.