Add automatic switching between Dark Mode and Light

[say25]

Overview

Add automatic switching between Dark Mode and Light Mode or ability to change it programmatically.

Closes #5037

Description

• Add automatic switching between Dark Mode and Light Mode (macOS Mojave Dark Mode).

Release notes

Notes: Add automatic switching between Dark Mode and Light Mode (macOS Mojave Dark Mode).

[say25]

@desktop/engineering not sure where the best place to put the currently commented code would be. We would basically need to subscribe to the electron event systemPreferences.subscribeNotification('AppleInterfaceThemeChangedNotification', updateThemeBasedOnSystem); on app start up (maybe DARWIN conditional). Opening an early PR for early feedback. I also think the electron.d.ts could be slightly broken :/

[shiftkey]

@say25 thanks for opening this up!

not sure where the best place to put the currently commented code would be.

As this is a UI-specific feature, what about a module under app/src/ui/lib? systemPreferences is a main-process specific API, but you could use remote to access it - that's how we've done it for the title bar interaction:

desktop/app/src/ui/window/title-bar.tsx

Lines 55 to 58 in 412959e

	const actionOnDoubleClick = remote.systemPreferences.getUserDefault(
	'AppleActionOnDoubleClick',
	'string'
	)

And like UpdateStore you could define an event emitter interface to bubble up the event to the listeners.

Alternatively, you could have this live in app/src/main-process and it could just emit IPC calls into the renderer.

Inside AppWindow you can see the example of sending messages:

this.window.on('blur', () => this.window.webContents.send('blur'))

And app/src/ui/index.tsx has the example of the subscription:

ipcRenderer.on('blur', () => {
  dispatcher.setAccessKeyHighlightState(false)
  dispatcher.setAppFocusState(false)
})

on app start up (maybe DARWIN conditional).

The module could be conditionalized using __DARWIN__ to subscribe or no-op (for non-macOS platforms). You might even be able to read the OS build for macOS and not bother with the subscription for builds earlier than Mojave.

[say25]

🎉

[say25]

Merge conflict from #6109 resolved. @desktop/engineering this should be Ready for Review.

[shiftkey]

I haven't updated to Mojave yet so I'm not the ideal person to be testing this, but I can help out from the "non-Mojave" and "non-macOS" testing side.

Legend for the emoji next to comments:

• 🎨 suggestions for cleanup
• 💭 things I'm still wrapping my head around

Other things are changes that I'd recommend making

[shiftkey]

@say25 do you have any pointers for reviewers to help with testing this out on Mojave? It'd be nice to be able to quickly verify that the theme changes in macOS are propagated to Desktop, and that the preferences setting is respected...

[shiftkey]

A couple of minor things, but I've also tested out the current theme switching behaviour non-Mojave and Windows setups and those seem fine. I'm not going to upgrade to Mojave for a few weeks, so I'm going to ask someone running Mojave to also review and test this out.

[say25]

Given that #6069 just got approved (and hopefully shall merge soon) I'll remove the electron 2.0.9 pollyfill at that time

[say25]

@shiftkey installed macOS Mojave on her machine, can confirm this works without state. Changed the check you sugguested to parts.length < 2 as macOS is generally 10.X.Y format though we only care about the middle digit I relaxed the constraint to < 2

Changes are fine, and I've tested non-Mojave scenarios, but I don't have Mojave installed to do the real testing

[say25]

With #6069 merged down to master, I have merged master into this branch and removed the pollyfill shim.

[tierninho]

The UI was tested for this change, and overall it functions well.

• toggling Auto-theme checkbox is functional
• switching OS theme is reflected in Desktop checkbox/theme
• Switch theme in Desktop toggles off auto checkbox if it doesn't match OS
• theme key:value is updated in local storage

Notes: cannot test if the setting in Desktop is saved until this feature hits beta.

Build/OS: 1.5.1-beta0, mac

[say25]

@shiftkey welcome back :D, noticed a merge conflict on this a couple days ago, resolved so it isn't currently blocked by me.

[shiftkey]

@say25 I don't plan to upgrade to Mojave this year (lol) so someone else who does have it is going to have to jump in here to help out with reviewing...

[say25]

🎉

[billygriffin]

🎉 This is super cool, thanks @say25! May I suggest we hold off on merging this in until #6660 gets into prod in 1.6.1 next week. I'd like to let this have at least a week in beta prior to shipping to prod in case there's any unexpected goofiness. Does that seem reasonable?

[say25]

Seems legit to me.

[shiftkey]

@billygriffin @say25 because we've already merged in a bit of new work since the release I suspect we'll just cherry-pick the specific changes we want to go out and have a short beta cycle for 1.6.1, and shuffle this and the other work into 1.6.2.