Closed jhen0409 closed 5 years ago
Hey @jhen0409 :smile:
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!
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 inreact-devtools
package.