Differential D21979

[System Load Viewer] Add a tooltip about the "CPUs separately" option
AbandonedPublic
Actions

Authored by nhiga on Jun 22 2019, 6:14 AM.

Details

Reviewers
ngraham
Group Reviewers
Plasma
VDG
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.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
nhiga requested review of this revision.Jun 22 2019, 6:14 AM
nhiga created this revision.
ngraham requested changes to this revision.Jun 22 2019, 9:56 AM
ngraham added reviewers: Plasma, VDG.
ngraham added a subscriber: ngraham.
ngraham added inline comments.
applets/systemloadviewer/package/contents/ui/GeneralSettings.qml
54 ↗(On Diff #60279)

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

This revision now requires changes to proceed.Jun 22 2019, 9:56 AM
nhiga updated this revision to Diff 60326.Jun 22 2019, 11:28 AM

Rephrased the tooltip message according to ngraham's suggestion.

ngraham accepted this revision.Jun 22 2019, 12:33 PM

Thank you!

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

This revision is now accepted and ready to land.Jun 22 2019, 12:33 PM
ngraham requested changes to this revision.Jun 22 2019, 2:31 PM

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 revision now requires changes to proceed.Jun 22 2019, 2:31 PM
nhiga added a comment.Jun 23 2019, 1:07 AM
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".

ngraham added a comment.Jun 23 2019, 10:00 AM

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.

nhiga abandoned this revision.Jun 23 2019, 11:13 AM

Revision Contents
Changeset List

Diff 60326

View Options

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

Loading...