thompsonate / Shifty

☀️ A macOS menu bar app that gives you more control over Night Shift.
http://shifty.natethompson.io
GNU General Public License v3.0
1.24k stars 33 forks source link

Color temperature resets to middle value #27

Closed choco closed 6 years ago

choco commented 6 years ago

I just started today using the non-default color temperature value, but the highest possible value and it already happened twice that it got reset back to the middle value.

thompsonate commented 6 years ago

Do you have steps to reproduce this?

choco commented 6 years ago

Nope, but I noticed that in the power function you manually set the value to 50 if the slider value is 0 which seems wrong? Also I'd probably set the slider value from the strength in the awakeFromNib method, since the slider value in the nib is 50 and a spurious sliderValueChanged may get triggered and reset it to 50 also in that case.

thompsonate commented 6 years ago

What I'm doing in the power function is turning Night Shift on with the color temp at 50% if the power menu item is pressed and slider is at zero. Otherwise, you'd click on the menu item and nothing would happen. Even though Night Shift is on, the color temperature is the same. This shouldn't be causing any problems.

What would cause it to get reset to 50? The slider is set to the Night Shift strength value in menuWillOpen()

choco commented 6 years ago

What I'm doing in the power function is turning Night Shift on with the color temp at 50% if the power menu item is pressed and slider is at zero. Otherwise, you'd click on the menu item and nothing would happen. Even though Night Shift is on, the color temperature is the same. This shouldn't be causing any problems.

The color temperature when Night Shift is at minimum is actually different from when it is disabled (that's clearly visibile on my display at least), that's why it imo it doesn't make lot of sense. We should probably change the logic slidervalue==0 -> night shift disabled, because it is a wrong assumption.

What would cause it to get reset to 50? The slider is set to the Night Shift strength value in menuWillOpen()

Actually that's not enough, shift(isEnabled:) can execute without ever opening the menu in a couple of ways, for example using a shortcut, or through the updateCurrenApp method. To test:

  1. start Shifty
  2. add a disable rule for an app, any app
  3. quit Shifty
  4. start Shifty
  5. focus the app you disabled before
  6. focus any other app which isn't disabled

Following this steps you end up with the strength set to 50. I added a commit with the fix in https://github.com/thompsonate/Shifty/pull/26

thompsonate commented 6 years ago

The color temperature when Night Shift is at minimum is actually different from when it is disabled (that's clearly visibile on my display at least), that's why it imo it doesn't make lot of sense. We should probably change the logic slidervalue==0 -> night shift disabled, because it is a wrong assumption.

You're right. I didn't realize that before. I'll fix it.