D19011: Thunderbolt KCM and KDED module

Kai Uwe Broulik noreply at phabricator.kde.org
Fri Feb 15 08:07:12 GMT 2019


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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190215/29af3725/attachment-0001.html>


More information about the Plasma-devel mailing list