From b5195a5f16fb900c924401066ee453e6ef53baea Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 12 Aug 2019 08:26:48 -0700 Subject: [PATCH] Fixed a couple of edge case styling and layout issues I noticed while testing the inline target --- package.json | 1 + src/constants.js | 18 +++++++ src/devtools/views/Components/SearchInput.css | 6 ++- src/devtools/views/Components/Tree.js | 4 ++ .../views/Profiler/ProfilerContext.js | 4 +- .../views/Profiler/ReloadAndProfileButton.js | 13 +++-- .../views/Settings/SettingsContext.js | 50 +++---------------- yarn.lock | 16 ++++++ 8 files changed, 61 insertions(+), 51 deletions(-) diff --git a/package.json b/package.json index 1540785118c9c..c0c5ffadc0dbc 100644 --- a/package.json +++ b/package.json @@ -139,6 +139,7 @@ "opener": "^1.5.1", "prettier": "^1.16.4", "prop-types": "^15.6.2", + "raw-loader": "^3.1.0", "react": "^0.0.0-424099da6", "react-15": "npm:react@^15", "react-color": "^2.11.7", diff --git a/src/constants.js b/src/constants.js index d18624fbf04fa..daef7b0e70bf3 100644 --- a/src/constants.js +++ b/src/constants.js @@ -1,8 +1,26 @@ // @flow +// $FlowFixMe Cannot resolve module +import rawStyleString from '!!raw-loader!src/devtools/views/root.css'; // eslint-disable-line import/no-webpack-loader-syntax + // Flip this flag to true to enable verbose console debug logging. export const __DEBUG__ = false; +const extractVar = varName => { + const regExp = new RegExp(`${varName}: ([0-9]+)`); + const match = rawStyleString.match(regExp); + return parseInt(match[1], 10); +}; + +// TRICKY +// Extracting during build time avoids a temporarily invalid state for the inline target. +// Sometimes the inline target is rendered before root styles are applied, +// which would result in e.g. NaN itemSize being passed to react-window list. +export const COMFORTABLE_LINE_HEIGHT = extractVar( + 'comfortable-line-height-data' +); +export const COMPACT_LINE_HEIGHT = extractVar('compact-line-height-data'); + export const TREE_OPERATION_ADD = 1; export const TREE_OPERATION_REMOVE = 2; export const TREE_OPERATION_REORDER_CHILDREN = 3; diff --git a/src/devtools/views/Components/SearchInput.css b/src/devtools/views/Components/SearchInput.css index ddd1e70825b70..5db009aa13702 100644 --- a/src/devtools/views/Components/SearchInput.css +++ b/src/devtools/views/Components/SearchInput.css @@ -1,11 +1,12 @@ .SearchInput { - flex: 1; + flex: 1 1; display: flex; align-items: center; } .Input { - flex: 1; + flex: 1 1 100px; + width: 100px; font-size: var(--font-size-sans-large); outline: none; border: none; @@ -24,6 +25,7 @@ .IndexLabel { color: var(--color-dim); font-size: var(--font-size-sans-normal); + white-space: pre; } .LeftVRule, diff --git a/src/devtools/views/Components/Tree.js b/src/devtools/views/Components/Tree.js index 285cb39c41655..3f807b16b427d 100644 --- a/src/devtools/views/Components/Tree.js +++ b/src/devtools/views/Components/Tree.js @@ -104,6 +104,10 @@ export default function Tree(props: Props) { return; } + // TODO We should ignore arrow keys if the focus is outside of DevTools. + // Otherwise the inline (embedded) DevTools might change selection unexpectedly, + // e.g. when a text input or a select has focus. + let element; switch (event.key) { case 'ArrowDown': diff --git a/src/devtools/views/Profiler/ProfilerContext.js b/src/devtools/views/Profiler/ProfilerContext.js index 9c5f1ed2b6782..f50947e1cf778 100644 --- a/src/devtools/views/Profiler/ProfilerContext.js +++ b/src/devtools/views/Profiler/ProfilerContext.js @@ -33,8 +33,8 @@ export type Context = {| isProcessingData: boolean, isProfiling: boolean, profilingData: ProfilingDataFrontend | null, - startProfiling(value: boolean): void, - stopProfiling(value: boolean): void, + startProfiling(): void, + stopProfiling(): void, supportsProfiling: boolean, // Which root should profiling data be shown for? diff --git a/src/devtools/views/Profiler/ReloadAndProfileButton.js b/src/devtools/views/Profiler/ReloadAndProfileButton.js index 933d453ab04f0..45f805048d93f 100644 --- a/src/devtools/views/Profiler/ReloadAndProfileButton.js +++ b/src/devtools/views/Profiler/ReloadAndProfileButton.js @@ -6,6 +6,7 @@ import ButtonIcon from '../ButtonIcon'; import { BridgeContext, StoreContext } from '../context'; import { useSubscription } from '../hooks'; import Store from 'src/devtools/store'; +import { ProfilerContext } from './ProfilerContext'; type SubscriptionData = {| recordChangeDescriptions: boolean, @@ -16,6 +17,8 @@ export default function ReloadAndProfileButton() { const bridge = useContext(BridgeContext); const store = useContext(StoreContext); + const { startProfiling } = useContext(ProfilerContext); + const subscription = useMemo( () => ({ getCurrentValue: () => ({ @@ -38,10 +41,12 @@ export default function ReloadAndProfileButton() { supportsReloadAndProfile, } = useSubscription(subscription); - const reloadAndProfile = useCallback( - () => bridge.send('reloadAndProfile', recordChangeDescriptions), - [bridge, recordChangeDescriptions] - ); + const reloadAndProfile = useCallback(() => { + bridge.send('reloadAndProfile', recordChangeDescriptions); + + // In case the DevTools UI itself doesn't reload along with the app, also start profiling. + startProfiling(); + }, [bridge, recordChangeDescriptions, startProfiling]); if (!supportsReloadAndProfile) { return null; diff --git a/src/devtools/views/Settings/SettingsContext.js b/src/devtools/views/Settings/SettingsContext.js index ac55069a6c19f..62f3ae01da832 100644 --- a/src/devtools/views/Settings/SettingsContext.js +++ b/src/devtools/views/Settings/SettingsContext.js @@ -6,9 +6,12 @@ import React, { useEffect, useLayoutEffect, useMemo, - useState, } from 'react'; -import { LOCAL_STORAGE_SHOULD_PATCH_CONSOLE_KEY } from 'src/constants'; +import { + COMFORTABLE_LINE_HEIGHT, + COMPACT_LINE_HEIGHT, + LOCAL_STORAGE_SHOULD_PATCH_CONSOLE_KEY, +} from 'src/constants'; import { useLocalStorage } from '../hooks'; import { BridgeContext } from '../context'; @@ -37,11 +40,6 @@ SettingsContext.displayName = 'SettingsContext'; type DocumentElements = Array; -type CurrentLineHeights = {| - comfortableLineHeight: number, - compactLineHeight: number, -|}; - type Props = {| browserTheme: BrowserTheme, children: React$Node, @@ -89,25 +87,6 @@ function SettingsContextController({ return array; }, [componentsPortalContainer, profilerPortalContainer]); - const [ - currentLineHeights, - setCurrentLineHeights, - ] = useState(getCurrentLineHeights); - - // In case the root styles have not yet been applied, wait a bit and then check again. - // This avoids a bad case of NaN line heights in lists/trees. - useEffect(() => { - if (Number.isNaN(currentLineHeights.compactLineHeight)) { - const intervalID = setInterval(() => { - const newLineHeights = getCurrentLineHeights(); - if (!Number.isNaN(newLineHeights.compactLineHeight)) { - setCurrentLineHeights(newLineHeights); - } - }, 100); - return () => clearInterval(intervalID); - } - }, [currentLineHeights]); - useLayoutEffect(() => { switch (displayDensity) { case 'comfortable': @@ -151,11 +130,10 @@ function SettingsContextController({ setAppendComponentStack, lineHeight: displayDensity === 'compact' - ? currentLineHeights.compactLineHeight - : currentLineHeights.comfortableLineHeight, + ? COMPACT_LINE_HEIGHT + : COMFORTABLE_LINE_HEIGHT, }), [ - currentLineHeights, displayDensity, setDisplayDensity, setTheme, @@ -172,20 +150,6 @@ function SettingsContextController({ ); } -function getCurrentLineHeights(): CurrentLineHeights { - const computedStyle = getComputedStyle((document.body: any)); - return { - comfortableLineHeight: parseInt( - computedStyle.getPropertyValue('--comfortable-line-height-data'), - 10 - ), - compactLineHeight: parseInt( - computedStyle.getPropertyValue('--compact-line-height-data'), - 10 - ), - }; -} - function setStyleVariable( name: string, value: string, diff --git a/yarn.lock b/yarn.lock index 86531c672e4a1..df60e608338a1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9931,6 +9931,14 @@ raw-body@2.3.3: iconv-lite "0.4.23" unpipe "1.0.0" +raw-loader@^3.1.0: + version "3.1.0" + resolved "https://registry.yarnpkg.com/raw-loader/-/raw-loader-3.1.0.tgz#5e9d399a5a222cc0de18f42c3bc5e49677532b3f" + integrity sha512-lzUVMuJ06HF4rYveaz9Tv0WRlUMxJ0Y1hgSkkgg+50iEdaI0TthyEDe08KIHb0XsF6rn8WYTqPCaGTZg3sX+qA== + dependencies: + loader-utils "^1.1.0" + schema-utils "^2.0.1" + rc@^1.0.1, rc@^1.1.6, rc@^1.2.1, rc@^1.2.7: version "1.2.8" resolved "https://registry.yarnpkg.com/rc/-/rc-1.2.8.tgz#cd924bf5200a075b83c188cd6b9e211b7fc0d3ed" @@ -10715,6 +10723,14 @@ schema-utils@^1.0.0: ajv-errors "^1.0.0" ajv-keywords "^3.1.0" +schema-utils@^2.0.1: + version "2.1.0" + resolved "https://registry.yarnpkg.com/schema-utils/-/schema-utils-2.1.0.tgz#940363b6b1ec407800a22951bdcc23363c039393" + integrity sha512-g6SViEZAfGNrToD82ZPUjq52KUPDYc+fN5+g6Euo5mLokl/9Yx14z0Cu4RR1m55HtBXejO0sBt+qw79axN+Fiw== + dependencies: + ajv "^6.1.0" + ajv-keywords "^3.1.0" + select-hose@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/select-hose/-/select-hose-2.0.0.tgz#625d8658f865af43ec962bfc376a37359a4994ca"