D28922: Port Sensor Face loading from plasmoid

Nathaniel Graham noreply at phabricator.kde.org
Mon May 4 17:45:25 BST 2020


ngraham added inline comments.

INLINE COMMENTS

> ConfigAppearance.qml:24
> +import QtQuick.Layouts 1.2
> +import QtQuick.Controls 2.2 as Controls
> +

`as QQC2`

> ConfigAppearance.qml:69
> +            delegate: Kirigami.SwipeListItem {
> +                contentItem: Controls.Label {
> +                    Layout.fillWidth: true

swipeListItem has its own label property; can we use that instead of overriding the contentItem?

> ConfigSensors.qml:24
> +import QtQuick.Layouts 1.2
> +import QtQuick.Controls 2.2 as Controls
> +import QtQml.Models 2.12

`as QQC2`

> FaceDetailsConfig.qml:24
> +import QtQuick.Layouts 1.2
> +import QtQuick.Controls 2.2 as Controls
> +

`as QQC2`

> FaceDetailsConfig.qml:45
> +            if (item.hasOwnProperty("cfg_" + key)) {
> +                root.controller.faceConfiguration[key] = item["cfg_"+key]
> +            }

`"cfg_" + key`

> UsedSensorsView.qml:24
> +import QtQuick.Layouts 1.2
> +import QtQuick.Controls 2.2 as Controls
> +import QtQml.Models 2.12

`as QQC2`

> CompactRepresentation.qml:24
>  import QtQuick 2.9
> +import QtQuick.Controls 2.2 as Controls
>  import QtQuick.Layouts 1.1

`as QQC2`

> Config.qml:23
> +import QtQuick.Layouts 1.1
> +import QtQuick.Controls 2.2 as Controls
> +

`as QQC2`

> CompactRepresentation.qml:24
>  import QtQuick 2.9
> +import QtQuick.Controls 2.2 as Controls
>  import QtQuick.Layouts 1.1

`as QQC2`

> Config.qml:22
> +import QtQuick.Layouts 1.1
> +import QtQuick.Controls 2.2 as Controls
> +

`as QQC2`

> LineChart.qml:47
> +    //TODO: Have a central heading here too?
> +    //TODO: Have a plasmoid config value for line thickness?
> +

Well, should we? :)

> CompactRepresentation.qml:24
>  import QtQuick 2.9
> +import QtQuick.Controls 2.2 as Controls
>  import QtQuick.Layouts 1.1

`as QQC2`

> Config.qml:22
> +import QtQuick.Layouts 1.1
> +import QtQuick.Controls 2.2 as Controls
> +

`as QQC2`

> Config.qml:22
> +import QtQuick.Layouts 1.1
> +import QtQuick.Controls 2.2 as Controls
> +import org.kde.ksysguard.faces 1.0 as Faces

`as QQC2`

> Config.qml:45
> +                sourceModel: Sensors.SensorDataModel {
> +                    // FIXME: unqualified property
> +                    sensors: controller.sensorIds

fixit :)

> Config.qml:53
> +            id: sortDescendingCheckBox
> +            // or do a ComboBox like in Dolphin with pretty names "highest first"<->"lowest first"?
> +            text: i18nc("Sort descending", "Descending")

That would be nice, yes. :) Nobody can ever figure out what "ascending" and "descending" means in the context of sort ordering.

> FullRepresentation.qml:24
> +import QtQuick 2.12
> +import QtQuick.Controls 2.2
> +import QtQuick.Layouts 1.2

`import QtQuick.Controls 2.2 as QQC2`

> FullRepresentation.qml:102
> +                    //FIXME: why +2 is necessary?
> +                    Layout.preferredWidth: metrics.width+2
> +                    Layout.maximumWidth: x > 0 ? metrics.width+2 : -1

Ugh, I don't like any of this :(

> FullRepresentation.qml:141
> +
> +            //See https://codereview.qt-project.org/c/qt/qtdeclarative/+/262876
> +            onWidthChanged: Qt.callLater(function() {if (tableView.columns > 0) {

this was merged almost a year ago; probably not needed anymore

> FullRepresentation.qml:159
> +                    onSensorsChanged: {
> +                        //note: this re sets the models in order to make the table work with any new role
> +                        tableView.model = null;

`resets the models`

REPOSITORY
  R111 KSysguard Library

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

To: mart, #plasma, ahiemstra, #vdg
Cc: ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200504/4317f185/attachment-0001.html>


More information about the Plasma-devel mailing list