[KCMs/Night Color] Make activation criteria and times more obvious
ClosedPublic

Authored by ngraham on Dec 12 2019, 2:15 PM.

Details

Summary

This patch polishes the Night Color UI to make it more obvious what the feature does,
what the activation triggers are and mean, and when activation will start and end.

Test Plan

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ngraham created this revision.Dec 12 2019, 2:15 PM
Restricted Application added a project: Plasma. · View Herald TranscriptDec 12 2019, 2:15 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ngraham requested review of this revision.Dec 12 2019, 2:15 PM
ngraham updated this revision to Diff 71381.Dec 12 2019, 5:28 PM
  • Disclose location usage
  • Use better string for manual location mode
ngraham edited the test plan for this revision. (Show Details)Dec 12 2019, 5:29 PM

Yea, new strings are better. I like the overall look of the KCM in this redesign. Maybe the description at the top could be in some kind of "info" box or in another way better styled?

For the strings: Sunset/rise at current location?

Yea, new strings are better. I like the overall look of the KCM in this redesign.

Thanks!

Maybe the description at the top could be in some kind of "info" box or in another way better styled?

That's slightly out of scope for the current patch, and I kind of like it floating there where it currently. I tried using a KMessageWidget and a frame and it didn't feel right. Maybe we could make it wider?

For the strings: Sunset/rise at current location?

Good idea!

ngraham updated this revision to Diff 71387.Dec 12 2019, 5:57 PM

Tweak location-related mode strings

romangg added a comment.EditedDec 12 2019, 8:25 PM

One more thing I forgot: strings "Turn on at" and "Fully on at" and so on are too unclear for me and also sound a bit dumb. ;) Proposal: "Begin shift at" and "Fully engaged on at".

The "engaged" does not work after I thought about it because of the off line where it would need to say then awkwardly "Fully disengaged at".

ngraham updated this revision to Diff 71399.Dec 12 2019, 9:24 PM

Improve begin/end strings again

romangg accepted this revision.Dec 13 2019, 6:25 AM
This revision is now accepted and ready to land.Dec 13 2019, 6:25 AM
ngraham updated this revision to Diff 71419.Dec 13 2019, 10:16 AM

Polish everything even more to ensure a consistent top-to-bottom workflow. To do this
I had to do a lot of refactoring that also has the side effect of making the UI a lot
more declarative.

ngraham edited the test plan for this revision. (Show Details)Dec 13 2019, 10:16 AM

Sorry, but I don't like how the text is "hanging in the air" and how it is jumping around when changing the setting.

This idea was to have a top-to-bottom flow. First you turn it on, then you select the activation mode, then you manipulate the controls specific to that mode (if applicable), then you see the times that result from those settings. I think it makes logical sense. If you have any suggestions for how to improve the presentation, I'm all ears. Also interested in VDG feedback.

davidedmundson accepted this revision.Dec 13 2019, 1:04 PM
ngraham updated this revision to Diff 71428.Dec 13 2019, 1:17 PM

Put the labels back in a FormLayout to align the times, but don't align the formlayout
itself with the top one or else the whole thing is too far over to the left

ngraham updated this revision to Diff 71429.Dec 13 2019, 1:18 PM

Restore original import order for timings view now that we don't need layouts anymore

ngraham updated this revision to Diff 71430.Dec 13 2019, 1:20 PM

Put the separator in the formlayout for more simplicity

abetts added a subscriber: abetts.EditedDec 13 2019, 4:38 PM

Suggestions:

"To reduce eye strain, Night Color makes the colors on the screen warmer at the time of your choosing"

Change to:

"Night Color makes the screen colors warmer to reduce eye strain"

"Night color temperature"

Change to:

"Temperature"

Activation

  • Sunset/sunrise at current location
  • Sunset/sunrise at manual location
  • Manual times
  • Always on

Change to:

Time:

  • Sunset to sunrise at your location
  • Sunset to sunrise at different location
  • Custom time
  • Always on

Instead of having latitude/longitude + Detect Location, swap to Detect Location + Lat/long after.

"Color change begins at XX:XX"

Change to:

Night color begins at XX;XX

Color fully changed, might not be necessary? Too much detail?

"Color begins changing back at XX:XX"

Change to

Night color ends XX:XX

Normal coloration restored by XX:XX, might be too much info.

Since the color changes gradually, maybe we could add an option to enable/disable that function? Something like

  • Gradual activation
  • Change colors gradually
ngraham added a comment.EditedDec 13 2019, 4:49 PM

Suggestions:

"To reduce eye strain, Night Color makes the colors on the screen warmer at the time of your choosing"

Change to:

"Night Color makes the screen colors warmer to reduce eye strain"

That eliminates the "at the time of your choosing", bit, without which I worry that users might mistakenly believe that this feature permanently tints the screen while on, missing the entire point that it does this on;y at certain times of day.

"Night color temperature"

Change to:

"Temperature"

Or maybe even... "Color"?

  • Sunset/sunrise at manual location

    Change to:
  • Sunset to sunrise at different location

Other proposed changes seem fine, but this would make that feature misleading, I fear. My first iteration of this patch used "different location", but upon reflection and consultation with the developers, using a location other than your own makes no sense. The point of the manual location entry feature is to enter your own location without needing to use the geolocator in the case that it's broken or you're very privacy-conscious. There are no valid user use cases to living in Australia and setting the location to Canada. A valid developer use case is living in Ukraine and setting the location to California to test the feature when a Californian user reports a bug. But that's not the primary purpose.

Instead of having latitude/longitude + Detect Location, swap to Detect Location + Lat/long after.

Good idea!

"Color change begins at XX:XX"

Change to:

Night color begins at XX;XX

Color fully changed, might not be necessary? Too much detail?

"Color begins changing back at XX:XX"

Change to

Night color ends XX:XX

Normal coloration restored by XX:XX, might be too much info.

Because of the lengthy transition time, if we just list the time when it starts, people will get confused because they'll say, "hey, this thing says it's on, but I don't see any color change!" because the color has just started changing and has not finished the transition yet.

Since the color changes gradually, maybe we could add an option to enable/disable that function? Something like

  • Gradual activation
  • Change colors gradually

There is, when using manual times mode. In that mode, you can choose the transition time. When using a location-based mode, this isn't user-selectable because the transition time is perfectly coordinated to come on when the sun starts to set and complete when it's dark. It doesn't make sense to let the user override that when in a location-based mode.

ngraham updated this revision to Diff 71460.Dec 13 2019, 4:58 PM

Address VDG review comments from here

ngraham updated this revision to Diff 71461.Dec 13 2019, 5:32 PM

Address other VDG comments from chatroom (add time labels to main formlayout and align)

ngraham edited the test plan for this revision. (Show Details)Dec 13 2019, 5:33 PM
filipf accepted this revision.Dec 13 2019, 7:34 PM
filipf added a subscriber: filipf.

Looks good. I just realized that maybe the message at the top could be an info inline message, but I see how that could be too much.

This revision was automatically updated to reflect the committed changes.