D25947: Add applet for controlling Night Color

[Yuri Chornoivan]

yurchor added a comment.


  In D25947#592565 , @victorr wrote:
  
  > There is a small typo.
  >  You need to replace the quotation marks.
  >  Attached a patch.
  >  F7879213: kdeplasma-addons-5.17.80-fix.patch 

  
  
  Fixed.
  
https://commits.kde.org/kdeplasma-addons/e76ed74f056bdbbe69c2242b75aa77624f7e471b

REPOSITORY
  R114 Plasma Addons

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

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

D25947: Add applet for controlling Night Color

[Victor Ryzhykh]

victorr added a comment.


  There is a small typo.
  You need to replace the quotation marks.
  Attached a patch.
  F7879213: kdeplasma-addons-5.17.80-fix.patch 


REPOSITORY
  R114 Plasma Addons

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

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

D25947: Add applet for controlling Night Color

[Vlad Zahorodnii]

This revision was automatically updated to reflect the committed changes.
Closed by commit R114:a18bc820ca90: Add applet for controlling Night Color 
(authored by zzag).

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25947?vs=72987=72988

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

AFFECTED FILES
  applets/CMakeLists.txt
  applets/nightcolor/CMakeLists.txt
  applets/nightcolor/Messages.sh
  applets/nightcolor/package/contents/ui/main.qml
  applets/nightcolor/package/metadata.desktop
  applets/nightcolor/plugin/CMakeLists.txt
  applets/nightcolor/plugin/inhibitor.cpp
  applets/nightcolor/plugin/inhibitor.h
  applets/nightcolor/plugin/monitor.cpp
  applets/nightcolor/plugin/monitor.h
  applets/nightcolor/plugin/monitor_p.h
  applets/nightcolor/plugin/plugin.cpp
  applets/nightcolor/plugin/plugin.h
  applets/nightcolor/plugin/qmldir

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

D25947: Add applet for controlling Night Color

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> zzag wrote in main.qml:98
> Could you please explain why it needs to be in an else?

I assumed it was trying to remove the action created here:
plasmoid.setAction(..."configure");

I see now that "configure" on the setAction is the icon name and this is 
removing a different action

REPOSITORY
  R114 Plasma Addons

BRANCH
  arcpatch-D25947

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

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

D25947: Add applet for controlling Night Color

[Vlad Zahorodnii]

zzag updated this revision to Diff 72987.
zzag added a comment.


  Add braces.

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25947?vs=71537=72987

BRANCH
  arcpatch-D25947

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

AFFECTED FILES
  applets/CMakeLists.txt
  applets/nightcolor/CMakeLists.txt
  applets/nightcolor/Messages.sh
  applets/nightcolor/package/contents/ui/main.qml
  applets/nightcolor/package/metadata.desktop
  applets/nightcolor/plugin/CMakeLists.txt
  applets/nightcolor/plugin/inhibitor.cpp
  applets/nightcolor/plugin/inhibitor.h
  applets/nightcolor/plugin/monitor.cpp
  applets/nightcolor/plugin/monitor.h
  applets/nightcolor/plugin/monitor_p.h
  applets/nightcolor/plugin/plugin.cpp
  applets/nightcolor/plugin/plugin.h
  applets/nightcolor/plugin/qmldir

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

D25947: Add applet for controlling Night Color

[Vlad Zahorodnii]

zzag added a comment.


  > Does it make sense to dump the C++ classes in libcolorcorrect?
  
  I think it will be better to keep them here for now. Once somebody needs one 
of them, we can move them to libcolorcorrect (bad name).

INLINE COMMENTS

> davidedmundson wrote in main.qml:98
> Should
> 
>   plasmoid.removeAction("configure");
> 
> be in an else?

Could you please explain why it needs to be in an else?

REPOSITORY
  R114 Plasma Addons

BRANCH
  nightcolor-applet

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

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

D25947: Add applet for controlling Night Color

[David Edmundson]

davidedmundson accepted this revision.
davidedmundson added a comment.


  Does it make sense to dump the C++ classes in libcolorcorrect?
  
  It has an import for use client side usage for the KCM, his is just another 
client, especially inhibitor.

INLINE COMMENTS

> main.qml:96
> +Component.onCompleted: {
> +if (KCMShell.authorize("kcm_nightcolor.desktop").length > 0)
> +plasmoid.setAction("KCMNightColor", i18n("Configure Night 
> Color..."), "configure");

I don't think we have an official QML coding guideline, but convention 
throughout plasma has been to put {} for single line if statements.

> main.qml:98
> +plasmoid.setAction("KCMNightColor", i18n("Configure Night 
> Color..."), "configure");
> +plasmoid.removeAction("configure");
> +}

Should

  plasmoid.removeAction("configure");

be in an else?

REPOSITORY
  R114 Plasma Addons

BRANCH
  nightcolor-applet

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

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

D25947: Add applet for controlling Night Color

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  UI and functionality seem spot-on to me!

REPOSITORY
  R114 Plasma Addons

BRANCH
  nightcolor-applet

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

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

D25947: Add applet for controlling Night Color

[Vlad Zahorodnii]

zzag updated this revision to Diff 71537.
zzag added a comment.


  Nate's suggestion.

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25947?vs=71479=71537

BRANCH
  nightcolor-applet

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

AFFECTED FILES
  applets/CMakeLists.txt
  applets/nightcolor/CMakeLists.txt
  applets/nightcolor/Messages.sh
  applets/nightcolor/package/contents/ui/main.qml
  applets/nightcolor/package/metadata.desktop
  applets/nightcolor/plugin/CMakeLists.txt
  applets/nightcolor/plugin/inhibitor.cpp
  applets/nightcolor/plugin/inhibitor.h
  applets/nightcolor/plugin/monitor.cpp
  applets/nightcolor/plugin/monitor.h
  applets/nightcolor/plugin/monitor_p.h
  applets/nightcolor/plugin/plugin.cpp
  applets/nightcolor/plugin/plugin.h
  applets/nightcolor/plugin/qmldir

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

D25947: Add applet for controlling Night Color

[Nathaniel Graham]

ngraham added a comment.


  Thanks, that fixed it. I haven't noticed any shifting of the plasmoid in the 
system tray, but if you want to minimize that, you can add 
`X-Plasma-NotificationAreaCategory=SystemServices` to the `metadata.desktop` 
file. See D11352  for more details.

REPOSITORY
  R114 Plasma Addons

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

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

D25947: Add applet for controlling Night Color

[Vlad Zahorodnii]

zzag updated this revision to Diff 71479.
zzag added a comment.


  Revert applet visibility changes.
  
  Unfortunately, this may cause unnecessary shuffling of system tray items.

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25947?vs=71477=71479

BRANCH
  nightcolor-applet

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

AFFECTED FILES
  applets/CMakeLists.txt
  applets/nightcolor/CMakeLists.txt
  applets/nightcolor/Messages.sh
  applets/nightcolor/package/contents/ui/main.qml
  applets/nightcolor/package/metadata.desktop
  applets/nightcolor/plugin/CMakeLists.txt
  applets/nightcolor/plugin/inhibitor.cpp
  applets/nightcolor/plugin/inhibitor.h
  applets/nightcolor/plugin/monitor.cpp
  applets/nightcolor/plugin/monitor.h
  applets/nightcolor/plugin/monitor_p.h
  applets/nightcolor/plugin/plugin.cpp
  applets/nightcolor/plugin/plugin.h
  applets/nightcolor/plugin/qmldir

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

D25947: Add applet for controlling Night Color

[Nathaniel Graham]

ngraham added a comment.


  Hmm, that last change broke clicking on the applet. Now it does nothing. +1 
for the general ideal behind it though.

REPOSITORY
  R114 Plasma Addons

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

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

D25947: Add applet for controlling Night Color

[Vlad Zahorodnii]

zzag updated this revision to Diff 71477.
zzag added a comment.


  Display the applet in the system tray when Night Color is inhibited.

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25947?vs=71474=71477

BRANCH
  nightcolor-applet

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

AFFECTED FILES
  applets/CMakeLists.txt
  applets/nightcolor/CMakeLists.txt
  applets/nightcolor/Messages.sh
  applets/nightcolor/package/contents/ui/main.qml
  applets/nightcolor/package/metadata.desktop
  applets/nightcolor/plugin/CMakeLists.txt
  applets/nightcolor/plugin/inhibitor.cpp
  applets/nightcolor/plugin/inhibitor.h
  applets/nightcolor/plugin/monitor.cpp
  applets/nightcolor/plugin/monitor.h
  applets/nightcolor/plugin/monitor_p.h
  applets/nightcolor/plugin/plugin.cpp
  applets/nightcolor/plugin/plugin.h
  applets/nightcolor/plugin/qmldir

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

D25947: Add applet for controlling Night Color

[Nathaniel Graham]

ngraham accepted this revision as: VDG.
ngraham added a comment.


  UI and interactivity seems spot-on to me now.

REPOSITORY
  R114 Plasma Addons

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

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

D25947: Add applet for controlling Night Color

[Vlad Zahorodnii]

zzag updated this revision to Diff 71474.
zzag added a comment.


  Nate's suggestion.

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25947?vs=71438=71474

BRANCH
  nightcolor-applet

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

AFFECTED FILES
  applets/CMakeLists.txt
  applets/nightcolor/CMakeLists.txt
  applets/nightcolor/Messages.sh
  applets/nightcolor/package/contents/ui/main.qml
  applets/nightcolor/package/metadata.desktop
  applets/nightcolor/plugin/CMakeLists.txt
  applets/nightcolor/plugin/inhibitor.cpp
  applets/nightcolor/plugin/inhibitor.h
  applets/nightcolor/plugin/monitor.cpp
  applets/nightcolor/plugin/monitor.h
  applets/nightcolor/plugin/monitor_p.h
  applets/nightcolor/plugin/plugin.cpp
  applets/nightcolor/plugin/plugin.h
  applets/nightcolor/plugin/qmldir

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

D25947: Add applet for controlling Night Color

[Nathaniel Graham]

ngraham added a comment.


  This is fantastic! My on UI suggestion is to only the applet in the system 
tray (when using "Auto" visibility) only when Night Color is actually activated 
or activating. People who want it in their tray all the time can manually set 
it to "shown". The one exception would be that when Night Color is is using the 
"Constant" mode, the applet should always be visible in the tray when using 
"Auto" visibility.

REPOSITORY
  R114 Plasma Addons

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

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

D25947: Add applet for controlling Night Color

[Vlad Zahorodnii]

zzag updated this revision to Diff 71438.
zzag added a comment.


  Delete stray include.

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25947?vs=71433=71438

BRANCH
  nightcolor-applet

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

AFFECTED FILES
  applets/CMakeLists.txt
  applets/nightcolor/CMakeLists.txt
  applets/nightcolor/Messages.sh
  applets/nightcolor/package/contents/ui/main.qml
  applets/nightcolor/package/metadata.desktop
  applets/nightcolor/plugin/CMakeLists.txt
  applets/nightcolor/plugin/inhibitor.cpp
  applets/nightcolor/plugin/inhibitor.h
  applets/nightcolor/plugin/monitor.cpp
  applets/nightcolor/plugin/monitor.h
  applets/nightcolor/plugin/monitor_p.h
  applets/nightcolor/plugin/plugin.cpp
  applets/nightcolor/plugin/plugin.h
  applets/nightcolor/plugin/qmldir

To: zzag, #plasma
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

D25947: Add applet for controlling Night Color

[Vlad Zahorodnii]

zzag added a comment.


  This patch depends on D25946  and D25786 
.

REPOSITORY
  R114 Plasma Addons

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

To: zzag, #plasma
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

D25947: Add applet for controlling Night Color

[Vlad Zahorodnii]

zzag created this revision.
zzag added a reviewer: Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
zzag requested review of this revision.

REVISION SUMMARY
  The new applet allows to temporarily disable Night Color, for example if
  one wants to edit a video or an image.
  
  BUG: 400418

REPOSITORY
  R114 Plasma Addons

BRANCH
  nightcolor-applet

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

AFFECTED FILES
  applets/CMakeLists.txt
  applets/nightcolor/CMakeLists.txt
  applets/nightcolor/Messages.sh
  applets/nightcolor/package/contents/ui/main.qml
  applets/nightcolor/package/metadata.desktop
  applets/nightcolor/plugin/CMakeLists.txt
  applets/nightcolor/plugin/inhibitor.cpp
  applets/nightcolor/plugin/inhibitor.h
  applets/nightcolor/plugin/monitor.cpp
  applets/nightcolor/plugin/monitor.h
  applets/nightcolor/plugin/monitor_p.h
  applets/nightcolor/plugin/plugin.cpp
  applets/nightcolor/plugin/plugin.h
  applets/nightcolor/plugin/qmldir

To: zzag, #plasma
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