feat: add APIs to support mojave dark modes #14755

MarshallOfSound · 2018-09-21T13:02:26Z

This PR implements the following APIs:

### `systemPreferences.getEffectiveAppearance()` _macOS_

Returns `String` - Can be `dark`, `light` or `unknown`.

Gets the macOS appearance setting that is currently applied to your application,
maps to [NSApplication.effectiverAppearance](https://developer.apple.com/documentation/appkit/nsapplication/2967171-effectiveappearance?language=objc)

Please note that because Electron is not built targetting the 10.14 SDK your applications
`effectiveAppearance` will always default to "light" and never automatically inherit the OS
level setting.  We have provided a helper method `startAppLevelAppearanceTrackingOS()` which
will emulate this behavior until we start targetting the 10.14 SDK.

### `systemPreferences.getAppLevelAppearance()` _macOS_

Returns `String` | `null` - Can be `dark`, `light` or `unknown`.

Gets the macOS appearance setting that is you have declared you want for
your application, maps to [NSApplication.appearance](https://developer.apple.com/documentation/appkit/nsapplication/2967170-appearance?language=objc).
You can use the `setAppLevelAppearance` API to set this value.

### `systemPreferences.setAppLevelAppearance(appearance)` _macOS_

* `appearance` String | null - Can be `dark` or `light`

Sets the appearance setting for your application, this should override the
system default and override the value of `getEffectiveAppearance`.

### `systemPreferences.startAppLevelAppearanceTrackingOS()` _macOS_

This is a helper method to make your applications "appearance" setting track the
users OS level appearance setting.  I.e. Your app will have dark mode enabled if
the users system has dark mode enabled.

You can track this automatic change with the `appearance-changed` event.

### `systemPreferences.stopAppLevelAppearanceTrackingOS()` _macOS_

This is a helper method to stop your application tracking the OS level appearance
setting.  It is a no-op if you have not called `startAppLevelAppearanceTrackingOS()`

Please note the lack of tests is currently delibrate due to our tests not running on mojave machines (so these APIs are all noops)

Notes: Added systemPreferences APIs to help apps respect mojave dark mode settings

ckerr · 2018-09-21T13:22:10Z

docs/api/system-preferences.md

+
+Returns `String` | `null` - Can be `dark`, `light` or `unknown`.
+
+Gets the macOS appearance setting that is you have declared you want for


ckerr · 2018-09-21T13:23:37Z

docs/api/system-preferences.md

+
+### `systemPreferences.startAppLevelAppearanceTrackingOS()` _macOS_
+
+This is a helper method to make your applications "appearance" setting track the


application's

ckerr · 2018-09-21T13:23:53Z

docs/api/system-preferences.md

+
+This is a helper method to make your applications "appearance" setting track the
+users OS level appearance setting.  I.e. Your app will have dark mode enabled if
+the users system has dark mode enabled.


ckerr · 2018-09-21T13:24:48Z

lib/browser/api/browser-window.js

@@ -141,7 +141,7 @@ BrowserWindow.fromId = (id) => {
 }

 BrowserWindow.getAllWindows = () => {
-  return TopLevelWindow.getAllWindows().filter(isBrowserWindow)
+  return TopLevelWindow.getAllWindows()


This seems significant.

semver/minor?

Oh this is a mis-commit, will remove

ckerr · 2018-09-21T13:26:07Z

lib/browser/api/system-preferences.js

+        : 'light'
+
+      systemPreferences.emit('appearance-changed', newAppearance)
+      systemPreferences.setAppLevelAppearance(newAppearance)


Should appearance-changed emit after the appearance has been changed?

ckerr · 2018-09-21T13:30:04Z

docs/api/system-preferences.md

+### `systemPreferences.startAppLevelAppearanceTrackingOS()` _macOS_
+
+This is a helper method to make your applications "appearance" setting track the
+users OS level appearance setting.  I.e. Your app will have dark mode enabled if


s/users/user's/

s/Your/your/

ckerr · 2018-09-21T13:38:00Z

atom/browser/api/atom_api_system_preferences_mac.mm

+
+    if (val.name == NSAppearanceNameAqua) {
+      return mate::ConvertToV8(isolate, "light");
+    } else if (val.name == NSAppearanceNameDarkAqua) {


Inconsistent_ use of else if after a return clause.

This is a style thing that I would let slide but there's a conditional return just above this paragraph and another return immediately following, making the inconsistency more jarring.

This would be good:

if (val == nil) { return v8::Null(isolate); } if (val.name == NSAppearanceNameAqua) { return mate::ConvertToV8(isolate, "light"); } if (val.name == NSAppearanceNameDarkAqua) { return mate::ConvertToV8(isolate, "dark"); } return mate::ConvertToV8(isolate, "unknown")

This would also be OK:

if (val == nil) { return v8::Null(isolate); } else if (val.name == NSAppearanceNameAqua) { return mate::ConvertToV8(isolate, "light"); } else if (val.name == NSAppearanceNameDarkAqua) { return mate::ConvertToV8(isolate, "dark"); } else { return mate::ConvertToV8(isolate, "unknown") }

atom/browser/api/atom_api_system_preferences_mac.mm

ckerr · 2018-09-21T13:44:27Z

docs/api/system-preferences.md

+Gets the macOS appearance setting that is currently applied to your application,
+maps to [NSApplication.effectiverAppearance](https://developer.apple.com/documentation/appkit/nsapplication/2967171-effectiveappearance?language=objc)
+
+Please note that because Electron is not built targetting the 10.14 SDK your applications


s/not built/not yet built/

targeting

s/SDK/SDK,/

application's

ckerr · 2018-09-21T13:46:07Z

docs/api/system-preferences.md

+maps to [NSApplication.effectiverAppearance](https://developer.apple.com/documentation/appkit/nsapplication/2967171-effectiveappearance?language=objc)
+
+Please note that because Electron is not built targetting the 10.14 SDK your applications
+`effectiveAppearance` will always default to "light" and never automatically inherit the OS


Confusing use of 'never' here since we're implying that it will work correctly once Electron is built targeting the 10.14 SDK.

Possibly better:

Please note that until Electron is built targeting the 10.14 SDK, your application's effectiveAppearance will default to 'light' and won't inherit the OS preference. In the interim we have provided a helper method startAppLevelAppearanceTrackingOS() which emulates this behavior.

Actually -- rather than make this a public API that people will have to opt-in on and which we'll have to deprecate after moving to 10.14 SDK -- why not make this run automatically so that the color scheme will Just Work both before and after the SDK switch?

why not make this run automatically so that the color scheme will Just Work both before and after the SDK switch?

This is a good question, I personally wouldn't want this to be forced upon apps because enabling dark mode in your app is more complex than just the native macOS UI looking dark mode. For instance if we forced this on all app's context menus would become dark themed, which may not look too good with a light themed app.

In my mind, this would be opt-in for apps that have a dark theme, and once we move to 10.14's SDK the auto-move can be opt-out through a field in the Info.plist which we can document once we make that move. (There is a built-in field macOS respects to not auto-switch that Chrome uses at the moment)

I'll add a note to this API though that it is exempt from the standard deprecation cycle, it will be removed in a single major bump at some point in the future because as soon as we reach 10.14 this API will be pointless and probably not work correctly.

MarshallOfSound · 2018-09-22T05:14:19Z

@ckerr Updated 👍

albinekb · 2018-09-24T20:08:52Z

docs/api/system-preferences.md

+Returns `String` - Can be `dark`, `light` or `unknown`.
+
+Gets the macOS appearance setting that is currently applied to your application,
+maps to [NSApplication.effectiverAppearance](https://developer.apple.com/documentation/appkit/nsapplication/2967171-effectiveappearance?language=objc)


[NSApplication.effectiveAppearance]?

This isn't objective c, it's intended to indicate a property style value to JS devs that they can click on to learn more

Adding an r into "effectiveAppearance" so it's "effectiverAppearance" does that?

It's more effective that way

*more effectiver

Closes #13387

release-clerk · 2018-09-27T15:33:37Z

Release Notes Persisted

Added systemPreferences APIs to help apps respect mojave dark mode settings

* feat: add APIs to support mojave dark mode Closes #13387 * docs: fix system-prefs typo

vanejung · 2018-10-06T02:23:04Z

@MarshallOfSound Perhaps this is going to be a dumb question..

I've added the code below in the main process in order to dynamically set a theme status state in my app.

const { ..., systemPreferences } = require('electron')
systemPreferences.startAppLevelAppearanceTrackingOS()
systemPreferences.on('appearance-changed', (newAppearance) => {
    mainWindow.send('theme-mode', newAppearance)
})

But I'm getting TypeError: systemPreferences.startAppLevelAppearanceTrackingOS is not a function ....

What did I do wrong here? :(

vanejung · 2018-10-07T22:13:05Z

@MarshallOfSound Don't mind me... Fixed own problem by doing this instead,

systemPreferences.subscribeNotification('AppleInterfaceThemeChangedNotification', () => {
    mainWindow.send('theme-mode', systemPreferences.isDarkMode())
})

Got the idea from here. - sindresorhus/caprine#510

markozxuu · 2018-10-11T14:28:53Z

@MarshallOfSound In what version is this change available? ^-^

ckerr · 2018-10-11T15:47:24Z

@mapeso I believe this is in 4.0.0-beta.1

RyanZim · 2018-10-17T22:02:29Z

Are there plans to backport this to 3.x or 2.x release lines?

MarshallOfSound requested review from a team September 21, 2018 13:02

MarshallOfSound added the semver/minor backwards-compatible functionality label Sep 21, 2018

MarshallOfSound force-pushed the majove-dark-mode branch from 79e0ba4 to 78a3ed2 Compare September 21, 2018 13:12

MarshallOfSound mentioned this pull request Sep 21, 2018

Add automatic switching between Dark Mode and Light Mode or ability to change it programmatically. desktop/desktop#5037

Closed

ckerr reviewed Sep 21, 2018

View reviewed changes

MarshallOfSound force-pushed the majove-dark-mode branch from 78a3ed2 to 965b7c8 Compare September 22, 2018 05:13

skllcrn mentioned this pull request Sep 24, 2018

Dark Mode support wulkano/Kap#478

Closed

albinekb reviewed Sep 24, 2018

View reviewed changes

jsynowiec mentioned this pull request Sep 25, 2018

Support macOS Mojave dark mode jsynowiec/airqmon#35

Closed

alesso-x mentioned this pull request Sep 25, 2018

macOS: Show native tabs in dark when dark mode enabled microsoft/vscode#59283

Closed

pento mentioned this pull request Sep 25, 2018

Add Dark Mode support pento/testpress#49

Closed

MarshallOfSound mentioned this pull request Sep 25, 2018

Add API's to support dark mode in Mojave #13387

Closed

MarshallOfSound and others added 2 commits September 26, 2018 16:47

feat: add APIs to support mojave dark mode

3cb3e9c

Closes #13387

docs: fix system-prefs typo

40ecd21

MarshallOfSound force-pushed the majove-dark-mode branch from 7716c60 to 40ecd21 Compare September 26, 2018 06:47

MarshallOfSound changed the title ~~feat: add APIs to support mojave dark mode~~ feat: add APIs to support mojave dark modes Sep 27, 2018

MarshallOfSound mentioned this pull request Sep 27, 2018

[Linux] Menubar background color is always white #13381

Closed

ckerr approved these changes Sep 27, 2018

View reviewed changes

ckerr merged commit 0d2a0c7 into master Sep 27, 2018

ckerr deleted the majove-dark-mode branch September 27, 2018 15:33

codebytere pushed a commit that referenced this pull request Sep 27, 2018

feat: add APIs to support mojave dark modes (#14755)

d9608f1

* feat: add APIs to support mojave dark mode Closes #13387 * docs: fix system-prefs typo

simurai mentioned this pull request Oct 2, 2018

Mojave dark mode atom/atom#18137

Closed

karlhorky mentioned this pull request Oct 11, 2018

Mojave dark mode causes a white line at the top of Electron apps #13164

Closed

ckerr mentioned this pull request Dec 18, 2018

Draft 4.0.0 Release Notes #16117

Closed

kuoruan mentioned this pull request Jan 6, 2019

Dark 模式下输入框的黑边 rime/squirrel#250

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add APIs to support mojave dark modes #14755

feat: add APIs to support mojave dark modes #14755

MarshallOfSound commented Sep 21, 2018

ckerr Sep 21, 2018

ckerr Sep 21, 2018

ckerr Sep 21, 2018

ckerr Sep 21, 2018

MarshallOfSound Sep 21, 2018

ckerr Sep 21, 2018

ckerr Sep 21, 2018

ckerr Sep 21, 2018

ckerr Sep 21, 2018

ckerr Sep 21, 2018

ckerr Sep 21, 2018

MarshallOfSound Sep 22, 2018

MarshallOfSound commented Sep 22, 2018

albinekb Sep 24, 2018

MarshallOfSound Sep 24, 2018

abotkin-cpi Sep 24, 2018

albinekb Sep 24, 2018

ckerr Sep 27, 2018

release-clerk bot commented Sep 27, 2018

vanejung commented Oct 6, 2018 •

edited

vanejung commented Oct 7, 2018 •

edited

markozxuu commented Oct 11, 2018

ckerr commented Oct 11, 2018

RyanZim commented Oct 17, 2018


		Returns `String` \| `null` - Can be `dark`, `light` or `unknown`.

		Gets the macOS appearance setting that is you have declared you want for


		### `systemPreferences.startAppLevelAppearanceTrackingOS()` _macOS_

		This is a helper method to make your applications "appearance" setting track the

feat: add APIs to support mojave dark modes #14755

feat: add APIs to support mojave dark modes #14755

Conversation

MarshallOfSound commented Sep 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarshallOfSound commented Sep 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

release-clerk bot commented Sep 27, 2018

vanejung commented Oct 6, 2018 • edited

vanejung commented Oct 7, 2018 • edited

markozxuu commented Oct 11, 2018

ckerr commented Oct 11, 2018

RyanZim commented Oct 17, 2018

vanejung commented Oct 6, 2018 •

edited

vanejung commented Oct 7, 2018 •

edited