Skip to content

Commit

Permalink
fix: macOS setColor inconsistencies
Browse files Browse the repository at this point in the history
  • Loading branch information
codebytere committed Oct 17, 2019
1 parent 5273930 commit 9bd224b
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 9 deletions.
6 changes: 4 additions & 2 deletions docs/api/system-preferences.md
Expand Up @@ -291,7 +291,7 @@ This API is only available on macOS 10.14 Mojave or newer.
* `window-frame` - Window frame.
* `window-text` - Text in windows.
* On **macOS**
* `alternate-selected-control-text` - The text on a selected surface in a list or table.
* `alternate-selected-control-text` - The text on a selected surface in a list or table. _deprecated_
* `control-background` - The background of a large interface element, such as a browser or table.
* `control` - The surface of a control.
* `control-text` -The text of a control that isn’t disabled.
Expand All @@ -310,7 +310,7 @@ This API is only available on macOS 10.14 Mojave or newer.
* `selected-content-background` - The background for selected content in a key window or view.
* `selected-control` - The surface of a selected control.
* `selected-control-text` - The text of a selected control.
* `selected-menu-item` - The text of a selected menu.
* `selected-menu-item-text` - The text of a selected menu.
* `selected-text-background` - The background of selected text.
* `selected-text` - Selected text.
* `separator` - A separator between different sections of content.
Expand All @@ -328,6 +328,8 @@ This API is only available on macOS 10.14 Mojave or newer.
Returns `String` - The system color setting in RGB hexadecimal form (`#ABCDEF`).
See the [Windows docs][windows-colors] and the [MacOS docs][macos-colors] for more details.

The following colors are only available on macOS 10.14: `find-highlight`, `selected-content-background`, `separator`, `unemphasized-selected-content-background`, `unemphasized-selected-text-background`, and `unemphasized-selected-text`.

[windows-colors]:https://msdn.microsoft.com/en-us/library/windows/desktop/ms724371(v=vs.85).aspx
[macos-colors]:https://developer.apple.com/design/human-interface-guidelines/macos/visual-design/color#dynamic-system-colors

Expand Down
4 changes: 3 additions & 1 deletion shell/browser/api/atom_api_system_preferences.h
Expand Up @@ -13,6 +13,7 @@
#include "native_mate/handle.h"
#include "shell/browser/api/event_emitter.h"
#include "shell/common/gin_helper/error_thrower.h"
#include "shell/common/node_includes.h"
#include "shell/common/promise_util.h"

#if defined(OS_WIN)
Expand Down Expand Up @@ -52,7 +53,8 @@ class SystemPreferences : public mate::EventEmitter<SystemPreferences>

#if defined(OS_WIN) || defined(OS_MACOSX)
std::string GetAccentColor();
std::string GetColor(const std::string& color, mate::Arguments* args);
std::string GetColor(gin_helper::ErrorThrower thrower,
const std::string& color);
#endif
#if defined(OS_WIN)
bool IsAeroGlassEnabled();
Expand Down
19 changes: 13 additions & 6 deletions shell/browser/api/atom_api_system_preferences_mac.mm
Expand Up @@ -503,19 +503,25 @@ AVMediaType ParseMediaType(const std::string& media_type) {
return AXIsProcessTrustedWithOptions((CFDictionaryRef)options);
}

std::string SystemPreferences::GetColor(const std::string& color,
mate::Arguments* args) {
std::string SystemPreferences::GetColor(gin_helper::ErrorThrower thrower,
const std::string& color) {
NSColor* sysColor = nil;
if (color == "alternate-selected-control-text") {
sysColor = [NSColor alternateSelectedControlTextColor];
EmitDeprecationWarning(node::Environment::GetCurrent(thrower.isolate()),
"Use selected-content-background instead.",
"electron");
} else if (color == "control-background") {
sysColor = [NSColor controlBackgroundColor];
} else if (color == "control") {
sysColor = [NSColor controlColor];
} else if (color == "control-text") {
sysColor = [NSColor controlTextColor];
} else if (color == "disabled-control") {
} else if (color == "disabled-control-text") {
sysColor = [NSColor disabledControlTextColor];
EmitDeprecationWarning(node::Environment::GetCurrent(thrower.isolate()),
"Use a color that matches the semantics being used.",
"electron");
} else if (color == "find-highlight") {
if (@available(macOS 10.14, *))
sysColor = [NSColor findHighlightColor];
Expand Down Expand Up @@ -580,11 +586,12 @@ AVMediaType ParseMediaType(const std::string& media_type) {
} else if (color == "window-frame-text") {
sysColor = [NSColor windowFrameTextColor];
} else {
args->ThrowError("Unknown color: " + color);
return "";
thrower.ThrowError("Unknown color: " + color);
}

return base::SysNSStringToUTF8([sysColor hexadecimalValue]);
if (sysColor)
return base::SysNSStringToUTF8([sysColor hexadecimalValue]);
return "";
}

std::string SystemPreferences::GetMediaAccessStatus(
Expand Down
52 changes: 52 additions & 0 deletions spec-main/api-system-preferences-spec.ts
Expand Up @@ -134,6 +134,58 @@ describe('systemPreferences module', () => {
})
})

ifdescribe(process.platform === 'darwin')('systemPreferences.getColor(color)', () => {
it('throws on invalid colors', () => {
const color = 'bad-color'
expect(() => {
systemPreferences.getColor(color as any)
}).to.throw(`Unknown color: ${color}`)
})

it('returns a valid color', () => {
const colors = [
'alternate-selected-control-text',
'control-background',
'control',
'control-text',
'disabled-control-text',
'find-highlight',
'grid',
'header-text',
'highlight',
'keyboard-focus-indicator',
'label',
'link',
'placeholder-text',
'quaternary-label',
'scrubber-textured-background',
'secondary-label',
'selected-content-background',
'selected-control',
'selected-control-text',
'selected-menu-item-text',
'selected-text-background',
'selected-text',
'separator',
'shadow',
'tertiary-label',
'text-background',
'text',
'under-page-background',
'unemphasized-selected-content-background',
'unemphasized-selected-text-background',
'unemphasized-selected-text',
'window-background',
'window-frame-text'
]

colors.forEach(color => {
const sysColor = systemPreferences.getColor(color as any)
expect(sysColor).to.be.a('string')
})
})
})

ifdescribe(process.platform === 'darwin')('systemPreferences.appLevelAppearance', () => {
it('has an appLevelAppearance property', () => {
expect(systemPreferences).to.have.property('appLevelAppearance')
Expand Down

0 comments on commit 9bd224b

Please sign in to comment.