Skip to content
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

Add a brightness offset option #5

Merged
merged 1 commit into from
Oct 21, 2019

Conversation

matej
Copy link
Contributor

@matej matej commented Oct 16, 2019

A slider that can offset the synced value by +/- 50%. This enables the user to manually sync up the apparent brightness when the displays have a different brightness range.

Also:

  • parametrizes the update interval
  • schedules the updated timer in the common runloop mode so updates happen while interacting with the app
  • removes empty whitespace

Screenshot 2019-10-14 at 12 43 21

Fixes #4

A slider that can offset the synced value by +/- 50%. This enables the user to manually sync up the apparent brightness when the displays have a different brightness range.

Also:
- parametrizes the update interval
- schedules the updated timer in the common runloop mode so updates happen while interacting with the app
- removes empty whitespace


let brightnessOffsetKey = "BSBrightnessOffset"
var brightnessOffset: Double {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps having a different default value than 0 would be more appropriate. I'm using about -20% / - 25% to match up my MBP and the LG. On the other hand, this means the display is automatically deprived of it's max brightness, which somebody might not like. So 0 might be the safer option.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll go with 0. This should also be the correct setting for users still on Mojave.

let menu = NSMenu()
menu.addItem(statusIndicator)
menu.addItem(NSMenuItem.separator())


menu.addItem(NSMenuItem(title: "Brightness Offset", action: nil, keyEquivalent: ""))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open for suggestions for a better name for this. 🤷‍♂

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, can't think of any. I also wish I had a better name for the app than “Brightness Sync”...

@@ -88,7 +107,8 @@ class AppDelegate: NSObject, NSApplicationDelegate {
self.lastSaneBrightness = newBrightness
}
}

RunLoop.current.add(syncTimer!, forMode: .common)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enables brightness adjustments while interacting with the slider.

container.addSubview(statusSlider)
statusSlider.translatesAutoresizingMaskIntoConstraints = false

statusSlider.leadingAnchor.constraint(equalTo: container.leadingAnchor, constant: 20).isActive = true
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better way to match up the slider to the text, than using a magic number (20) here?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I searched for it, but can't find anything. I made a screenshot of the system menu Volume slider and counted the pixels, so I'm going to just copy those measurements 😬

@OCJvanDijk OCJvanDijk merged commit 3830d29 into OCJvanDijk:master Oct 21, 2019
@OCJvanDijk
Copy link
Owner

I merged this and I'll make some small adjustments on top.
Thanks a lot for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Brightness offset option
2 participants