broulik added inline comments.

INLINE COMMENTS

> DeviceList.qml:11
> +
> +Kirigami.Page {
> +    id: page

You might want to be using a `ScrollViewKCM` and put the enable checkbox in the 
`header` and the `ListView` in `view`, see for instance KWin's virtual desktop 
KCM

> DeviceList.qml:29
> +
> +                onCheckedChanged: function() {
> +                    deviceModel.manager.authMode = enableBox.checked

Use `onToggled` which is only fired when the user explicitly clicks it rather 
than if some binding causes it to change.
Then you could also bind the `checked: deviceModel.manager.authMode ...` 
directly (it shouldn't break the binding when you click it as in QQC2 that 
stuff is all done in C++)

Also, no need for `function()`

> main.qml:88
> +
> +            onClicked: {
> +                pageRow.push(deviceView, { manager: manager, device: device 
> })

`onItemClicked`?

> kded_bolt.cpp:90
> +                        mPendingDevices.size()),
> +            /*icon*/ QPixmap{}, /* widget */ nullptr,
> +            KNotification::Persistent,

Now we need a beautiful Breeze Thunderbolt icon :)

> kded_bolt.cpp:116
> +{
> +    QVector<QSharedPointer<Bolt::Device>> sorted;
> +    // Sort the devices so that parents go before their children. Probably

`reserve()`?

> kded_bolt.cpp:146
> +                i18n("Thunderbolt Device Authorization Error"),
> +                i18n("Failed to authorize Thunderbolt device <b>%1</b>: %2", 
> device->name(), error),
> +                /* icon */ QPixmap{}, /* parent */ nullptr,

Please HTML escape the device name

> device.h:45
> +    Q_PROPERTY(QString uid READ uid CONSTANT)
> +    Q_PROPERTY(QString name READ name CONSTANT STORED false)
> +    Q_PROPERTY(QString vendor READ vendor CONSTANT STORED false)

Does bolt not notify property changes on dbus? Not a huge fan of this Timer 
refresh hack

REPOSITORY
  R119 Plasma Desktop

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

To: dvratil
Cc: broulik, ognarb, yurchor, asturmlechner, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

Reply via email to