Merge pull request #13794 from desktop/refactor-remote-usage-in-app-theme-settings-to-main-process

Refactor remote usage in app's theme settings to main process

app/src/lib/ipc-shared.ts

@@ -12,6 +12,7 @@ import { URLActionType } from './parse-app-url'
 import { Architecture } from './get-architecture'
 import { EndpointToken } from './endpoint-token'
 import { PathType } from '../ui/lib/app-proxy'
+import { ThemeSource } from '../ui/lib/theme-source'
 
 /**
  * Defines the simplex IPC channel names we use from the renderer
@@ -70,6 +71,7 @@ export type RequestChannels = {
   'auto-updater-update-not-available': () => void
   'auto-updater-update-downloaded': () => void
   'native-theme-updated': () => void
+  'set-native-theme-source': (themeName: ThemeSource) => void
   'move-to-applications-folder': () => void
   'focus-window': () => void
 }
@@ -109,4 +111,5 @@ export type RequestResponseChannels = {
   'get-apple-action-on-double-click': () => Promise<
     Electron.AppleActionOnDoubleClickPref
   >
+  'should-use-dark-colors': () => Promise<boolean>
 }

app/src/lib/stores/app-store.ts

@@ -1856,7 +1856,7 @@ export class AppStore extends TypedBaseStore<IAppState> {
 
     this.currentTheme =
       this.selectedTheme !== ApplicationTheme.HighContrast
-        ? getCurrentlyAppliedTheme()
+        ? await getCurrentlyAppliedTheme()
         : this.selectedTheme
 
     themeChangeMonitor.onThemeChanged(theme => {

app/src/main-process/main.ts

@@ -7,6 +7,7 @@ import {
   shell,
   session,
   systemPreferences,
+  nativeTheme,
 } from 'electron'
 import * as Fs from 'fs'
 import * as URL from 'url'
@@ -38,6 +39,7 @@ import * as ipcMain from './ipc-main'
 import { getArchitecture } from '../lib/get-architecture'
 import * as remoteMain from '@electron/remote/main'
 import { buildSpellCheckMenu } from './menu/build-spell-check-menu'
+
 remoteMain.initialize()
 
 app.setAppLogsPath()
@@ -693,6 +695,15 @@ app.on('ready', () => {
   ipcMain.on('focus-window', () => {
     mainWindow?.focus()
   })
+
+  ipcMain.on('set-native-theme-source', (_, themeName) => {
+    nativeTheme.themeSource = themeName
+  })
+
+  ipcMain.handle(
+    'should-use-dark-colors',
+    async () => nativeTheme.shouldUseDarkColors
+  )
 })
 
 app.on('activate', () => {

app/src/ui/app-theme.tsx

@@ -39,7 +39,7 @@ export class AppTheme extends React.PureComponent<IAppThemeProps> {
     this.clearThemes()
   }
 
-  private ensureTheme() {
+  private async ensureTheme() {
     const { customTheme, useCustomTheme } = this.props
     if (customTheme !== undefined && useCustomTheme) {
       this.clearThemes()
@@ -50,7 +50,7 @@ export class AppTheme extends React.PureComponent<IAppThemeProps> {
     let themeToDisplay = this.props.theme
 
     if (this.props.theme === ApplicationTheme.System) {
-      themeToDisplay = getCurrentlyAppliedTheme()
+      themeToDisplay = await getCurrentlyAppliedTheme()
     }
 
     const newThemeClassName = `theme-${getThemeName(themeToDisplay)}`

app/src/ui/lib/application-theme.ts

@@ -1,9 +1,13 @@
-import * as remote from '@electron/remote'
 import {
   isMacOSMojaveOrLater,
   isWindows10And1809Preview17666OrLater,
 } from '../../lib/get-os'
 import { getBoolean } from '../../lib/local-storage'
+import {
+  setNativeThemeSource,
+  shouldUseDarkColors,
+} from '../main-process-proxy'
+import { ThemeSource } from './theme-source'
 
 /** Interface for set of customizable styles */
 export interface ICustomTheme {
@@ -39,9 +43,7 @@ export type ApplicableTheme =
  * in persisting to storage and/or calculating the required
  * body class name to set in order to apply the theme.
  */
-export function getThemeName(
-  theme: ApplicationTheme
-): 'light' | 'dark' | 'system' {
+export function getThemeName(theme: ApplicationTheme): ThemeSource {
   switch (theme) {
     case ApplicationTheme.Light:
       return 'light'
@@ -101,8 +103,10 @@ function getApplicationThemeSetting(): ApplicationTheme {
 /**
  * Load the name of the currently selected theme
  */
-export function getCurrentlyAppliedTheme(): ApplicableTheme {
-  return isDarkModeEnabled() ? ApplicationTheme.Dark : ApplicationTheme.Light
+export async function getCurrentlyAppliedTheme(): Promise<ApplicableTheme> {
+  return (await isDarkModeEnabled())
+    ? ApplicationTheme.Dark
+    : ApplicationTheme.Light
 }
 
 /**
@@ -122,7 +126,7 @@ export function getPersistedThemeName(): ApplicationTheme {
 export function setPersistedTheme(theme: ApplicationTheme): void {
   const themeName = getThemeName(theme)
   localStorage.setItem(applicationThemeKey, theme)
-  remote.nativeTheme.themeSource = themeName
+  setNativeThemeSource(themeName)
 }
 
 /**
@@ -141,6 +145,6 @@ export function supportsSystemThemeChanges(): boolean {
   return false
 }
 
-function isDarkModeEnabled(): boolean {
-  return remote.nativeTheme.shouldUseDarkColors
+function isDarkModeEnabled(): Promise<boolean> {
+  return shouldUseDarkColors()
 }

app/src/ui/lib/theme-change-monitor.ts

@@ -21,8 +21,8 @@ class ThemeChangeMonitor {
     onNativeThemeUpdated(this.onThemeNotificationUpdated)
   }
 
-  private onThemeNotificationUpdated = () => {
-    const theme = getCurrentlyAppliedTheme()
+  private onThemeNotificationUpdated = async () => {
+    const theme = await getCurrentlyAppliedTheme()
     this.emitThemeChanged(theme)
   }
 

app/src/ui/lib/theme-source.ts

@@ -0,0 +1 @@
+export type ThemeSource = 'light' | 'dark' | 'system'

app/src/ui/main-process-proxy.ts

@@ -141,6 +141,12 @@ export function onNativeThemeUpdated(eventHandler: () => void) {
   ipcRenderer.on('native-theme-updated', eventHandler)
 }
 
+/** Tell the main process to set the native theme source */
+export const setNativeThemeSource = sendProxy('set-native-theme-source', 1)
+
+/** Tell the main process to obtain wether the native theme uses dark colors */
+export const shouldUseDarkColors = invokeProxy('should-use-dark-colors', 0)
+
 /** Tell the main process to minimize the window */
 export const minimizeWindow = sendProxy('minimize-window', 0)
 

app/src/ui/preferences/appearance.tsx

@@ -21,6 +21,10 @@ interface IAppearanceProps {
   readonly onCustomThemeChanged: (theme: ICustomTheme) => void
 }
 
+interface IAppearanceState {
+  readonly selectedTheme: ApplicationTheme | null
+}
+
 const systemTheme: ISegmentedItem<ApplicationTheme> = {
   title: 'System',
   description: 'Automatically switch theme to match system theme',
@@ -50,7 +54,45 @@ const themes: ReadonlyArray<ISegmentedItem<ApplicationTheme>> = [
   ...(supportsSystemThemeChanges() ? [systemTheme] : []),
 ]
 
-export class Appearance extends React.Component<IAppearanceProps, {}> {
+export class Appearance extends React.Component<
+  IAppearanceProps,
+  IAppearanceState
+> {
+  public constructor(props: IAppearanceProps) {
+    super(props)
+
+    const usePropTheme =
+      props.selectedTheme !== ApplicationTheme.System ||
+      supportsSystemThemeChanges()
+
+    this.state = { selectedTheme: usePropTheme ? props.selectedTheme : null }
+
+    if (!usePropTheme) {
+      this.initializeSelectedTheme()
+    }
+  }
+
+  public async componentDidUpdate(prevProps: IAppearanceProps) {
+    if (prevProps.selectedTheme === this.props.selectedTheme) {
+      return
+    }
+
+    const usePropTheme =
+      this.props.selectedTheme !== ApplicationTheme.System ||
+      supportsSystemThemeChanges()
+
+    const selectedTheme = usePropTheme
+      ? this.props.selectedTheme
+      : await getCurrentlyAppliedTheme()
+
+    this.setState({ selectedTheme })
+  }
+
+  private initializeSelectedTheme = async () => {
+    const selectedTheme = await getCurrentlyAppliedTheme()
+    this.setState({ selectedTheme })
+  }
+
   private onSelectedThemeChanged = (theme: ApplicationTheme) => {
     this.props.onSelectedThemeChanged(theme)
   }
@@ -60,13 +102,14 @@ export class Appearance extends React.Component<IAppearanceProps, {}> {
   }
 
   public render() {
-    let selectedTheme = this.props.selectedTheme
+    const { selectedTheme } = this.state
 
-    if (
-      this.props.selectedTheme === ApplicationTheme.System &&
-      !supportsSystemThemeChanges()
-    ) {
-      selectedTheme = getCurrentlyAppliedTheme()
+    if (selectedTheme == null) {
+      return (
+        <DialogContent>
+          <Row>Loading system theme</Row>
+        </DialogContent>
+      )
     }
 
     return (