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

2019-12-13 Thread Nathaniel Graham 
This revision was automatically updated to reflect the committed changes.
Closed by commit R119:2a42a15cdf68: [KCMs/Night Color] Make activation criteria 
and times more obvious (authored by ngraham).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25920?vs=71461&id=71470

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

AFFECTED FILES
  kcms/nightcolor/package/contents/ui/LocationsAutoView.qml
  kcms/nightcolor/package/contents/ui/LocationsFixedView.qml
  kcms/nightcolor/package/contents/ui/TimingsView.qml
  kcms/nightcolor/package/contents/ui/main.qml

To: ngraham, #vdg, #plasma, davidedmundson, romangg, filipf
Cc: filipf, abetts, romangg, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, ahiemstra, mart

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

2019-12-13 Thread Filip Fila 
filipf accepted this revision.
filipf added a comment.


  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.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  polish-nightcolor-ui (branched from master)

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

To: ngraham, #vdg, #plasma, davidedmundson, romangg, filipf
Cc: filipf, abetts, romangg, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, ahiemstra, mart

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

2019-12-13 Thread Nathaniel Graham 
ngraham updated this revision to Diff 71461.
ngraham added a comment.


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

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25920?vs=71460&id=71461

BRANCH
  polish-nightcolor-ui (branched from master)

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

AFFECTED FILES
  kcms/nightcolor/package/contents/ui/LocationsAutoView.qml
  kcms/nightcolor/package/contents/ui/LocationsFixedView.qml
  kcms/nightcolor/package/contents/ui/TimingsView.qml
  kcms/nightcolor/package/contents/ui/main.qml

To: ngraham, #vdg, #plasma, davidedmundson, romangg
Cc: abetts, romangg, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, ahiemstra, mart

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

2019-12-13 Thread Nathaniel Graham 
ngraham updated this revision to Diff 71460.
ngraham added a comment.


  Address VDG review comments from here

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25920?vs=71430&id=71460

BRANCH
  polish-nightcolor-ui (branched from master)

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

AFFECTED FILES
  kcms/nightcolor/package/contents/ui/LocationsAutoView.qml
  kcms/nightcolor/package/contents/ui/LocationsFixedView.qml
  kcms/nightcolor/package/contents/ui/TimingsView.qml
  kcms/nightcolor/package/contents/ui/main.qml

To: ngraham, #vdg, #plasma, davidedmundson, romangg
Cc: abetts, romangg, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, ahiemstra, mart

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

2019-12-13 Thread Nathaniel Graham 
ngraham added a comment.


  In D25920#576966 , @abetts wrote:
  
  > 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 feat. 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 whatsoever. The point of the manual location 
entry 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.
  
  > 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
  > 
  > [X] Gradual activation
  > [X] 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.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  polish-nightcolor-ui (branched from master)

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

To: ngraham, #vdg, #plasma, davidedmundson, romangg
Cc: abetts, romangg, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, ahiemstra, mart

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

2019-12-13 Thread Andres Betts 
abetts added a comment.


  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
  
  [X] Gradual activation
  [X] Change colors gradually

REPOSITORY
  R119 Plasma Desktop

BRANCH
  polish-nightcolor-ui (branched from master)

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

To: ngraham, #vdg, #plasma, davidedmundson, romangg
Cc: abetts, romangg, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, ahiemstra, mart

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

2019-12-13 Thread Nathaniel Graham 
ngraham updated this revision to Diff 71430.
ngraham added a comment.


  Put the separator in the formlayout for more simplicity

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25920?vs=71429&id=71430

BRANCH
  polish-nightcolor-ui (branched from master)

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

AFFECTED FILES
  kcms/nightcolor/package/contents/ui/LocationsAutoView.qml
  kcms/nightcolor/package/contents/ui/TimingsView.qml
  kcms/nightcolor/package/contents/ui/main.qml

To: ngraham, #vdg, #plasma, davidedmundson, romangg
Cc: romangg, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

2019-12-13 Thread Nathaniel Graham 
ngraham updated this revision to Diff 71429.
ngraham added a comment.


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

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25920?vs=71428&id=71429

BRANCH
  polish-nightcolor-ui (branched from master)

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

AFFECTED FILES
  kcms/nightcolor/package/contents/ui/LocationsAutoView.qml
  kcms/nightcolor/package/contents/ui/TimingsView.qml
  kcms/nightcolor/package/contents/ui/main.qml

To: ngraham, #vdg, #plasma, davidedmundson, romangg
Cc: romangg, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

2019-12-13 Thread Nathaniel Graham 
ngraham updated this revision to Diff 71428.
ngraham added a comment.


  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

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25920?vs=71419&id=71428

BRANCH
  polish-nightcolor-ui (branched from master)

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

AFFECTED FILES
  kcms/nightcolor/package/contents/ui/LocationsAutoView.qml
  kcms/nightcolor/package/contents/ui/TimingsView.qml
  kcms/nightcolor/package/contents/ui/main.qml

To: ngraham, #vdg, #plasma, davidedmundson, romangg
Cc: romangg, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

2019-12-13 Thread Nathaniel Graham 
ngraham added a comment.


  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.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  polish-nightcolor-ui (branched from master)

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

To: ngraham, #vdg, #plasma, davidedmundson, romangg
Cc: romangg, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

2019-12-13 Thread Roman Gilg 
romangg added a comment.


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

REPOSITORY
  R119 Plasma Desktop

BRANCH
  polish-nightcolor-ui (branched from master)

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

To: ngraham, #vdg, #plasma, davidedmundson, romangg
Cc: romangg, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

2019-12-13 Thread Nathaniel Graham 
ngraham updated this revision to Diff 71419.
ngraham added a comment.


  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.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25920?vs=71399&id=71419

BRANCH
  polish-nightcolor-ui (branched from master)

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

AFFECTED FILES
  kcms/nightcolor/package/contents/ui/LocationsAutoView.qml
  kcms/nightcolor/package/contents/ui/TimingsView.qml
  kcms/nightcolor/package/contents/ui/main.qml

To: ngraham, #vdg, #plasma, davidedmundson, romangg
Cc: romangg, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

2019-12-12 Thread Nathaniel Graham 
ngraham updated this revision to Diff 71399.
ngraham added a comment.


  Improve begin/end strings again

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25920?vs=71387&id=71399

BRANCH
  polish-nightcolor-ui (branched from master)

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

AFFECTED FILES
  kcms/nightcolor/package/contents/ui/TimingsView.qml
  kcms/nightcolor/package/contents/ui/main.qml

To: ngraham, #vdg, #plasma, davidedmundson
Cc: romangg, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

2019-12-12 Thread Roman Gilg 
romangg added a comment.


  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 at".

REPOSITORY
  R119 Plasma Desktop

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

To: ngraham, #vdg, #plasma, davidedmundson
Cc: romangg, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

2019-12-12 Thread Nathaniel Graham 
ngraham updated this revision to Diff 71387.
ngraham added a comment.


  Tweak location-related mode strings

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25920?vs=71381&id=71387

BRANCH
  polish-nightcolor-ui (branched from master)

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

AFFECTED FILES
  kcms/nightcolor/package/contents/ui/TimingsView.qml
  kcms/nightcolor/package/contents/ui/main.qml

To: ngraham, #vdg, #plasma, davidedmundson
Cc: romangg, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

2019-12-12 Thread Nathaniel Graham 
ngraham added a comment.


  In D25920#576403 , @romangg wrote:
  
  > 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!

REPOSITORY
  R119 Plasma Desktop

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

To: ngraham, #vdg, #plasma, davidedmundson
Cc: romangg, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

2019-12-12 Thread Roman Gilg 
romangg added a comment.


  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`?

REPOSITORY
  R119 Plasma Desktop

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

To: ngraham, #vdg, #plasma, davidedmundson
Cc: romangg, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

2019-12-12 Thread Nathaniel Graham 
ngraham updated this revision to Diff 71381.
ngraham added a comment.


  - Disclose location usage
  - Use better string for manual location mode

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25920?vs=71359&id=71381

BRANCH
  polish-nightcolor-ui (branched from master)

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

AFFECTED FILES
  kcms/nightcolor/package/contents/ui/TimingsView.qml
  kcms/nightcolor/package/contents/ui/main.qml

To: ngraham, #vdg, #plasma, davidedmundson
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

2019-12-12 Thread Nathaniel Graham 
ngraham created this revision.
ngraham added reviewers: VDG, Plasma, davidedmundson.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
ngraham requested review of this revision.

REVISION 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
  F7814072: demo-2019-12-12_15.13.35.webm 

REPOSITORY
  R119 Plasma Desktop

BRANCH
  polish-nightcolor-ui (branched from master)

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

AFFECTED FILES
  kcms/nightcolor/package/contents/ui/TimingsView.qml
  kcms/nightcolor/package/contents/ui/main.qml

To: ngraham, #vdg, #plasma, davidedmundson
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart