-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
keeping the timer screen on with timer running #508
base: master
Are you sure you want to change the base?
keeping the timer screen on with timer running #508
Conversation
|
||
private var idleTimer: Timer? | ||
private var defaultBrightnessLevel: CGFloat = UIScreen.main.brightness | ||
private let dimmedBrightnessLevel: CGFloat = 0.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this should be a function of the default brightness level? If the user has brightness turned low this could end up increasing it, right?
|
||
|
||
private var idleTimer: Timer? | ||
private var defaultBrightnessLevel: CGFloat = UIScreen.main.brightness |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably capture the current value as late as possible (i.e. just before changing it), to minimize surprising behavior if the user changes brightness while on this screen?
@@ -90,6 +98,16 @@ class TimerViewController: UIViewController { | |||
resetButton.setTitle("Reset", for: .normal) | |||
} | |||
|
|||
override func viewWillDisappear(_ animated: Bool) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to when leaving the screen, should we disable the brightness override when the app is moved to background?
@@ -125,6 +147,8 @@ class TimerViewController: UIViewController { | |||
self.timer = nil | |||
self.timingSince = nil | |||
} | |||
|
|||
resetIdleTimer() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lifecycle of "when we disable the screen" is a little surprising to me. In particular, we enter "do not sleep" mode when you start the timer, but do not leave it when you stop the timer. I'd suggest either enabling it as soon as the user enters the screen, or disabling it when the timer is not running.
8cb1085
to
91c96c2
Compare
keep screen on, albeit dimmed, with active timer on timer screen
fixes: #268