D21979: [System Load Viewer] Add a tooltip about the "CPUs separately" option

2019-06-23 Thread Nathaniel Graham
ngraham added a comment.


  It never makes sense to put a tooltip on the wrong control.
  
  The fact that you need to check "CPU Monitor" first to get the "Show CPUs 
separately" option to become enables id visually communicated by the fact that 
it's indented below the other checkbox.
  
  If https://bugreports.qt.io/browse/QTBUG-30801 is the blocker here, I think 
we can safely commit this now (but for the right control) and then once that 
bug is fixed, the tooltip will just automatically work.

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

To: nhiga, #plasma, #vdg, ngraham
Cc: ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart


D21979: [System Load Viewer] Add a tooltip about the "CPUs separately" option

2019-06-22 Thread N. Higa
nhiga added a comment.


  In D21979#484281 , @ngraham wrote:
  
  > Sorry, my mistake. I missed that this tooltip is set on the wrong checkbox. 
It needs to be set on the "CPUs separately" checkbox
  
  
  This is intentional - if we set this on the "CPUs separately" checkbox, 
QTBUG-30801  will make this patch 
not useful, because the tooltip will not be shown when the checkbox is disabled 
(i.e. grayed out), and therefore Jung's issue will not be addressed (see below).
  
  > In Bug 408959 , Jung wrote:
  > The "CPUs separately" box in the system load viewer widget configuration is 
grayed out. There is no indication what I could/should do to be allowed to 
enable that checkbox.  That's at best confusing and at worst frustrating UI 
design.
  
  Another option would be to put this patch on hold and wait for QTBUG-30801 to 
be fixed, and find other ways to let the user know that he/she should choose 
"Compact bar" to enable "CPUs separately".

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

To: nhiga, #plasma, #vdg, ngraham
Cc: ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart


D21979: [System Load Viewer] Add a tooltip about the "CPUs separately" option

2019-06-22 Thread Nathaniel Graham
ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  Sorry, my mistake. I missed that this tooltip is set on the wrong checkbox. 
It needs to be set on the "CPUs separately" checkbox

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

To: nhiga, #plasma, #vdg, ngraham
Cc: ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart


D21979: [System Load Viewer] Add a tooltip about the "CPUs separately" option

2019-06-22 Thread Nathaniel Graham
ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Thank you!
  
  The email address I have on file for you is 
`c822c4f23bca1ea6faac7...@mail.xn--3ds443g`. Is there any chance you can 
provide a different, more human-readable email address?

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

To: nhiga, #plasma, #vdg, ngraham
Cc: ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart


D21979: [System Load Viewer] Add a tooltip about the "CPUs separately" option

2019-06-22 Thread N. Higa
nhiga updated this revision to Diff 60326.
nhiga added a comment.


  Rephrased the tooltip message according to ngraham's suggestion.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21979?vs=60279=60326

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

AFFECTED FILES
  applets/systemloadviewer/package/contents/ui/GeneralSettings.qml

To: nhiga, #plasma, #vdg, ngraham
Cc: ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart


D21979: [System Load Viewer] Add a tooltip about the "CPUs separately" option

2019-06-22 Thread Nathaniel Graham
ngraham requested changes to this revision.
ngraham added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> GeneralSettings.qml:54
> +visible: parent.hovered
> +text: i18n("CPUs can be shown separately when using compact bar 
> monitor")
> +}

Rephrase this in the form of a command (e.g. "Show multiple CPUs separately 
when using compact bar monitor")

REPOSITORY
  R114 Plasma Addons

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

To: nhiga, #plasma, #vdg, ngraham
Cc: ngraham, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart


D21979: [System Load Viewer] Add a tooltip about the "CPUs separately" option

2019-06-22 Thread N. Higa
nhiga created this revision.
nhiga added a project: Plasma.
nhiga requested review of this revision.

REVISION SUMMARY
  There is a "CPUs separately" option which is available when using the 
"Compact bar" style. If "Bar" or "Circular" is used, the "CPUs separately" 
option will not be available even if the user has enabled showing the CPU 
monitor. This may confuse the user.
  
  This patch adds a tooltip to the "CPU monitor" option indicating the user 
that he/she should select "Compact bar" to enable the "CPUs separately" option.
  
  Adding the tooltip to "CPUs separately" instead of "CPU monitor" would be a 
better option, but due to QTBUG-30801, the tooltip will not be shown when the 
checkbox is disabled.
  
  BUG: 408959

TEST PLAN
  1. Open the Settings page for the System Load Viewer applet.
  2. Hover the mouse above "CPU monitor".
  3. A tooltip "CPUs can be shown separately when using compact bar monitor" is 
shown.

REPOSITORY
  R114 Plasma Addons

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

AFFECTED FILES
  applets/systemloadviewer/package/contents/ui/GeneralSettings.qml

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