D19289: [DeviceNotifier] Port settings to QQC2 and Kirigami

2019-02-25 Thread Filip Fila 
This revision was automatically updated to reflect the committed changes.
Closed by commit R120:6be6a8c003c6: [DeviceNotifier] Port settings to QQC2 and 
Kirigami (authored by filipf).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19289?vs=52497=52516

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

AFFECTED FILES
  applets/devicenotifier/package/contents/ui/configGeneral.qml

To: filipf, #vdg, #plasma, ngraham, davidedmundson
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19289: [DeviceNotifier] Port settings to QQC2 and Kirigami

2019-02-24 Thread Filip Fila 
filipf updated this revision to Diff 52497.
filipf added a comment.


  remove PlasmaCore as an import

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19289?vs=52493=52497

BRANCH
  modernize-devicenotifier-settings (branched from master)

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

AFFECTED FILES
  applets/devicenotifier/package/contents/ui/configGeneral.qml

To: filipf, #vdg, #plasma, ngraham, davidedmundson
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19289: [DeviceNotifier] Port settings to QQC2 and Kirigami

2019-02-24 Thread Nathaniel Graham 
ngraham added inline comments.

INLINE COMMENTS

> filipf wrote in configGeneral.qml:23
> Is PlasmaCore also needed here?

Looks like it's not used anywhere, so I'd say no.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  modernize-devicenotifier-settings (branched from master)

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

To: filipf, #vdg, #plasma, ngraham, davidedmundson
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19289: [DeviceNotifier] Port settings to QQC2 and Kirigami

2019-02-24 Thread Filip Fila 
filipf added inline comments.

INLINE COMMENTS

> configGeneral.qml:23
>  
>  import org.kde.plasma.core 2.0 as PlasmaCore
>  

Is PlasmaCore also needed here?

REPOSITORY
  R120 Plasma Workspace

BRANCH
  modernize-devicenotifier-settings (branched from master)

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

To: filipf, #vdg, #plasma, ngraham, davidedmundson
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19289: [DeviceNotifier] Port settings to QQC2 and Kirigami

2019-02-24 Thread Filip Fila 
filipf added a comment.


  In D19289#418964 , @davidedmundson 
wrote:
  
  > Out of curiosity, did you need the exclusive group. 
  >  In theory RadioButton should do it automatically when we share a parent, 
I'm curious if FormLayout breaks that.
  
  
  You're right, it wasn't necessary; I was following the original code too 
closely.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  modernize-devicenotifier-settings (branched from master)

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

To: filipf, #vdg, #plasma, ngraham, davidedmundson
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19289: [DeviceNotifier] Port settings to QQC2 and Kirigami

2019-02-24 Thread Filip Fila 
filipf updated this revision to Diff 52493.
filipf added a comment.


  ButtonGroup is not needed

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19289?vs=52489=52493

BRANCH
  modernize-devicenotifier-settings (branched from master)

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

AFFECTED FILES
  applets/devicenotifier/package/contents/ui/configGeneral.qml

To: filipf, #vdg, #plasma, ngraham, davidedmundson
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19289: [DeviceNotifier] Port settings to QQC2 and Kirigami

2019-02-24 Thread David Edmundson 
davidedmundson added a comment.


  Out of curiosity, did you need the exclusive group. 
  In theory RadioButton should do it automatically when we share a parent, I'm 
curious if FormLayout breaks that.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  modernize-devicenotifier-settings (branched from master)

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

To: filipf, #vdg, #plasma, ngraham, davidedmundson
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19289: [DeviceNotifier] Port settings to QQC2 and Kirigami

2019-02-24 Thread Nathaniel Graham 
ngraham accepted this revision.
ngraham added a comment.


  Nice work!

REPOSITORY
  R120 Plasma Workspace

BRANCH
  modernize-devicenotifier-settings (branched from master)

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

To: filipf, #vdg, #plasma, ngraham, davidedmundson
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19289: [DeviceNotifier] Port settings to QQC2 and Kirigami

2019-02-24 Thread Filip Fila 
filipf created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
filipf requested review of this revision.

REVISION SUMMARY
  Settings are ported to QtQuickControls 2 and now use a Kirigami Form Layout.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  modernize-devicenotifier-settings (branched from master)

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

AFFECTED FILES
  applets/devicenotifier/package/contents/ui/configGeneral.qml

To: filipf
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart