From 9fc2a3400cddbb00448c5e57c10ca349f2b3e4c8 Mon Sep 17 00:00:00 2001 From: Dan Date: Sat, 6 Apr 2019 21:36:08 +0100 Subject: [PATCH 01/36] Remove dead code --- src/backend/renderer.js | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/backend/renderer.js b/src/backend/renderer.js index 5d3c5b9991..ebc5b0684a 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -810,21 +810,6 @@ export function attach( } } - function unmountFiberRecursively(fiber, traverseSiblings = false) { - if (__DEBUG__) { - debug('unmountFiberRecursively()', fiber, traverseSiblings); - } - if (!shouldFilterFiber(fiber)) { - recordUnmount(fiber); - } - if (fiber.child !== null) { - unmountFiberRecursively(fiber.child, true); - } - if (traverseSiblings && fiber.sibling !== null) { - unmountFiberRecursively(fiber.sibling, true); - } - } - function maybeRecordUpdate(fiber: Fiber, hasChildOrderChanged: boolean) { if (__DEBUG__) { debug('maybeRecordUpdate()', fiber); From d766d3b2e554e2998fbc3b30c6585520c75d95f3 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sat, 6 Apr 2019 13:58:55 -0700 Subject: [PATCH 02/36] Throttle screenshots to reduce impact on performance. --- package.json | 3 ++- src/devtools/store.js | 20 ++++++++++---- .../views/Profiler/SidebarCommitInfo.css | 10 +++++++ .../views/Profiler/SidebarCommitInfo.js | 26 +++++++++++++------ src/devtools/views/Settings/Settings.css | 6 +++++ src/devtools/views/Settings/Settings.js | 24 ++++++++++------- yarn.lock | 5 ++++ 7 files changed, 70 insertions(+), 24 deletions(-) diff --git a/package.json b/package.json index e843d0f133..cfdf5851a6 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,6 @@ "cli-spinners": "^1.0.0", "clipboard-js": "^0.3.6", "css-loader": "^1.0.1", - "html2canvas": "^1.0.0-alpha.12", "error-stack-parser": "^2.0.2", "es6-symbol": "3.0.2", "escape-string-regexp": "^1.0.5", @@ -81,10 +80,12 @@ "flow-bin": "^0.94.0", "fs-extra": "^3.0.1", "gh-pages": "^1.0.0", + "html2canvas": "^1.0.0-alpha.12", "immutable": "3.7.6", "jest": "22.1.4", "lerna": "^2.8.0", "lint-staged": "^7.0.5", + "lodash.throttle": "^4.1.1", "log-update": "^2.0.0", "lru-cache": "^4.1.3", "memoize-one": "^3.1.1", diff --git a/src/devtools/store.js b/src/devtools/store.js index 69dbf45133..225f9a8ac8 100644 --- a/src/devtools/store.js +++ b/src/devtools/store.js @@ -1,6 +1,8 @@ // @flow import EventEmitter from 'events'; +import memoize from 'memoize-one'; +import throttle from 'lodash.throttle'; import { TREE_OPERATION_ADD, TREE_OPERATION_RECURSIVE_REMOVE_CHILDREN, @@ -35,6 +37,8 @@ const debug = (methodName, ...args) => { const LOCAL_STORAGE_CAPTURE_SCREENSHOTS_KEY = 'React::DevTools::captureScreenshots'; +const THROTTLE_CAPTURE_SCREENSHOT_DURATION = 500; + type Config = {| isProfiling?: boolean, supportsCaptureScreenshots?: boolean, @@ -129,8 +133,8 @@ export default class Store extends EventEmitter { if (supportsCaptureScreenshots) { this._supportsCaptureScreenshots = true; this._captureScreenshots = - localStorage.getItem(LOCAL_STORAGE_CAPTURE_SCREENSHOTS_KEY) !== - 'false'; + localStorage.getItem(LOCAL_STORAGE_CAPTURE_SCREENSHOTS_KEY) === + 'true'; } if (supportsFileDownloads) { this._supportsFileDownloads = true; @@ -416,6 +420,13 @@ export default class Store extends EventEmitter { this.emit('isProfiling'); } + _captureScreenshot = throttle( + memoize((commitIndex: number) => { + this._bridge.send('captureScreenshot', { commitIndex }); + }), + THROTTLE_CAPTURE_SCREENSHOT_DURATION + ); + _takeProfilingSnapshotRecursive = (id: number) => { const element = this.getElementByID(id); if (element !== null) { @@ -454,10 +465,9 @@ export default class Store extends EventEmitter { profilingOperations.push(operations); } - const commitIndex = profilingOperations.length - 1; - if (this._captureScreenshots) { - this._bridge.send('captureScreenshot', { commitIndex }); + const commitIndex = profilingOperations.length - 1; + this._captureScreenshot(commitIndex); } } diff --git a/src/devtools/views/Profiler/SidebarCommitInfo.css b/src/devtools/views/Profiler/SidebarCommitInfo.css index a9bd56a2e7..6e99ac4ff3 100644 --- a/src/devtools/views/Profiler/SidebarCommitInfo.css +++ b/src/devtools/views/Profiler/SidebarCommitInfo.css @@ -79,3 +79,13 @@ border: 1px solid var(--color-border); border-radius: 0.5rem; } + +.NoScreenshot { + width: 100%; + height: 200px; + background-color: var(--color-button-background-focus); + border-radius: 0.25rem; + display: flex; + align-items: center; + justify-content: center; +} diff --git a/src/devtools/views/Profiler/SidebarCommitInfo.js b/src/devtools/views/Profiler/SidebarCommitInfo.js index 86debc8e04..a786d6e8db 100644 --- a/src/devtools/views/Profiler/SidebarCommitInfo.js +++ b/src/devtools/views/Profiler/SidebarCommitInfo.js @@ -18,7 +18,11 @@ export default function SidebarCommitInfo(_: Props) { selectTab, } = useContext(ProfilerContext); - const { profilingCache, profilingScreenshots } = useContext(StoreContext); + const { + captureScreenshots, + profilingCache, + profilingScreenshots, + } = useContext(StoreContext); const screenshot = selectedCommitIndex !== null @@ -97,14 +101,20 @@ export default function SidebarCommitInfo(_: Props) { ))} - {screenshot != null && ( + {captureScreenshots && (
  • - Screenshot + {screenshot != null ? ( + Screenshot + ) : ( +
    + No screenshot available +
    + )}
    • )} {screenshot != null && isScreenshotModalVisible && ( diff --git a/src/devtools/views/Settings/Settings.css b/src/devtools/views/Settings/Settings.css index 1b29a756a5..01afd5a23d 100644 --- a/src/devtools/views/Settings/Settings.css +++ b/src/devtools/views/Settings/Settings.css @@ -51,3 +51,9 @@ border-bottom-right-radius: 0.25rem; border-right: 1px solid var(--color-border); } + +.ScreenshotThrottling { + background-color: var(--color-hover-background); + padding: 0.25rem 0.5rem; + border-radius: 0.25rem; +} diff --git a/src/devtools/views/Settings/Settings.js b/src/devtools/views/Settings/Settings.js index 808697ea4c..f647162a8a 100644 --- a/src/devtools/views/Settings/Settings.js +++ b/src/devtools/views/Settings/Settings.js @@ -117,16 +117,20 @@ export default function Settings({ portalContainer }: Props) { {store.supportsCaptureScreenshots && (
    Profiler
    -
    - -
    +
    )} diff --git a/yarn.lock b/yarn.lock index 622ecd77fa..b92e1ee9d8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7053,6 +7053,11 @@ lodash.templatesettings@^4.0.0: dependencies: lodash._reinterpolate "~3.0.0" +lodash.throttle@^4.1.1: + version "4.1.1" + resolved "https://registry.yarnpkg.com/lodash.throttle/-/lodash.throttle-4.1.1.tgz#c23e91b710242ac70c37f1e1cda9274cc39bf2f4" + integrity sha1-wj6RtxAkKscMN/HhzaknTMOb8vQ= + lodash@3.10.1, lodash@^3.10.0: version "3.10.1" resolved "https://registry.yarnpkg.com/lodash/-/lodash-3.10.1.tgz#5bf45e8e49ba4189e17d482789dfd15bd140b7b6" From 5334249bda1917ead38860b2272af2169bb7036d Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sat, 6 Apr 2019 18:01:44 -0700 Subject: [PATCH 03/36] Fixed a typo in flamegraph duration label --- src/devtools/views/Profiler/ChartNode.css | 8 ++++---- src/devtools/views/Profiler/FlamegraphChartBuilder.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/devtools/views/Profiler/ChartNode.css b/src/devtools/views/Profiler/ChartNode.css index e9ebea1a7a..3117ea1932 100644 --- a/src/devtools/views/Profiler/ChartNode.css +++ b/src/devtools/views/Profiler/ChartNode.css @@ -18,10 +18,10 @@ white-space: nowrap; text-overflow: ellipsis; overflow: hidden; - font-size: var(--font-family-sans); - font-family: var(--font-size-sans-normal); - margin-left: 4px; - margin-right: 4px; + font-family: var(--font-family-sans); + font-size: var(--font-size-sans-normal); + margin-left: 0.25rem; + margin-right: 0.25rem; line-height: 1.5; padding: 0 0 0; font-weight: 400; diff --git a/src/devtools/views/Profiler/FlamegraphChartBuilder.js b/src/devtools/views/Profiler/FlamegraphChartBuilder.js index 927f129181..9478c42d7e 100644 --- a/src/devtools/views/Profiler/FlamegraphChartBuilder.js +++ b/src/devtools/views/Profiler/FlamegraphChartBuilder.js @@ -69,7 +69,7 @@ export function getChartData({ let label = `${name}${maybeKey}`; if (didRender) { - label += ` (${selfDuration.toFixed(1)}ms) of ${actualDuration.toFixed( + label += ` (${selfDuration.toFixed(1)}ms of ${actualDuration.toFixed( 1 )}ms)`; } From ce04f531d41ca79640ded42e6d1f9736fd8f9e67 Mon Sep 17 00:00:00 2001 From: Hristo Kanchev Date: Sun, 7 Apr 2019 11:37:12 +0200 Subject: [PATCH 04/36] Implemented new OwnerStack UI enhancement --- .flowconfig | 1 - .gitignore | 3 +- src/backend/views/Overlay.js | 19 +- src/devtools/views/ButtonIcon.js | 7 + src/devtools/views/Components/OwnersStack.css | 44 ++++- src/devtools/views/Components/OwnersStack.js | 173 ++++++++++++++++-- src/utils.js | 18 ++ 7 files changed, 226 insertions(+), 39 deletions(-) diff --git a/.flowconfig b/.flowconfig index 35306bd54a..c1209df273 100644 --- a/.flowconfig +++ b/.flowconfig @@ -4,7 +4,6 @@ .*node_modules/archiver-utils .*node_modules/babel.* .*node_modules/browserify-zlib/.* -.*node_modules/classnames.* .*node_modules/gh-pages/.* .*node_modules/invariant/.* .*node_modules/json-loader.* diff --git a/.gitignore b/.gitignore index 514ad873cd..ee4e445f06 100644 --- a/.gitignore +++ b/.gitignore @@ -7,4 +7,5 @@ npm-debug.log yarn-error.log .DS_Store yarn-error.log -.vscode \ No newline at end of file +.vscode +.idea diff --git a/src/backend/views/Overlay.js b/src/backend/views/Overlay.js index 24b0933a43..a54547e525 100644 --- a/src/backend/views/Overlay.js +++ b/src/backend/views/Overlay.js @@ -1,6 +1,7 @@ // @flow import assign from 'object-assign'; +import { getElementDimensions } from '../../utils'; type Rect = { bottom: number, @@ -215,24 +216,6 @@ function findTipPos(dims, win) { return { top, left: dims.left + margin + 'px' }; } -function getElementDimensions(domElement) { - const calculatedStyle = window.getComputedStyle(domElement); - return { - borderLeft: +calculatedStyle.borderLeftWidth.match(/[0-9]*/)[0], - borderRight: +calculatedStyle.borderRightWidth.match(/[0-9]*/)[0], - borderTop: +calculatedStyle.borderTopWidth.match(/[0-9]*/)[0], - borderBottom: +calculatedStyle.borderBottomWidth.match(/[0-9]*/)[0], - marginLeft: +calculatedStyle.marginLeft.match(/[0-9]*/)[0], - marginRight: +calculatedStyle.marginRight.match(/[0-9]*/)[0], - marginTop: +calculatedStyle.marginTop.match(/[0-9]*/)[0], - marginBottom: +calculatedStyle.marginBottom.match(/[0-9]*/)[0], - paddingLeft: +calculatedStyle.paddingLeft.match(/[0-9]*/)[0], - paddingRight: +calculatedStyle.paddingRight.match(/[0-9]*/)[0], - paddingTop: +calculatedStyle.paddingTop.match(/[0-9]*/)[0], - paddingBottom: +calculatedStyle.paddingBottom.match(/[0-9]*/)[0], - }; -} - // Get the window object for the document that a node belongs to, // or return null if it cannot be found (node not attached to DOM, // etc). diff --git a/src/devtools/views/ButtonIcon.js b/src/devtools/views/ButtonIcon.js index 6d423dca00..7558ddfec8 100644 --- a/src/devtools/views/ButtonIcon.js +++ b/src/devtools/views/ButtonIcon.js @@ -7,6 +7,7 @@ export type IconType = | 'back' | 'cancel' | 'close' + | 'colon' | 'copy' | 'down' | 'export' @@ -39,6 +40,9 @@ export default function ButtonIcon({ type }: Props) { case 'close': pathData = PATH_CLOSE; break; + case 'colon': + pathData = PATH_COLON; + break; case 'copy': pathData = PATH_COPY; break; @@ -117,6 +121,9 @@ const PATH_CANCEL = ` const PATH_CLOSE = 'M19 6.41L17.59 5 12 10.59 6.41 5 5 6.41 10.59 12 5 17.59 6.41 19 12 13.41 17.59 19 19 17.59 13.41 12z'; +const PATH_COLON = + 'M10,9a2,2 0 1,0 4,0a2,2 0 1,0 -4,0 M10,19a2,2 0 1,0 4,0a2,2 0 1,0 -4,0'; + const PATH_COPY = ` M3 13h2v-2H3v2zm0 4h2v-2H3v2zm2 4v-2H3a2 2 0 0 0 2 2zM3 9h2V7H3v2zm12 12h2v-2h-2v2zm4-18H9a2 2 0 0 0-2 2v10a2 2 0 0 0 2 2h10c1.1 0 2-.9 2-2V5c0-1.1-.9-2-2-2zm0 12H9V5h10v10zm-8 6h2v-2h-2v2zm-4 0h2v-2H7v2z diff --git a/src/devtools/views/Components/OwnersStack.css b/src/devtools/views/Components/OwnersStack.css index 2f0ebbd2a8..2af09bc30e 100644 --- a/src/devtools/views/Components/OwnersStack.css +++ b/src/devtools/views/Components/OwnersStack.css @@ -2,7 +2,6 @@ flex: 1; display: flex; align-items: center; - overflow-x: auto; } .Component, @@ -12,7 +11,6 @@ color: var(--color-component-name); font-family: var(--font-family-monospace); font-size: var(--font-size-monospace-normal); - white-space: nowrap; border-radius: 0.125rem; border: none; background: none; @@ -38,7 +36,49 @@ outline: none; } +.ElementsBar { + flex: 1 0 auto; +} + +.ElementsBarSelectedOnly { + margin-left: 0.25rem; +} + +.ElementsBarSelectedOnly .Component { + visibility: hidden; +} +.ElementsBarSelectedOnly .FocusedComponent { + float: left; +} + +.ElementsDropdown { + position: relative; +} + +.Dropdown { + z-index: 1; + position: absolute; + top: calc(100% + 5px); + left: 0; + min-height: 200px; + background-color: var(--color-background); + border: 1px solid var(--color-selected-border); + overflow-y: auto; +} + +.Dropdown .Component, +.Dropdown .FocusedComponent { + display: block; + margin: 0.25rem 0.75rem 0.35rem; +} + +.DropdownButtonActive { + background-color: var(--color-selected-background); + color: var(--color-selected-foreground); +} + .VRule { + flex: 0 0 auto; height: 20px; width: 1px; background-color: var(--color-border); diff --git a/src/devtools/views/Components/OwnersStack.js b/src/devtools/views/Components/OwnersStack.js index 2049d4d5c1..55afea5de3 100644 --- a/src/devtools/views/Components/OwnersStack.js +++ b/src/devtools/views/Components/OwnersStack.js @@ -1,43 +1,84 @@ // @flow - -import React, { useCallback, useContext } from 'react'; +import React, { + useCallback, + useContext, + useEffect, + useState, + createRef, + forwardRef, +} from 'react'; +import classNames from 'classnames'; import Button from '../Button'; import ButtonIcon from '../ButtonIcon'; import { TreeContext } from './TreeContext'; import { StoreContext } from '../context'; +import { getElementDimensions } from '../../../utils'; import type { Element } from './types'; import styles from './OwnersStack.css'; -export default function OwnerStack() { - const { ownerStack, resetOwnerStack } = useContext(TreeContext); +type ElementsDropdownProps = { + selectedElementIndex: number | null, + children: Array, +}; +function ElementsDropdown({ + selectedElementIndex, + children, +}: ElementsDropdownProps) { + const [isDropdownVisible, setIsDropdownVisible] = useState(false); + const handleClick = useCallback(() => { + setIsDropdownVisible(!isDropdownVisible); + }, [isDropdownVisible, setIsDropdownVisible]); - const elements = ownerStack.map((id, index) => ( - - )); + useEffect(() => { + setIsDropdownVisible(false); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [selectedElementIndex]); return ( -
    +
    -
    - {elements} + {isDropdownVisible &&
    {children}
    }
    ); } -type Props = { +type ElementsBarProps = { + elements: Array, + showSelectedOnly: boolean, +}; +const ElementsBar = forwardRef( + ( + { elements, showSelectedOnly }: ElementsBarProps, + ref: Object + ) => { + return ( +
    + {elements} +
    + ); + } +); + +type ElementViewProps = { id: number, index: number, }; - -function ElementView({ id, index }: Props) { +function ElementView({ id, index }: ElementViewProps) { const { ownerStackIndex, selectOwner } = useContext(TreeContext); const store = useContext(StoreContext); const { displayName } = ((store.getElementByID(id): any): Element); @@ -59,3 +100,101 @@ function ElementView({ id, index }: Props) { ); } + +export default function OwnerStack() { + const { ownerStack, ownerStackIndex, resetOwnerStack } = useContext( + TreeContext + ); + const [isElementsBarOverflowing, setIsElementsBarOverflowing] = useState( + false + ); + const [elementsTotalWidth, setElementsTotalWidth] = useState(0); + const elementsBarRef = createRef(); + const elements = ownerStack.map((id, index) => ( + + )); + + useEffect(() => { + if (elementsBarRef.current === null) { + return () => {}; + } + const elements = Array.from(elementsBarRef.current.children); + const elementsTotalWidth = elements.reduce((acc, el) => { + const { offsetWidth } = el; + const { marginRight } = getElementDimensions(el); + return acc + (offsetWidth + marginRight); + }, 0); + + setElementsTotalWidth(elementsTotalWidth); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [ownerStackIndex, elementsBarRef]); + + useElementsBarOverflowing( + elementsBarRef, + elementsTotalWidth, + isElementsBarOverflowing => { + setIsElementsBarOverflowing(isElementsBarOverflowing); + } + ); + + return ( +
    + + {isElementsBarOverflowing && ( + + {elements} + + )} +
    + +
    + ); +} + +function useElementsBarOverflowing( + elementsBarRef: Object, + elementsTotalWidth: number, + callback: Function +) { + const isElementsBarOverflowing = useCallback(() => { + if (elementsBarRef.current !== null) { + const elementsBarWidth = elementsBarRef.current.clientWidth; + return elementsBarWidth <= elementsTotalWidth; + } + return false; + }, [elementsBarRef, elementsTotalWidth]); + + useEffect(() => { + let timeoutID = null; + const handleResize = () => { + callback(isElementsBarOverflowing()); + }; + const debounceHandleResize = () => { + clearTimeout(((timeoutID: any): TimeoutID)); + timeoutID = setTimeout(handleResize, 100); + }; + + handleResize(); + // It's important to listen to the ownerDocument.defaultView to support the browser extension. + // Here we use portals to render individual tabs (e.g. Profiler), + // and the root document might belong to a different window. + const ownerWindow = elementsBarRef.current.ownerDocument.defaultView; + ownerWindow.addEventListener('resize', debounceHandleResize); + return () => { + ownerWindow.removeEventListener('resize', debounceHandleResize); + if (timeoutID !== null) { + clearTimeout(timeoutID); + } + }; + }, [elementsBarRef, isElementsBarOverflowing, callback]); +} diff --git a/src/utils.js b/src/utils.js index 9c2c9189b9..aa79351fc6 100644 --- a/src/utils.js +++ b/src/utils.js @@ -63,3 +63,21 @@ export function utfEncodeString(string: string): Uint32Array { function toCodePoint(string: string) { return string.codePointAt(0); } + +export function getElementDimensions(domElement: Element) { + const calculatedStyle = window.getComputedStyle(domElement); + return { + borderLeft: +calculatedStyle.borderLeftWidth.match(/[0-9]*/)[0], + borderRight: +calculatedStyle.borderRightWidth.match(/[0-9]*/)[0], + borderTop: +calculatedStyle.borderTopWidth.match(/[0-9]*/)[0], + borderBottom: +calculatedStyle.borderBottomWidth.match(/[0-9]*/)[0], + marginLeft: +calculatedStyle.marginLeft.match(/[0-9]*/)[0], + marginRight: +calculatedStyle.marginRight.match(/[0-9]*/)[0], + marginTop: +calculatedStyle.marginTop.match(/[0-9]*/)[0], + marginBottom: +calculatedStyle.marginBottom.match(/[0-9]*/)[0], + paddingLeft: +calculatedStyle.paddingLeft.match(/[0-9]*/)[0], + paddingRight: +calculatedStyle.paddingRight.match(/[0-9]*/)[0], + paddingTop: +calculatedStyle.paddingTop.match(/[0-9]*/)[0], + paddingBottom: +calculatedStyle.paddingBottom.match(/[0-9]*/)[0], + }; +} From 2620050abfce392f53c19bec170d1501a25e49eb Mon Sep 17 00:00:00 2001 From: Hristo Kanchev Date: Sun, 7 Apr 2019 11:48:35 +0200 Subject: [PATCH 05/36] Removed unneeded min-height. Using throttle for resize function. --- src/devtools/views/Components/OwnersStack.css | 1 - src/devtools/views/Components/OwnersStack.js | 10 ++-------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/devtools/views/Components/OwnersStack.css b/src/devtools/views/Components/OwnersStack.css index 2af09bc30e..d2ebdde61e 100644 --- a/src/devtools/views/Components/OwnersStack.css +++ b/src/devtools/views/Components/OwnersStack.css @@ -60,7 +60,6 @@ position: absolute; top: calc(100% + 5px); left: 0; - min-height: 200px; background-color: var(--color-background); border: 1px solid var(--color-selected-border); overflow-y: auto; diff --git a/src/devtools/views/Components/OwnersStack.js b/src/devtools/views/Components/OwnersStack.js index 55afea5de3..295c3fd97d 100644 --- a/src/devtools/views/Components/OwnersStack.js +++ b/src/devtools/views/Components/OwnersStack.js @@ -7,6 +7,7 @@ import React, { createRef, forwardRef, } from 'react'; +import throttle from 'lodash.throttle'; import classNames from 'classnames'; import Button from '../Button'; import ButtonIcon from '../ButtonIcon'; @@ -175,14 +176,10 @@ function useElementsBarOverflowing( }, [elementsBarRef, elementsTotalWidth]); useEffect(() => { - let timeoutID = null; const handleResize = () => { callback(isElementsBarOverflowing()); }; - const debounceHandleResize = () => { - clearTimeout(((timeoutID: any): TimeoutID)); - timeoutID = setTimeout(handleResize, 100); - }; + const debounceHandleResize = throttle(handleResize, 100); handleResize(); // It's important to listen to the ownerDocument.defaultView to support the browser extension. @@ -192,9 +189,6 @@ function useElementsBarOverflowing( ownerWindow.addEventListener('resize', debounceHandleResize); return () => { ownerWindow.removeEventListener('resize', debounceHandleResize); - if (timeoutID !== null) { - clearTimeout(timeoutID); - } }; }, [elementsBarRef, isElementsBarOverflowing, callback]); } From 44c0febd234bb7c4c26af0f2643f850395db386f Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sun, 7 Apr 2019 08:36:37 -0700 Subject: [PATCH 06/36] Profiler shows commit times and durations for selected fiber --- src/backend/agent.js | 21 +++++ src/backend/renderer.js | 35 ++++++++ src/backend/types.js | 7 ++ src/devtools/ProfilingCache.js | 71 ++++++++++++++++- .../views/Profiler/CommitFlamegraph.js | 4 +- .../Profiler/CommitFlamegraphListItem.js | 7 +- src/devtools/views/Profiler/CommitRanked.js | 4 +- .../views/Profiler/CommitRankedListItem.js | 2 +- .../views/Profiler/FlamegraphChartBuilder.js | 2 + src/devtools/views/Profiler/Profiler.js | 11 ++- .../views/Profiler/ProfilerContext.js | 12 ++- .../views/Profiler/RankedChartBuilder.js | 2 + .../views/Profiler/SidebarCommitInfo.css | 9 ++- .../views/Profiler/SidebarCommitInfo.js | 3 +- .../Profiler/SidebarSelectedFiberInfo.css | 52 ++++++++++++ .../Profiler/SidebarSelectedFiberInfo.js | 79 +++++++++++++++++++ src/devtools/views/Profiler/types.js | 6 ++ 17 files changed, 308 insertions(+), 19 deletions(-) create mode 100644 src/devtools/views/Profiler/SidebarSelectedFiberInfo.css create mode 100644 src/devtools/views/Profiler/SidebarSelectedFiberInfo.js diff --git a/src/backend/agent.js b/src/backend/agent.js index ca711c4194..3bde89b953 100644 --- a/src/backend/agent.js +++ b/src/backend/agent.js @@ -65,6 +65,7 @@ export default class Agent extends EventEmitter { bridge.addListener('captureScreenshot', this.captureScreenshot); bridge.addListener('exportProfilingSummary', this.exportProfilingSummary); bridge.addListener('getCommitDetails', this.getCommitDetails); + bridge.addListener('getFiberCommits', this.getFiberCommits); bridge.addListener('getInteractions', this.getInteractions); bridge.addListener('getProfilingStatus', this.getProfilingStatus); bridge.addListener('getProfilingSummary', this.getProfilingSummary); @@ -158,6 +159,26 @@ export default class Agent extends EventEmitter { } }; + getFiberCommits = ({ + fiberID, + rendererID, + rootID, + }: { + fiberID: number, + rendererID: number, + rootID: number, + }) => { + const renderer = this._rendererInterfaces[rendererID]; + if (renderer == null) { + console.warn(`Invalid renderer id "${rendererID}"`); + } else { + this._bridge.send( + 'fiberCommits', + renderer.getFiberCommits(rootID, fiberID) + ); + } + }; + getInteractions = ({ rendererID, rootID, diff --git a/src/backend/renderer.js b/src/backend/renderer.js index ebc5b0684a..3a4cb9f1c9 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -32,6 +32,7 @@ import type { CommitDetails, DevToolsHook, Fiber, + FiberCommits, FiberData, Interaction, Interactions, @@ -1652,6 +1653,39 @@ export function attach( }; } + function getFiberCommits(rootID: number, fiberID: number): FiberCommits { + const commitProfilingMetadata = ((rootToCommitProfilingMetadataMap: any): CommitProfilingMetadataMap).get( + rootID + ); + if (commitProfilingMetadata != null) { + const commitDurations = []; + commitProfilingMetadata.forEach(({ actualDurations }, commitIndex) => { + for (let i = 0; i < actualDurations.length; i += 2) { + if (actualDurations[i] === fiberID) { + commitDurations.push(commitIndex, actualDurations[i + 1]); + break; + } + } + }); + + return { + commitDurations, + fiberID, + rootID, + }; + } + + console.warn( + `getFiberCommits(): No profiling info recorded for root "${rootID}"` + ); + + return { + commitDurations: [], + fiberID, + rootID, + }; + } + function getInteractions(rootID: number): Interactions { const commitProfilingMetadata = ((rootToCommitProfilingMetadataMap: any): CommitProfilingMetadataMap).get( rootID @@ -1817,6 +1851,7 @@ export function attach( flushInitialOperations, getCommitDetails, getFiberIDFromNative, + getFiberCommits, getInteractions, findNativeByFiberID, getProfilingDataForDownload, diff --git a/src/backend/types.js b/src/backend/types.js index 0ee472a015..620119d790 100644 --- a/src/backend/types.js +++ b/src/backend/types.js @@ -66,6 +66,12 @@ export type CommitDetails = {| rootID: number, |}; +export type FiberCommits = {| + commitDurations: Array, + fiberID: number, + rootID: number, +|}; + export type InteractionWithCommits = {| ...Interaction, commits: Array, @@ -93,6 +99,7 @@ export type RendererInterface = { component: NativeType, findNearestUnfilteredAncestor?: boolean ) => number | null, + getFiberCommits: (rootID: number, fiberID: number) => FiberCommits, getInteractions: (rootID: number) => Interactions, getProfilingDataForDownload: (rootID: number) => Object, getProfilingSummary: (rootID: number) => ProfilingSummary, diff --git a/src/devtools/ProfilingCache.js b/src/devtools/ProfilingCache.js index 1699f95f8c..e0b4cfabc9 100644 --- a/src/devtools/ProfilingCache.js +++ b/src/devtools/ProfilingCache.js @@ -23,11 +23,13 @@ import type { Resource } from './cache'; import type { Bridge } from '../types'; import type { CommitDetails as CommitDetailsBackend, + FiberCommits as FiberCommitsBackend, Interactions as InteractionsBackend, ProfilingSummary as ProfilingSummaryBackend, } from 'src/backend/types'; import type { CommitDetails as CommitDetailsFrontend, + FiberCommits as FiberCommitsFrontend, Interactions as InteractionsFrontend, InteractionWithCommits, CommitTree as CommitTreeFrontend, @@ -39,13 +41,19 @@ import type { ChartData as RankedChartData } from 'src/devtools/views/Profiler/R type CommitDetailsParams = {| commitIndex: number, - rootID: number, rendererID: number, + rootID: number, +|}; + +type FiberCommitsParams = {| + fiberID: number, + rendererID: number, + rootID: number, |}; type InteractionsParams = {| - rootID: number, rendererID: number, + rootID: number, |}; type GetCommitTreeParams = {| @@ -54,8 +62,8 @@ type GetCommitTreeParams = {| |}; type ProfilingSummaryParams = {| - rootID: number, rendererID: number, + rootID: number, |}; export default class ProfilingCache { @@ -67,6 +75,11 @@ export default class ProfilingCache { (commitDetails: CommitDetailsFrontend) => void > = new Map(); + _pendingFiberCommitsMap: Map< + string, + (fiberCommits: FiberCommitsFrontend) => void + > = new Map(); + _pendingInteractionsMap: Map< number, (interactions: InteractionsFrontend) => void @@ -121,6 +134,38 @@ export default class ProfilingCache { `${rootID}-${commitIndex}` ); + FiberCommits: Resource< + FiberCommitsParams, + FiberCommitsFrontend + > = createResource( + ({ fiberID, rendererID, rootID }: FiberCommitsParams) => { + return new Promise(resolve => { + const importedProfilingData = this._store.importedProfilingData; + if (importedProfilingData !== null) { + // TODO (profiling) commit details + // Copy from renderer getFiberCommits() + } else if (this._store.profilingOperations.has(rootID)) { + this._pendingFiberCommitsMap.set(`${rootID}-${fiberID}`, resolve); + this._bridge.send('getFiberCommits', { + fiberID, + rendererID, + rootID, + }); + return; + } + + // If no profiling data was recorded for this root, skip the round trip. + resolve({ + commitDurations: [], + fiberID, + rootID, + }); + }); + }, + ({ fiberID, rendererID, rootID }: FiberCommitsParams) => + `${rootID}-${fiberID}` + ); + Interactions: Resource< InteractionsParams, InteractionsFrontend @@ -192,6 +237,7 @@ export default class ProfilingCache { this._store = store; bridge.addListener('commitDetails', this.onCommitDetails); + bridge.addListener('fiberCommits', this.onFiberCommits); bridge.addListener('interactions', this.onInteractions); bridge.addListener('profilingSummary', this.onProfileSummary); } @@ -285,6 +331,24 @@ export default class ProfilingCache { } }; + onFiberCommits = ({ + commitDurations, + fiberID, + rootID, + }: FiberCommitsBackend) => { + const key = `${rootID}-${fiberID}`; + const resolve = this._pendingFiberCommitsMap.get(key); + if (resolve != null) { + this._pendingFiberCommitsMap.delete(key); + + resolve({ + commitDurations, + fiberID, + rootID, + }); + } + }; + onInteractions = ({ interactions, rootID }: InteractionsBackend) => { const resolve = this._pendingInteractionsMap.get(rootID); if (resolve != null) { @@ -304,6 +368,7 @@ export default class ProfilingCache { const resolve = this._pendingProfileSummaryMap.get(rootID); if (resolve != null) { this._pendingProfileSummaryMap.delete(rootID); + const initialTreeBaseDurationsMap = new Map(); for (let i = 0; i < initialTreeBaseDurations.length; i += 2) { initialTreeBaseDurationsMap.set( diff --git a/src/devtools/views/Profiler/CommitFlamegraph.js b/src/devtools/views/Profiler/CommitFlamegraph.js index a44bce868b..a4ad534a82 100644 --- a/src/devtools/views/Profiler/CommitFlamegraph.js +++ b/src/devtools/views/Profiler/CommitFlamegraph.js @@ -20,7 +20,7 @@ export type ItemData = {| scaleX: (value: number, fallbackValue: number) => number, selectedChartNode: ChartNode, selectedChartNodeIndex: number, - selectFiber: (id: number | null) => void, + selectFiber: (id: number | null, name: string | null) => void, width: number, |}; @@ -33,7 +33,7 @@ export default function CommitFlamegraphAutoSizer(_: {||}) { const deselectCurrentFiber = useCallback( event => { event.stopPropagation(); - selectFiber(null); + selectFiber(null, null); }, [selectFiber] ); diff --git a/src/devtools/views/Profiler/CommitFlamegraphListItem.js b/src/devtools/views/Profiler/CommitFlamegraphListItem.js index fb655e25f5..8d55d781b0 100644 --- a/src/devtools/views/Profiler/CommitFlamegraphListItem.js +++ b/src/devtools/views/Profiler/CommitFlamegraphListItem.js @@ -26,9 +26,9 @@ function CommitFlamegraphListItem({ data, index, style }: Props) { const { maxSelfDuration, rows } = chartData; const handleClick = useCallback( - (event: MouseEvent, id: number) => { + (event: MouseEvent, id: number, name: string) => { event.stopPropagation(); - selectFiber(id); + selectFiber(id, name); }, [selectFiber] ); @@ -50,6 +50,7 @@ function CommitFlamegraphListItem({ data, index, style }: Props) { didRender, id, label, + name, offset, selfDuration, treeBaseDuration, @@ -84,7 +85,7 @@ function CommitFlamegraphListItem({ data, index, style }: Props) { isDimmed={index < selectedChartNodeIndex} key={id} label={label} - onClick={event => handleClick(event, id)} + onClick={event => handleClick(event, id, name)} width={nodeWidth} x={nodeOffset - selectedNodeOffset} y={top} diff --git a/src/devtools/views/Profiler/CommitRanked.js b/src/devtools/views/Profiler/CommitRanked.js index 238a7cbe3d..63cb41cd71 100644 --- a/src/devtools/views/Profiler/CommitRanked.js +++ b/src/devtools/views/Profiler/CommitRanked.js @@ -20,7 +20,7 @@ export type ItemData = {| scaleX: (value: number, fallbackValue: number) => number, selectedFiberID: number | null, selectedFiberIndex: number, - selectFiber: (id: number | null) => void, + selectFiber: (id: number | null, name: string | null) => void, width: number, |}; @@ -33,7 +33,7 @@ export default function CommitRankedAutoSizer(_: {||}) { const deselectCurrentFiber = useCallback( event => { event.stopPropagation(); - selectFiber(null); + selectFiber(null, null); }, [selectFiber] ); diff --git a/src/devtools/views/Profiler/CommitRankedListItem.js b/src/devtools/views/Profiler/CommitRankedListItem.js index 6c37a6db46..996a207469 100644 --- a/src/devtools/views/Profiler/CommitRankedListItem.js +++ b/src/devtools/views/Profiler/CommitRankedListItem.js @@ -22,7 +22,7 @@ function CommitRankedListItem({ data, index, style }: Props) { const handleClick = useCallback( event => { event.stopPropagation(); - selectFiber(node.id); + selectFiber(node.id, node.name); }, [node, selectFiber] ); diff --git a/src/devtools/views/Profiler/FlamegraphChartBuilder.js b/src/devtools/views/Profiler/FlamegraphChartBuilder.js index 9478c42d7e..bfb912adcc 100644 --- a/src/devtools/views/Profiler/FlamegraphChartBuilder.js +++ b/src/devtools/views/Profiler/FlamegraphChartBuilder.js @@ -9,6 +9,7 @@ export type ChartNode = {| didRender: boolean, id: number, label: string, + name: string, offset: number, selfDuration: number, treeBaseDuration: number, @@ -82,6 +83,7 @@ export function getChartData({ didRender, id, label, + name, offset: parentOffset, selfDuration, treeBaseDuration: node.treeBaseDuration, diff --git a/src/devtools/views/Profiler/Profiler.js b/src/devtools/views/Profiler/Profiler.js index 49145e47bc..4b346dd413 100644 --- a/src/devtools/views/Profiler/Profiler.js +++ b/src/devtools/views/Profiler/Profiler.js @@ -19,6 +19,7 @@ import ProfilingImportExportButtons from './ProfilingImportExportButtons'; import SnapshotSelector from './SnapshotSelector'; import SidebarCommitInfo from './SidebarCommitInfo'; import SidebarInteractions from './SidebarInteractions'; +import SidebarSelectedFiberInfo from './SidebarSelectedFiberInfo'; import ToggleCommitFilterModalButton from './ToggleCommitFilterModalButton'; import styles from './Profiler.css'; @@ -118,7 +119,9 @@ function SnapshotSelectorFallback() { // This view's subtree uses suspense to request profiler data from the backend. // NOTE that the structure of this UI should mirror NonSuspendingProfiler. function SuspendingProfiler() { - const { selectedTabID, selectTab } = useContext(ProfilerContext); + const { selectedFiberID, selectedTabID, selectTab } = useContext( + ProfilerContext + ); const { isFilterModalShowing, setIsFilterModalShowing } = useContext( CommitFilterModalContext ); @@ -147,7 +150,11 @@ function SuspendingProfiler() { break; case 'flame-chart': case 'ranked-chart': - sidebar = ; + if (selectedFiberID !== null) { + sidebar = ; + } else { + sidebar = ; + } break; default: break; diff --git a/src/devtools/views/Profiler/ProfilerContext.js b/src/devtools/views/Profiler/ProfilerContext.js index bd5eb831d6..d06b5dab73 100644 --- a/src/devtools/views/Profiler/ProfilerContext.js +++ b/src/devtools/views/Profiler/ProfilerContext.js @@ -56,7 +56,8 @@ type Context = {| // Which fiber is currently selected in the Ranked or Flamegraph charts? selectedFiberID: number | null, - selectFiber: (id: number | null) => void, + selectedFiberName: string | null, + selectFiber: (id: number | null, name: string | null) => void, // Which interaction is currently selected in the Interactions graph? selectedInteractionID: number | null, @@ -141,13 +142,15 @@ function ProfilerContextController({ children }: Props) { ); const [selectedTabID, selectTab] = useState('flame-chart'); const [selectedFiberID, selectFiberID] = useState(null); + const [selectedFiberName, selectFiberName] = useState(null); const [selectedInteractionID, selectInteraction] = useState( null ); const selectFiber = useCallback( - (id: number | null) => { + (id: number | null, name: string | null) => { selectFiberID(id); + selectFiberName(name); if (id !== null) { const index = store.getIndexOfElementID(id); if (index !== null) { @@ -155,7 +158,7 @@ function ProfilerContextController({ children }: Props) { } } }, - [selectElementAtIndex, selectFiberID, store] + [selectElementAtIndex, selectFiberID, selectFiberName, store] ); if (isProfiling) { @@ -165,6 +168,7 @@ function ProfilerContextController({ children }: Props) { } if (selectedFiberID !== null) { selectFiberID(null); + selectFiberName(null); } if (selectedInteractionID !== null) { selectInteraction(null); @@ -195,6 +199,7 @@ function ProfilerContextController({ children }: Props) { selectCommitIndex, selectedFiberID, + selectedFiberName, selectFiber, selectedInteractionID, @@ -222,6 +227,7 @@ function ProfilerContextController({ children }: Props) { selectCommitIndex, selectedFiberID, + selectedFiberName, selectFiber, selectedInteractionID, diff --git a/src/devtools/views/Profiler/RankedChartBuilder.js b/src/devtools/views/Profiler/RankedChartBuilder.js index 58235ad484..981692c90c 100644 --- a/src/devtools/views/Profiler/RankedChartBuilder.js +++ b/src/devtools/views/Profiler/RankedChartBuilder.js @@ -7,6 +7,7 @@ import type { CommitDetails, CommitTree, Node } from './types'; export type ChartNode = {| id: number, label: string, + name: string, value: number, |}; @@ -58,6 +59,7 @@ export function getChartData({ chartNodes.push({ id, label, + name, value: selfDuration, }); }); diff --git a/src/devtools/views/Profiler/SidebarCommitInfo.css b/src/devtools/views/Profiler/SidebarCommitInfo.css index 6e99ac4ff3..cde971ef67 100644 --- a/src/devtools/views/Profiler/SidebarCommitInfo.css +++ b/src/devtools/views/Profiler/SidebarCommitInfo.css @@ -20,20 +20,25 @@ } .InteractionList { - padding: 0.25rem; + margin: 0 0 0.5rem; } .ListItem { margin: 0 0 0.5rem; } -.InteractionListItem { +.InteractionListItem, +.InteractionListItemNone { padding: 0.25rem 0.5rem; } .InteractionListItem:hover { background-color: var(--color-hover-background); } +.InteractionListItemNone { + color: var(--color-dim); +} + .Label { overflow: hidden; text-overflow: ellipsis; diff --git a/src/devtools/views/Profiler/SidebarCommitInfo.js b/src/devtools/views/Profiler/SidebarCommitInfo.js index a786d6e8db..085ce46195 100644 --- a/src/devtools/views/Profiler/SidebarCommitInfo.js +++ b/src/devtools/views/Profiler/SidebarCommitInfo.js @@ -88,7 +88,7 @@ export default function SidebarCommitInfo(_: Props) { :
      {interactions.length === 0 ? ( -
    • None
      • +
    • None
      • ) : null} {interactions.map((interaction, index) => (
    • {captureScreenshots && (
    • + : {screenshot != null ? ( Screenshot'; + color: var(--color-jsx-arrow-brackets); +} + +.Label { + font-weight: bold; + margin-bottom: 0.5rem; +} + +.List { + list-style: none; + margin: 0; + padding: 0; +} + +.CurrentListItem, +.ListItem { + padding: 0.25rem 0.5rem; +} +.ListItem:hover { + background-color: var(--color-hover-background); +} + +.CurrentListItem { + background-color: var(--color-selected-background); + color: var(--color-selected-foreground); +} diff --git a/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js b/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js new file mode 100644 index 0000000000..cf8d1c84b7 --- /dev/null +++ b/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js @@ -0,0 +1,79 @@ +// @flow + +import React, { Fragment, useContext } from 'react'; +import { ProfilerContext } from './ProfilerContext'; +import { formatDuration, formatTime } from './utils'; +import { StoreContext } from '../context'; +import Button from '../Button'; +import ButtonIcon from '../ButtonIcon'; + +import styles from './SidebarSelectedFiberInfo.css'; + +export type Props = {||}; + +export default function SidebarSelectedFiberInfo(_: Props) { + const { profilingCache } = useContext(StoreContext); + const { + rendererID, + rootID, + selectCommitIndex, + selectedCommitIndex, + selectedFiberID, + selectedFiberName, + selectFiber, + } = useContext(ProfilerContext); + + const { commitTimes } = profilingCache.ProfilingSummary.read({ + rendererID: ((rendererID: any): number), + rootID: ((rootID: any): number), + }); + + const { commitDurations } = profilingCache.FiberCommits.read({ + fiberID: ((selectedFiberID: any): number), + rendererID: ((rendererID: any): number), + rootID: ((rootID: any): number), + }); + + const listItems = []; + for (let i = 0; i < commitDurations.length; i += 2) { + const commitIndex = commitDurations[i]; + const duration = commitDurations[i + 1]; + const time = commitTimes[commitIndex]; + + listItems.push( +
    • selectCommitIndex(commitIndex)} + > + {formatTime(time)}s for {formatDuration(duration)}ms +
      • + ); + } + + return ( + +
      +
      + {selectedFiberName || 'Selected component'} +
      + + +
      +
      + : +
        {listItems}
      +
      +       + ); +} diff --git a/src/devtools/views/Profiler/types.js b/src/devtools/views/Profiler/types.js index fc7d22e770..eb9dd1b494 100644 --- a/src/devtools/views/Profiler/types.js +++ b/src/devtools/views/Profiler/types.js @@ -34,6 +34,12 @@ export type CommitDetails = {| interactions: Array, |}; +export type FiberCommits = {| + commitDurations: Array, + fiberID: number, + rootID: number, +|}; + export type ProfilingSummary = {| rootID: number, From 2b868821d5ade63da9da4befd967377770c6b6fc Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sun, 7 Apr 2019 09:00:52 -0700 Subject: [PATCH 07/36] Improved keyboard navigation for tabs, interactions, and fiber-commits --- .../Profiler/ProfilingImportExportButtons.js | 1 + .../views/Profiler/SidebarCommitInfo.css | 33 +++++++++++-------- .../views/Profiler/SidebarCommitInfo.js | 14 ++++---- .../Profiler/SidebarSelectedFiberInfo.css | 25 ++++++++------ .../Profiler/SidebarSelectedFiberInfo.js | 11 +++---- .../views/Profiler/SnapshotSelector.css | 1 + src/devtools/views/TabBar.css | 4 +++ 7 files changed, 52 insertions(+), 37 deletions(-) diff --git a/src/devtools/views/Profiler/ProfilingImportExportButtons.js b/src/devtools/views/Profiler/ProfilingImportExportButtons.js index b97c2a317c..a6c0b14b2b 100644 --- a/src/devtools/views/Profiler/ProfilingImportExportButtons.js +++ b/src/devtools/views/Profiler/ProfilingImportExportButtons.js @@ -85,6 +85,7 @@ export default function ProfilingImportExportButtons() { className={styles.Input} type="file" onChange={handleFiles} + tabIndex={-1} /> ))} -
    +
    {captureScreenshots && (
  • diff --git a/src/devtools/views/Profiler/SidebarSelectedFiberInfo.css b/src/devtools/views/Profiler/SidebarSelectedFiberInfo.css index 357c3fd4f5..a3759a1a6d 100644 --- a/src/devtools/views/Profiler/SidebarSelectedFiberInfo.css +++ b/src/devtools/views/Profiler/SidebarSelectedFiberInfo.css @@ -32,21 +32,26 @@ margin-bottom: 0.5rem; } -.List { - list-style: none; - margin: 0; - padding: 0; -} - -.CurrentListItem, -.ListItem { +.CurrentCommit, +.Commit { + display: block; + width: 100%; + text-align: left; + background: none; + border: none; padding: 0.25rem 0.5rem; + color: var(--color-text); } -.ListItem:hover { +.Commit:focus, +.Commit:hover { + outline: none; background-color: var(--color-hover-background); } -.CurrentListItem { +.CurrentCommit { background-color: var(--color-selected-background); color: var(--color-selected-foreground); } +.CurrentCommit:focus { + outline: none; +} diff --git a/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js b/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js index cf8d1c84b7..8739509575 100644 --- a/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js +++ b/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js @@ -41,17 +41,17 @@ export default function SidebarSelectedFiberInfo(_: Props) { const time = commitTimes[commitIndex]; listItems.push( -
  • selectCommitIndex(commitIndex)} > {formatTime(time)}s for {formatDuration(duration)}ms -
    • + ); } @@ -71,8 +71,7 @@ export default function SidebarSelectedFiberInfo(_: Props) {
    - : -
      {listItems}
    + : {listItems}
    ); diff --git a/src/devtools/views/Profiler/SnapshotSelector.css b/src/devtools/views/Profiler/SnapshotSelector.css index 4a19d964bf..e4f8aec57d 100644 --- a/src/devtools/views/Profiler/SnapshotSelector.css +++ b/src/devtools/views/Profiler/SnapshotSelector.css @@ -11,6 +11,7 @@ } .Commits:focus { outline: none; + background-color: var(--color-button-background-focus); } .IndexLabel { diff --git a/src/devtools/views/TabBar.css b/src/devtools/views/TabBar.css index 07362e6bde..e1a536e07f 100644 --- a/src/devtools/views/TabBar.css +++ b/src/devtools/views/TabBar.css @@ -15,6 +15,10 @@ .TabCurrent:hover { background-color: var(--color-hover-background); } +.Tab:focus-within, +.TabCurrent:focus-within { + background-color: var(--color-hover-background); +} .TabCurrent { border-bottom: 3px solid var(--color-selected-border); From 9a2f9ac880ef47e0195afa20a1f23993ce35c425 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sun, 7 Apr 2019 09:42:52 -0700 Subject: [PATCH 08/36] Moved getElementDimensions() back into Overlay component --- src/backend/views/Overlay.js | 19 ++++++++++++++++++- src/devtools/views/Components/OwnersStack.js | 8 ++------ src/utils.js | 18 ------------------ 3 files changed, 20 insertions(+), 25 deletions(-) diff --git a/src/backend/views/Overlay.js b/src/backend/views/Overlay.js index a54547e525..90e7446461 100644 --- a/src/backend/views/Overlay.js +++ b/src/backend/views/Overlay.js @@ -1,7 +1,6 @@ // @flow import assign from 'object-assign'; -import { getElementDimensions } from '../../utils'; type Rect = { bottom: number, @@ -216,6 +215,24 @@ function findTipPos(dims, win) { return { top, left: dims.left + margin + 'px' }; } +export function getElementDimensions(domElement: Element) { + const calculatedStyle = window.getComputedStyle(domElement); + return { + borderLeft: +calculatedStyle.borderLeftWidth.match(/[0-9]*/)[0], + borderRight: +calculatedStyle.borderRightWidth.match(/[0-9]*/)[0], + borderTop: +calculatedStyle.borderTopWidth.match(/[0-9]*/)[0], + borderBottom: +calculatedStyle.borderBottomWidth.match(/[0-9]*/)[0], + marginLeft: +calculatedStyle.marginLeft.match(/[0-9]*/)[0], + marginRight: +calculatedStyle.marginRight.match(/[0-9]*/)[0], + marginTop: +calculatedStyle.marginTop.match(/[0-9]*/)[0], + marginBottom: +calculatedStyle.marginBottom.match(/[0-9]*/)[0], + paddingLeft: +calculatedStyle.paddingLeft.match(/[0-9]*/)[0], + paddingRight: +calculatedStyle.paddingRight.match(/[0-9]*/)[0], + paddingTop: +calculatedStyle.paddingTop.match(/[0-9]*/)[0], + paddingBottom: +calculatedStyle.paddingBottom.match(/[0-9]*/)[0], + }; +} + // Get the window object for the document that a node belongs to, // or return null if it cannot be found (node not attached to DOM, // etc). diff --git a/src/devtools/views/Components/OwnersStack.js b/src/devtools/views/Components/OwnersStack.js index 295c3fd97d..2fabc1e7fb 100644 --- a/src/devtools/views/Components/OwnersStack.js +++ b/src/devtools/views/Components/OwnersStack.js @@ -13,7 +13,6 @@ import Button from '../Button'; import ButtonIcon from '../ButtonIcon'; import { TreeContext } from './TreeContext'; import { StoreContext } from '../context'; -import { getElementDimensions } from '../../../utils'; import type { Element } from './types'; @@ -58,10 +57,7 @@ type ElementsBarProps = { showSelectedOnly: boolean, }; const ElementsBar = forwardRef( - ( - { elements, showSelectedOnly }: ElementsBarProps, - ref: Object - ) => { + ({ elements, showSelectedOnly }: ElementsBarProps, ref: Object) => { return (
    { const { offsetWidth } = el; - const { marginRight } = getElementDimensions(el); + const marginRight = parseInt(getComputedStyle(el).marginRight, 10); return acc + (offsetWidth + marginRight); }, 0); diff --git a/src/utils.js b/src/utils.js index aa79351fc6..9c2c9189b9 100644 --- a/src/utils.js +++ b/src/utils.js @@ -63,21 +63,3 @@ export function utfEncodeString(string: string): Uint32Array { function toCodePoint(string: string) { return string.codePointAt(0); } - -export function getElementDimensions(domElement: Element) { - const calculatedStyle = window.getComputedStyle(domElement); - return { - borderLeft: +calculatedStyle.borderLeftWidth.match(/[0-9]*/)[0], - borderRight: +calculatedStyle.borderRightWidth.match(/[0-9]*/)[0], - borderTop: +calculatedStyle.borderTopWidth.match(/[0-9]*/)[0], - borderBottom: +calculatedStyle.borderBottomWidth.match(/[0-9]*/)[0], - marginLeft: +calculatedStyle.marginLeft.match(/[0-9]*/)[0], - marginRight: +calculatedStyle.marginRight.match(/[0-9]*/)[0], - marginTop: +calculatedStyle.marginTop.match(/[0-9]*/)[0], - marginBottom: +calculatedStyle.marginBottom.match(/[0-9]*/)[0], - paddingLeft: +calculatedStyle.paddingLeft.match(/[0-9]*/)[0], - paddingRight: +calculatedStyle.paddingRight.match(/[0-9]*/)[0], - paddingTop: +calculatedStyle.paddingTop.match(/[0-9]*/)[0], - paddingBottom: +calculatedStyle.paddingBottom.match(/[0-9]*/)[0], - }; -} From f2951fb51fa338da7a651cf414a074f2acf8b4cb Mon Sep 17 00:00:00 2001 From: Dan Date: Sun, 7 Apr 2019 17:40:53 +0100 Subject: [PATCH 09/36] Preserve selection on exiting owner mode --- src/devtools/views/Components/TreeContext.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/devtools/views/Components/TreeContext.js b/src/devtools/views/Components/TreeContext.js index 940821c1a1..b83078f653 100644 --- a/src/devtools/views/Components/TreeContext.js +++ b/src/devtools/views/Components/TreeContext.js @@ -383,7 +383,10 @@ function reduceOwnersState(store: Store, state: State, action: Action): State { case 'RESET_OWNER_STACK': ownerStack = []; ownerStackIndex = null; - selectedElementIndex = null; + selectedElementIndex = + selectedElementID !== null + ? store.getIndexOfElementID(selectedElementID) + : null; _ownerFlatTree = null; break; case 'SELECT_ELEMENT_AT_INDEX': From 9255ad1025d219842e1b9ac2a1a2734533b357ce Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sun, 7 Apr 2019 09:46:44 -0700 Subject: [PATCH 10/36] Replaced "colon" icon with more standard 3-dot "more" --- src/devtools/views/ButtonIcon.js | 13 ++----------- src/devtools/views/Components/OwnersStack.js | 2 +- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/devtools/views/ButtonIcon.js b/src/devtools/views/ButtonIcon.js index 7558ddfec8..82a48d6415 100644 --- a/src/devtools/views/ButtonIcon.js +++ b/src/devtools/views/ButtonIcon.js @@ -7,7 +7,6 @@ export type IconType = | 'back' | 'cancel' | 'close' - | 'colon' | 'copy' | 'down' | 'export' @@ -40,9 +39,6 @@ export default function ButtonIcon({ type }: Props) { case 'close': pathData = PATH_CLOSE; break; - case 'colon': - pathData = PATH_COLON; - break; case 'copy': pathData = PATH_COPY; break; @@ -121,9 +117,6 @@ const PATH_CANCEL = ` const PATH_CLOSE = 'M19 6.41L17.59 5 12 10.59 6.41 5 5 6.41 10.59 12 5 17.59 6.41 19 12 13.41 17.59 19 19 17.59 13.41 12z'; -const PATH_COLON = - 'M10,9a2,2 0 1,0 4,0a2,2 0 1,0 -4,0 M10,19a2,2 0 1,0 4,0a2,2 0 1,0 -4,0'; - const PATH_COPY = ` M3 13h2v-2H3v2zm0 4h2v-2H3v2zm2 4v-2H3a2 2 0 0 0 2 2zM3 9h2V7H3v2zm12 12h2v-2h-2v2zm4-18H9a2 2 0 0 0-2 2v10a2 2 0 0 0 2 2h10c1.1 0 2-.9 2-2V5c0-1.1-.9-2-2-2zm0 12H9V5h10v10zm-8 6h2v-2h-2v2zm-4 0h2v-2H7v2z @@ -138,10 +131,8 @@ const PATH_FILTER = 'M10 18h4v-2h-4v2zM3 6v2h18V6H3zm3 7h12v-2H6v2z'; const PATH_IMPORT = 'M8.18,18.13v-7H3l9-8.95,9,9H15.82v7ZM3,20.13H21v1.73H3Z'; const PATH_MORE = ` - M22 3H7c-.69 0-1.23.35-1.59.88L0 12l5.41 8.11c.36.53.97.89 1.66.89H22c1.1 0 2-.9 2-2V5c0-1.1-.9-2-2-2zM9 - 13.5c-.83 0-1.5-.67-1.5-1.5s.67-1.5 1.5-1.5 1.5.67 1.5 1.5-.67 1.5-1.5 1.5zm5 0c-.83 - 0-1.5-.67-1.5-1.5s.67-1.5 1.5-1.5 1.5.67 1.5 1.5-.67 1.5-1.5 1.5zm5 0c-.83 0-1.5-.67-1.5-1.5s.67-1.5 1.5-1.5 - 1.5.67 1.5 1.5-.67 1.5-1.5 1.5z + M12 8c1.1 0 2-.9 2-2s-.9-2-2-2-2 .9-2 2 .9 2 2 2zm0 2c-1.1 0-2 .9-2 2s.9 + 2 2 2 2-.9 2-2-.9-2-2-2zm0 6c-1.1 0-2 .9-2 2s.9 2 2 2 2-.9 2-2-.9-2-2-2z `; const PATH_NEXT = 'M12 4l-1.41 1.41L16.17 11H4v2h12.17l-5.58 5.59L12 20l8-8z'; diff --git a/src/devtools/views/Components/OwnersStack.js b/src/devtools/views/Components/OwnersStack.js index 2fabc1e7fb..f2de0c47a9 100644 --- a/src/devtools/views/Components/OwnersStack.js +++ b/src/devtools/views/Components/OwnersStack.js @@ -45,7 +45,7 @@ function ElementsDropdown({ onClick={handleClick} title="Open elements dropdown" > - + {isDropdownVisible &&
    {children}
    }
    From 252a87efe58771bde9bc85d7156f7015f9f484ef Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sun, 7 Apr 2019 09:51:54 -0700 Subject: [PATCH 11/36] Use 'classnames' in TabBar to simplify an attribute --- src/devtools/views/TabBar.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/devtools/views/TabBar.js b/src/devtools/views/TabBar.js index 81611fb7a2..d5ba8c5cb6 100644 --- a/src/devtools/views/TabBar.js +++ b/src/devtools/views/TabBar.js @@ -1,5 +1,6 @@ // @flow +import classNames from 'classnames'; import React, { Fragment, useCallback } from 'react'; import Icon from './Icon'; @@ -60,9 +61,11 @@ export default function TabBar({ {tabs.map(({ icon, id, label, title }) => (