Misc improvements based on user feedback from Tim

Brian Vaughn · Brian Vaughn · commit f74c89b145c3 · 2019-07-24T11:25:11.000-07:00
* Added shadow to modals
* Change default "collapse new nodes" to be disabled rather than enabled
* Changed setting label "Collapse newly added components by default" to "Expand component tree by default"
* Change CSS media query for settings popup to show labels at smaller size
* Hide "Inspect the matching DOM element" button (since it doesnt really serve a purpose in standalone)
* Fixed small size bug for settings icon (viewbox of 20x20 instead of 24x24)
* bugfix: window.addEventListener/window.removeEventListener is not defined in Hermes.
diff --git a/packages/react-devtools-core/src/standalone.js b/packages/react-devtools-core/src/standalone.js
@@ -85,6 +85,7 @@ function reload() {
     root.render(
       createElement(DevTools, {
         bridge: ((bridge: any): FrontendBridge),
+        isBrowserMode: false,
         showTabBar: true,
         store: ((store: any): Store),
         warnIfLegacyBackendDetected: true,
diff --git a/src/backend/renderer.js b/src/backend/renderer.js
@@ -135,6 +135,12 @@ type ReactTypeOfSideEffectType = {|
   Placement: number,
 |};
 
+// Some environments (e.g. React Native / Hermes) don't support the performace API yet.
+const getCurrentTime =
+  typeof performance === 'object' && typeof performance.now === 'function'
+    ? () => performance.now()
+    : () => Date.now();
+
 export function getInternalReactConstants(
   version: string
 ): {|
@@ -1645,7 +1651,7 @@ export function attach(
           currentCommitProfilingMetadata = {
             changeDescriptions: recordChangeDescriptions ? new Map() : null,
             durations: [],
-            commitTime: performance.now() - profilingStartTime,
+            commitTime: getCurrentTime() - profilingStartTime,
             interactions: Array.from(root.memoizedInteractions).map(
               (interaction: Interaction) => ({
                 ...interaction,
@@ -1689,7 +1695,7 @@ export function attach(
       currentCommitProfilingMetadata = {
         changeDescriptions: recordChangeDescriptions ? new Map() : null,
         durations: [],
-        commitTime: performance.now() - profilingStartTime,
+        commitTime: getCurrentTime() - profilingStartTime,
         interactions: Array.from(root.memoizedInteractions).map(
           (interaction: Interaction) => ({
             ...interaction,
@@ -2738,7 +2744,7 @@ export function attach(
     });
 
     isProfiling = true;
-    profilingStartTime = performance.now();
+    profilingStartTime = getCurrentTime();
     rootToCommitProfilingMetadataMap = new Map();
   }
 
diff --git a/src/backend/views/Highlighter/index.js b/src/backend/views/Highlighter/index.js
@@ -7,6 +7,11 @@ import { hideOverlay, showOverlay } from './Highlighter';
 
 import type { BackendBridge } from 'src/bridge';
 
+// This plug-in provides in-page highlighting of the selected element.
+// It is used by the browser extension nad the standalone DevTools shell (when connected to a browser).
+// It is not currently the mechanism used to highlight React Native views.
+// That is done by the React Native Inspector component.
+
 export default function setupHighlighter(
   bridge: BackendBridge,
   agent: Agent
@@ -21,25 +26,31 @@ export default function setupHighlighter(
   bridge.addListener('stopInspectingNative', stopInspectingNative);
 
   function startInspectingNative() {
-    window.addEventListener('click', onClick, true);
-    window.addEventListener('mousedown', onMouseEvent, true);
-    window.addEventListener('mouseover', onMouseEvent, true);
-    window.addEventListener('mouseup', onMouseEvent, true);
-    window.addEventListener('pointerdown', onPointerDown, true);
-    window.addEventListener('pointerover', onPointerOver, true);
-    window.addEventListener('pointerup', onPointerUp, true);
+    // This plug-in may run in non-DOM environments (e.g. React Native).
+    if (window && typeof window.addEventListener === 'function') {
+      window.addEventListener('click', onClick, true);
+      window.addEventListener('mousedown', onMouseEvent, true);
+      window.addEventListener('mouseover', onMouseEvent, true);
+      window.addEventListener('mouseup', onMouseEvent, true);
+      window.addEventListener('pointerdown', onPointerDown, true);
+      window.addEventListener('pointerover', onPointerOver, true);
+      window.addEventListener('pointerup', onPointerUp, true);
+    }
   }
 
   function stopInspectingNative() {
     hideOverlay();
 
-    window.removeEventListener('click', onClick, true);
-    window.removeEventListener('mousedown', onMouseEvent, true);
-    window.removeEventListener('mouseover', onMouseEvent, true);
-    window.removeEventListener('mouseup', onMouseEvent, true);
-    window.removeEventListener('pointerdown', onPointerDown, true);
-    window.removeEventListener('pointerover', onPointerOver, true);
-    window.removeEventListener('pointerup', onPointerUp, true);
+    // This plug-in may run in non-DOM environments (e.g. React Native).
+    if (window && typeof window.removeEventListener === 'function') {
+      window.removeEventListener('click', onClick, true);
+      window.removeEventListener('mousedown', onMouseEvent, true);
+      window.removeEventListener('mouseover', onMouseEvent, true);
+      window.removeEventListener('mouseup', onMouseEvent, true);
+      window.removeEventListener('pointerdown', onPointerDown, true);
+      window.removeEventListener('pointerover', onPointerOver, true);
+      window.removeEventListener('pointerup', onPointerUp, true);
+    }
   }
 
   function clearNativeElementHighlight() {
diff --git a/src/devtools/store.js b/src/devtools/store.js
@@ -136,10 +136,9 @@ export default class Store extends EventEmitter<{|
       debug('constructor', 'subscribing to Bridge');
     }
 
-    // Default this setting to true unless otherwise specified.
     this._collapseNodesByDefault =
-      localStorageGetItem(LOCAL_STORAGE_COLLAPSE_ROOTS_BY_DEFAULT_KEY) !==
-      'false';
+      localStorageGetItem(LOCAL_STORAGE_COLLAPSE_ROOTS_BY_DEFAULT_KEY) ===
+      'true';
 
     this._recordChangeDescriptions =
       localStorageGetItem(LOCAL_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY) ===
diff --git a/src/devtools/views/ButtonIcon.js b/src/devtools/views/ButtonIcon.js
@@ -208,12 +208,13 @@ const PATH_SEARCH = `
 `;
 
 const PATH_SETTINGS = `
-  M15.95 10.78c.03-.25.05-.51.05-.78s-.02-.53-.06-.78l1.69-1.32c.15-.12.19-.34.1-.51l-1.6-2.77c-.1-.18-.31-.24-.49-.18l-1.99.8c-.42-.32-.86-.58-1.35-.78L12
-  2.34c-.03-.2-.2-.34-.4-.34H8.4c-.2 0-.36.14-.39.34l-.3 2.12c-.49.2-.94.47-1.35.78l-1.99-.8c-.18-.07-.39
-  0-.49.18l-1.6 2.77c-.1.18-.06.39.1.51l1.69
-  1.32c-.04.25-.07.52-.07.78s.02.53.06.78L2.37 12.1c-.15.12-.19.34-.1.51l1.6 2.77c.1.18.31.24.49.18l1.99-.8c.42.32.86.58
-  1.35.78l.3 2.12c.04.2.2.34.4.34h3.2c.2 0 .37-.14.39-.34l.3-2.12c.49-.2.94-.47 1.35-.78l1.99.8c.18.07.39 0
-  .49-.18l1.6-2.77c.1-.18.06-.39-.1-.51l-1.67-1.32zM10 13c-1.65 0-3-1.35-3-3s1.35-3 3-3 3 1.35 3 3-1.35 3-3 3z
+  M19.43 12.98c.04-.32.07-.64.07-.98s-.03-.66-.07-.98l2.11-1.65c.19-.15.24-.42.12-.64l-2-3.46c-.12-.22-.39-.3-.61-.22l-2.49
+  1c-.52-.4-1.08-.73-1.69-.98l-.38-2.65C14.46 2.18 14.25 2 14 2h-4c-.25 0-.46.18-.49.42l-.38
+  2.65c-.61.25-1.17.59-1.69.98l-2.49-1c-.23-.09-.49 0-.61.22l-2 3.46c-.13.22-.07.49.12.64l2.11
+  1.65c-.04.32-.07.65-.07.98s.03.66.07.98l-2.11 1.65c-.19.15-.24.42-.12.64l2 3.46c.12.22.39.3.61.22l2.49-1c.52.4
+  1.08.73 1.69.98l.38 2.65c.03.24.24.42.49.42h4c.25 0 .46-.18.49-.42l.38-2.65c.61-.25 1.17-.59 1.69-.98l2.49
+  1c.23.09.49 0 .61-.22l2-3.46c.12-.22.07-.49-.12-.64l-2.11-1.65zM12 15.5c-1.93 0-3.5-1.57-3.5-3.5s1.57-3.5
+  3.5-3.5 3.5 1.57 3.5 3.5-1.57 3.5-3.5 3.5z
 `;
 
 const PATH_SUSPEND = `
diff --git a/src/devtools/views/Components/Components.js b/src/devtools/views/Components/Components.js
@@ -13,7 +13,7 @@ import { SettingsModalContextController } from 'src/devtools/views/Settings/Sett
 
 import styles from './Components.css';
 
-function Components(_: {||}) {
+function Components({ isBrowserMode }: {| isBrowserMode?: boolean |}) {
   // TODO Flex wrappers below should be user resizable.
   return (
     <SettingsModalContextController>
@@ -26,7 +26,7 @@ function Components(_: {||}) {
             <div className={styles.SelectedElementWrapper}>
               <NativeStyleContextController>
                 <Suspense fallback={<Loading />}>
-                  <SelectedElement />
+                  <SelectedElement isBrowserMode={isBrowserMode} />
                 </Suspense>
               </NativeStyleContextController>
             </div>
diff --git a/src/devtools/views/Components/SelectedElement.css b/src/devtools/views/Components/SelectedElement.css
@@ -74,7 +74,6 @@
 }
 
 .CannotSuspendWarningMessage {
-  font-size: var(--font-size-sans-large);
 }
 
 .NotInStore {
diff --git a/src/devtools/views/Components/SelectedElement.js b/src/devtools/views/Components/SelectedElement.js
@@ -30,9 +30,11 @@ import type { GetInspectedElementPath } from './InspectedElementContext';
 import type { Element, InspectedElement } from './types';
 import type { ElementType } from 'src/types';
 
-export type Props = {||};
+export type Props = {|
+  isBrowserMode?: boolean,
+|};
 
-export default function SelectedElement(_: Props) {
+export default function SelectedElement({ isBrowserMode }: Props) {
   const { inspectedElementID } = useContext(TreeStateContext);
   const dispatch = useContext(TreeDispatcherContext);
   const { isFileLocationRequired, viewElementSourceFunction } = useContext(
@@ -186,13 +188,15 @@ export default function SelectedElement(_: Props) {
             <ButtonIcon type="suspend" />
           </Toggle>
         )}
-        <Button
-          className={styles.IconButton}
-          onClick={highlightElement}
-          title="Inspect the matching DOM element"
-        >
-          <ButtonIcon type="view-dom" />
-        </Button>
+        {isBrowserMode && (
+          <Button
+            className={styles.IconButton}
+            onClick={highlightElement}
+            title="Inspect the matching DOM element"
+          >
+            <ButtonIcon type="view-dom" />
+          </Button>
+        )}
         <Button
           className={styles.IconButton}
           onClick={logElement}
diff --git a/src/devtools/views/DevTools.js b/src/devtools/views/DevTools.js
@@ -37,6 +37,7 @@ export type Props = {|
   bridge: FrontendBridge,
   browserTheme?: BrowserTheme,
   defaultTab?: TabID,
+  isBrowserMode?: boolean,
   showTabBar?: boolean,
   store: Store,
   warnIfLegacyBackendDetected?: boolean,
@@ -77,6 +78,7 @@ export default function DevTools({
   browserTheme = 'light',
   defaultTab = 'components',
   componentsPortalContainer,
+  isBrowserMode = true,
   overrideTab,
   profilerPortalContainer,
   settingsPortalContainer,
@@ -124,16 +126,19 @@ export default function DevTools({
                           currentTab={tab}
                           id="DevTools"
                           selectTab={setTab}
-                          size="large"
                           tabs={tabs}
+                          type="navigation"
                         />
                       </div>
                     )}
                     <div
                       className={styles.TabContent}
                       hidden={tab !== 'components'}
                     >
-                      <Components portalContainer={componentsPortalContainer} />
+                      <Components
+                        isBrowserMode={isBrowserMode}
+                        portalContainer={componentsPortalContainer}
+                      />
                     </div>
                     <div
                       className={styles.TabContent}
diff --git a/src/devtools/views/Icon.js b/src/devtools/views/Icon.js
@@ -101,10 +101,11 @@ const PATH_SEARCH = `
 const PATH_RANKED_CHART = 'M3 5h18v3H3zM3 10.5h13v3H3zM3 16h8v3H3z';
 
 const PATH_SETTINGS = `
-  M15.95 10.78c.03-.25.05-.51.05-.78s-.02-.53-.06-.78l1.69-1.32c.15-.12.19-.34.1-.51l-1.6-2.77c-.1-.18-.31-.24-.49-.18l-1.99.8c-.42-.32-.86-.58-1.35-.78L12
-  2.34c-.03-.2-.2-.34-.4-.34H8.4c-.2 0-.36.14-.39.34l-.3 2.12c-.49.2-.94.47-1.35.78l-1.99-.8c-.18-.07-.39
-  0-.49.18l-1.6 2.77c-.1.18-.06.39.1.51l1.69
-  1.32c-.04.25-.07.52-.07.78s.02.53.06.78L2.37 12.1c-.15.12-.19.34-.1.51l1.6 2.77c.1.18.31.24.49.18l1.99-.8c.42.32.86.58
-  1.35.78l.3 2.12c.04.2.2.34.4.34h3.2c.2 0 .37-.14.39-.34l.3-2.12c.49-.2.94-.47 1.35-.78l1.99.8c.18.07.39 0
-  .49-.18l1.6-2.77c.1-.18.06-.39-.1-.51l-1.67-1.32zM10 13c-1.65 0-3-1.35-3-3s1.35-3 3-3 3 1.35 3 3-1.35 3-3 3z
+  M19.43 12.98c.04-.32.07-.64.07-.98s-.03-.66-.07-.98l2.11-1.65c.19-.15.24-.42.12-.64l-2-3.46c-.12-.22-.39-.3-.61-.22l-2.49
+  1c-.52-.4-1.08-.73-1.69-.98l-.38-2.65C14.46 2.18 14.25 2 14 2h-4c-.25 0-.46.18-.49.42l-.38
+  2.65c-.61.25-1.17.59-1.69.98l-2.49-1c-.23-.09-.49 0-.61.22l-2 3.46c-.13.22-.07.49.12.64l2.11
+  1.65c-.04.32-.07.65-.07.98s.03.66.07.98l-2.11 1.65c-.19.15-.24.42-.12.64l2 3.46c.12.22.39.3.61.22l2.49-1c.52.4
+  1.08.73 1.69.98l.38 2.65c.03.24.24.42.49.42h4c.25 0 .46-.18.49-.42l.38-2.65c.61-.25 1.17-.59 1.69-.98l2.49
+  1c.23.09.49 0 .61-.22l2-3.46c.12-.22.07-.49-.12-.64l-2.11-1.65zM12 15.5c-1.93 0-3.5-1.57-3.5-3.5s1.57-3.5
+  3.5-3.5 3.5 1.57 3.5 3.5-1.57 3.5-3.5 3.5z
 `;
diff --git a/src/devtools/views/ModalDialog.css b/src/devtools/views/ModalDialog.css
@@ -18,6 +18,7 @@
   max-width: 100%;
   display: inline-block;
   background-color: var(--color-background);
+  box-shadow: 0 2px 4px var(--color-shadow);
   padding: 0.5rem;
   border: 1px solid var(--color-border);
   border-radius: 0.25rem;
diff --git a/src/devtools/views/Profiler/Profiler.js b/src/devtools/views/Profiler/Profiler.js
@@ -92,8 +92,8 @@ function Profiler(_: {||}) {
               currentTab={selectedTabID}
               id="Profiler"
               selectTab={selectTab}
-              size="small"
               tabs={tabs}
+              type="profiler"
             />
             <RootSelector />
             <div className={styles.Spacer} />
diff --git a/src/devtools/views/Settings/ComponentsSettings.js b/src/devtools/views/Settings/ComponentsSettings.js
@@ -61,7 +61,7 @@ export default function ComponentsSettings(_: {||}) {
 
   const updateCollapseNodesByDefault = useCallback(
     ({ currentTarget }) => {
-      store.collapseNodesByDefault = currentTarget.checked;
+      store.collapseNodesByDefault = !currentTarget.checked;
     },
     [store]
   );
@@ -240,10 +240,10 @@ export default function ComponentsSettings(_: {||}) {
       <label className={styles.Setting}>
         <input
           type="checkbox"
-          checked={collapseNodesByDefault}
+          checked={!collapseNodesByDefault}
           onChange={updateCollapseNodesByDefault}
         />{' '}
-        Collapse newly added components by default
+        Expand component tree by default
       </label>
 
       <div className={styles.Header}>Hide components where...</div>
diff --git a/src/devtools/views/Settings/SettingsContext.js b/src/devtools/views/Settings/SettingsContext.js
@@ -308,6 +308,7 @@ function updateThemeVariables(
   updateStyleHelper(theme, 'color-color-scroll-thumb', documentElements);
   updateStyleHelper(theme, 'color-color-scroll-track', documentElements);
   updateStyleHelper(theme, 'color-search-match', documentElements);
+  updateStyleHelper(theme, 'color-shadow', documentElements);
   updateStyleHelper(theme, 'color-search-match-current', documentElements);
   updateStyleHelper(
     theme,
diff --git a/src/devtools/views/Settings/SettingsModal.css b/src/devtools/views/Settings/SettingsModal.css
@@ -1,5 +1,6 @@
 .Background {
   position: absolute;
+  z-index: 3;
   width: 100%;
   top: 0;
   bottom: 0;
@@ -18,6 +19,7 @@
   max-height: 100%;
   background-color: var(--color-background);
   border: 1px solid var(--color-border);
+  box-shadow: 0 2px 4px var(--color-shadow);
   border-radius: 0.25rem;
   overflow: auto;
   width: 400px;
diff --git a/src/devtools/views/Settings/SettingsModal.js b/src/devtools/views/Settings/SettingsModal.js
@@ -100,8 +100,8 @@ function SettingsModalImpl(_: {||}) {
             currentTab={selectedTabID}
             id="Settings"
             selectTab={selectTab}
-            size="small"
             tabs={tabs}
+            type="settings"
           />
           <div className={styles.Spacer} />
           <Button onClick={dismissModal} title="Close settings dialog">
diff --git a/src/devtools/views/TabBar.css b/src/devtools/views/TabBar.css
diff --git a/src/devtools/views/TabBar.js b/src/devtools/views/TabBar.js
diff --git a/src/devtools/views/root.css b/src/devtools/views/root.css

Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,6 @@`
`74`	`74`	`}`
`75`	`75`
`76`	`76`	`.CannotSuspendWarningMessage {`
`77`		`- font-size: var(--font-size-sans-large);`
`78`	`77`	`}`
`79`	`78`
`80`	`79`	`.NotInStore {`