bvaughn/react-devtools-experimental

Standalone: Provide method to set theme

Closed this issue · 3 comments

In v3, we have setDefaultThemeName for set theme. In case of react-native-debugger, we used it for change theme depends on chrome devtools theme. (code)

Also, the setDefaultThemeName method still used in react-devtools package.

Hey @jhen0409 😄

v4 doesn't have configurable themes like v3 did. (That turned out to be a pretty pervasive thing to support and I don't think it added enough value to justify the footprint.) It does accept a prop that you can pass in to the root DevTools component named browserTheme which can be either "light" or "dark", but that's only used to set the default value (until the user changes it, after which it is stored in local storage).

Hmm... I wasn't really aware of this use case. I kind of copied over that one reference to setDefaultThemeName method without thinking about it I guess.

How important do you think this is? I'm currently tempted just to say that it's not going to be supported in v4, because the "dark" and "light" themes in DevTools don't look anything like e.g. Chrome's themes anyway.

I started poking around to see what would be required to support this before deciding against it. Dumping the diff here in case I decide to re-visit in the future:

diff --git a/packages/react-devtools-core/src/standalone.js b/packages/react-devtools-core/src/standalone.js
index 7bd636b..659e2c0 100644
--- a/packages/react-devtools-core/src/standalone.js
+++ b/packages/react-devtools-core/src/standalone.js
@@ -1,6 +1,6 @@
 // @flow
 
-import { createElement } from 'react';
+import { createElement, createRef } from 'react';
 import {
   // $FlowFixMe Flow does not yet know about flushSync()
   flushSync,
@@ -18,16 +18,29 @@ import launchEditor from './launchEditor';
 import { __DEBUG__ } from 'src/constants';
 
 import type { InspectedElement } from 'src/devtools/views/Components/types';
+import type { BrowserTheme } from 'src/devtools/views/DevTools';
 
 installHook(window);
 
 export type StatusListener = (message: string) => void;
 
+let devToolsRef: React$Ref<DevTools | null> = createRef(null);
+let browserTheme: ?BrowserTheme = undefined;
 let node: HTMLElement = ((null: any): HTMLElement);
 let nodeWaitingToConnectHTML: string = '';
 let projectRoots: Array<string> = [];
 let statusListener: StatusListener = (message: string) => {};
 
+function setBrowserTheme(value: BrowserTheme) {
+  browserTheme = value;
+
+  if (devToolsRef.current !== null) {
+    devToolsRef.current.setBrowserTheme(value);
+  }
+
+  return DevtoolsUI;
+}
+
 function setContentDOMNode(value: HTMLElement) {
   node = value;
 
@@ -84,6 +97,7 @@ function reload() {
     root.render(
       createElement(DevTools, {
         bridge: ((bridge: any): Bridge),
+        ref: devToolsRef,
         showTabBar: true,
         store: ((store: any): Store),
         warnIfLegacyBackendDetected: true,
@@ -282,6 +296,7 @@ function startServer(port?: number = 8097) {
 
 const DevtoolsUI = {
   connectToSocket,
+  setBrowserTheme,
   setContentDOMNode,
   setProjectRoots,
   setStatusListener,
diff --git a/packages/react-devtools/app.js b/packages/react-devtools/app.js
index c9b415e..f7f3631 100644
--- a/packages/react-devtools/app.js
+++ b/packages/react-devtools/app.js
@@ -5,7 +5,7 @@ const { join } = require('path');
 
 const argv = require('minimist')(process.argv.slice(2));
 const projectRoots = argv._;
-const defaultThemeName = argv.theme;
+const defaultBrowserTheme = argv.theme;
 
 let mainWindow = null;
 
@@ -37,8 +37,8 @@ app.on('ready', function() {
 
   if (argv.theme) {
     mainWindow.webContents.executeJavaScript(
-      'window.devtools.setDefaultThemeName(' +
-        JSON.stringify(defaultThemeName) +
+      'window.devtools.setBrowserTheme(' +
+        JSON.stringify(defaultBrowserTheme) +
         ')'
     );
   }
diff --git a/src/devtools/views/DevTools.js b/src/devtools/views/DevTools.js
index f8d9bce..d6e7ff0 100644
--- a/src/devtools/views/DevTools.js
+++ b/src/devtools/views/DevTools.js
@@ -5,7 +5,7 @@
 import '@reach/menu-button/styles.css';
 import '@reach/tooltip/styles.css';
 
-import React, { useMemo, useState } from 'react';
+import React, { forwardRef, useImperativeHandle, useMemo, useState } from 'react';
 import Bridge from 'src/bridge';
 import Store from '../store';
 import { BridgeContext, StoreContext } from './context';
@@ -72,7 +72,7 @@ const profilerTab = {
 
 const tabs = [componentsTab, profilerTab];
 
-export default function DevTools({
+function DevTools({
   bridge,
   browserTheme = 'light',
   defaultTab = 'components',
@@ -85,7 +85,8 @@ export default function DevTools({
   warnIfLegacyBackendDetected = false,
   viewElementSourceFunction,
   viewElementSourceRequiresFileLocation = false,
-}: Props) {
+}: Props, ref) {
+  const [theme, setTheme] = useState<BrowserTheme>(browserTheme);
   const [tab, setTab] = useState(defaultTab);
   if (overrideTab != null && overrideTab !== tab) {
     setTab(overrideTab);
@@ -99,12 +100,16 @@ export default function DevTools({
     [viewElementSourceFunction, viewElementSourceRequiresFileLocation]
   );
 
+  useImperativeHandle(ref, () => ({
+    setBrowserTheme: setTheme,
+  }));
+
   return (
     <BridgeContext.Provider value={bridge}>
       <StoreContext.Provider value={store}>
         <ModalDialogContextController>
           <SettingsContextController
-            browserTheme={browserTheme}
+            browserTheme={theme}
             componentsPortalContainer={componentsPortalContainer}
             profilerPortalContainer={profilerPortalContainer}
             settingsPortalContainer={settingsPortalContainer}
@@ -152,3 +157,5 @@ export default function DevTools({
     </BridgeContext.Provider>
   );
 }
+
+export default forwardRef(DevTools);

I think I'm just going to remove the one reference to setDefaultThemeName though and say that this functionality just isn't supported for v4.

Going to close this issue for now, since I don't plan to take further action at the moment. We can keep chatting here though!