D24733: feedback kcm: Make it about Plasma

This revision was automatically updated to reflect the committed changes. Closed by commit R120:2dd40cfb21b4: feedback kcm: Make it about Plasma (authored by apol). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24733?vs=68620=68621 REVISION DETAIL

D24733: feedback kcm: Make it about Plasma

ngraham accepted this revision. ngraham added a comment. All right, let's fix it there. REPOSITORY R120 Plasma Workspace BRANCH nokillswitch REVISION DETAIL https://phabricator.kde.org/D24733 To: apol, #plasma, vkrause, davidedmundson, ngraham Cc: plasma-devel, LeGast00n,

D24733: feedback kcm: Make it about Plasma

apol added a comment. I integrated David's idea to move this patch forward, but I think here we should look into move this text elsewhere: https://cgit.kde.org/kuserfeedback.git/tree/src/provider/core/feedbackconfiguicontroller.cpp#n188 In this text we are talking about why to enable

D24733: feedback kcm: Make it about Plasma

apol updated this revision to Diff 68620. apol added a comment. Integrate David's feedback REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24733?vs=68184=68620 BRANCH nokillswitch REVISION DETAIL https://phabricator.kde.org/D24733 AFFECTED

D24733: feedback kcm: Make it about Plasma

apol marked an inline comment as done. apol added inline comments. INLINE COMMENTS > davidedmundson wrote in main.qml:116 > I don't understand this top line Yeah... feedbackController.telemetryDescription() function depends on applicationName, but since it's a function we don't have a way to

D24733: feedback kcm: Make it about Plasma

davidedmundson added a comment. I agree with Nate's comment, but lets get this in and then tackle that there. I just one QML line that I don't follow, then lets ship it INLINE COMMENTS > main.qml:41 > > -QQC2.CheckBox { > -Layout.topMargin: Kirigami.Units.gridUnit

D24733: feedback kcm: Make it about Plasma

apol added a comment. Oh, I see what you mean, this text is entirely coming from KUserFeedback. Maybe it's something we can tackle there: https://phabricator.kde.org/source/kuserfeedback/browse/master/src/provider/core/feedbackconfiguicontroller.cpp$187 REPOSITORY R120 Plasma Workspace

D24733: feedback kcm: Make it about Plasma

ngraham added a comment. The introductory "We make plasma for you..." text is still inappropriately shown in the explanatory text for the Disabled state rather than on top:F7617232: demo-2019-10-17_12.31.04.webm REPOSITORY R120 Plasma Workspace

D24733: feedback kcm: Make it about Plasma

apol updated this revision to Diff 68184. apol added a comment. update the description with the applicationName change REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24733?vs=68181=68184 BRANCH nokillswitch REVISION DETAIL

D24733: feedback kcm: Make it about Plasma

ngraham added a comment. Yay, that update fixed the issue, phew! However the strings are still a little garbled: F7616876: Screenshot_20191017_115423.png REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D24733 To:

D24733: feedback kcm: Make it about Plasma

apol updated this revision to Diff 68181. apol added a comment. Ehm... REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24733?vs=68153=68181 BRANCH nokillswitch REVISION DETAIL https://phabricator.kde.org/D24733 AFFECTED FILES

D24733: feedback kcm: Make it about Plasma

ngraham added a comment. It already is: F7616320: Screenshot_20191017_101011.png REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D24733 To: apol, #plasma, vkrause, davidedmundson, ngraham Cc: plasma-devel, LeGast00n,

D24733: feedback kcm: Make it about Plasma

apol added a comment. In D24733#549186 , @ngraham wrote: > In D24733#549185 , @apol wrote: > > > It's changed in the last iteration of the patch. You probably enabled the kill switch on the present

D24733: feedback kcm: Make it about Plasma

ngraham added a comment. In D24733#549185 , @apol wrote: > It's changed in the last iteration of the patch. You probably enabled the kill switch on the present version of the kcm. Yeah, I did. So now how do I turn it off?  Is there a

D24733: feedback kcm: Make it about Plasma

apol added a comment. It's changed in the last iteration of the patch. You probably enabled the kill switch on the present version of the kcm. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D24733 To: apol, #plasma, vkrause, davidedmundson, ngraham Cc:

D24733: feedback kcm: Make it about Plasma

ngraham added a comment. Ok, now I see this: F7616194: Screenshot_20191017_095308.png Is that caused by a change here, or did something truly change on my system in the last hour? REPOSITORY R120 Plasma Workspace REVISION DETAIL

D24733: feedback kcm: Make it about Plasma

apol updated this revision to Diff 68153. apol marked an inline comment as done. apol added a comment. Remove duplicated text, disable when the kill switch is on REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24733?vs=68135=68153 BRANCH

D24733: feedback kcm: Make it about Plasma

ngraham added a comment. Indeed, this is much nicer. The explanatory text for "Disabled" appears to duplicate some of the text above it though: F7615296: Screenshot_20191017_074048.png REPOSITORY R120 Plasma Workspace REVISION DETAIL

D24733: feedback kcm: Make it about Plasma

davidedmundson added a comment. +++ INLINE COMMENTS > feedback.h:31 > > -Q_PROPERTY(bool feedbackEnabled READ feedbackEnabled WRITE > setFeedbackEnabled NOTIFY feedbackEnabledChanged) > Q_PROPERTY(int plasmaFeedbackLevel READ plasmaFeedbackLevel WRITE > setPlasmaFeedbackLevel

D24733: feedback kcm: Make it about Plasma

apol created this revision. apol added reviewers: Plasma, vkrause, davidedmundson, ngraham. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. apol requested review of this revision. REVISION SUMMARY At the moment we were merging two different concepts into a same KCM and