ngraham added a comment.
The QML is generally fine! Since you specifically requested it, I added
nitpicky minor comments below. :) I don't consider them blockers, but feel free
to take a look anyway!
INLINE COMMENTS
> Comparison.qml:1
> +import QtQuick 2.0
> +import QtQuick.Controls 2.5 as QQC2
2.4
> Comparison.qml:3
> +import QtQuick.Controls 2.5 as QQC2
> +import QtQuick.Layouts 1.0
> +
1.3
> Comparison.qml:65
> + spacing: Kirigami.Units.largeSpacing
> + Component {
> + id: iconView
indentation
> Comparison.qml:69
> + width: comparisonSize
> + height: comparisonSize
> + source: modelData.iconPath
`height: width`
> Comparison.qml:70
> + height: comparisonSize
> + source: modelData.iconPath
> + }
Rather than creating a complicated additional item for the "no icon" case, I
would just do `source: modelData.iconPath ? modelData.iconPath : "paint-none"`.
If you don't like the `paint-none` icon, we can adjust it.
REPOSITORY
R118 Plasma SDK
BRANCH
comparison (branched from master)
REVISION DETAIL
https://phabricator.kde.org/D24305
To: davidre, #plasma, #vdg, ngraham
Cc: ngraham, filipf, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh,
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
