diff --git a/package.json b/package.json index 970f1f99d5..aa58a10370 100644 --- a/package.json +++ b/package.json @@ -104,7 +104,7 @@ "react-dom": "^16.8.4", "react-is": "^16.8.4", "react-virtualized-auto-sizer": "^1.0.2", - "react-window": "^1.5.1", + "react-window": "^1.7.2", "scheduler": "^0.13", "semver": "^5.5.1", "style-loader": "^0.23.1", diff --git a/src/devtools/store.js b/src/devtools/store.js index 225f9a8ac8..d81b6e89c1 100644 --- a/src/devtools/store.js +++ b/src/devtools/store.js @@ -71,10 +71,6 @@ export default class Store extends EventEmitter { // When profiling is in progress, operations are stored so that we can later reconstruct past commit trees. _isProfiling: boolean = false; - // Total number of visible elements (within all roots). - // Used for windowing purposes. - _numElements: number = 0; - // Suspense cache for reading profilign data. _profilingCache: ProfilingCache; @@ -112,6 +108,10 @@ export default class Store extends EventEmitter { _supportsProfiling: boolean = false; _supportsReloadAndProfile: boolean = false; + // Total number of visible elements (within all roots). + // Used for windowing purposes. + _weightAcrossRoots: number = 0; + constructor(bridge: Bridge, config?: Config) { super(); @@ -198,7 +198,7 @@ export default class Store extends EventEmitter { } get numElements(): number { - return this._numElements; + return this._weightAcrossRoots; } get profilingCache(): ProfilingCache { @@ -287,16 +287,18 @@ export default class Store extends EventEmitter { let currentElement = ((this._idToElement.get(firstChildID): any): Element); let currentWeight = rootWeight; while (index !== currentWeight) { - for (let i = 0; i < currentElement.children.length; i++) { + const numChildren = currentElement.children.length; + for (let i = 0; i < numChildren; i++) { const childID = currentElement.children[i]; const child = ((this._idToElement.get(childID): any): Element); - const { weight } = child; - if (index <= currentWeight + weight) { + const childWeight = child.isCollapsed ? 1 : child.weight; + + if (index <= currentWeight + childWeight) { currentWeight++; currentElement = child; break; } else { - currentWeight += weight; + currentWeight += childWeight; } } } @@ -351,7 +353,7 @@ export default class Store extends EventEmitter { break; } const child = ((this._idToElement.get(childID): any): Element); - index += child.weight; + index += child.isCollapsed ? 1 : child.weight; } previousID = current.id; @@ -420,6 +422,29 @@ export default class Store extends EventEmitter { this.emit('isProfiling'); } + toggleIsCollapsed(id: number, isCollapsed: boolean): void { + const element = this.getElementByID(id); + if (element !== null) { + const oldWeight = element.isCollapsed ? 1 : element.weight; + element.isCollapsed = isCollapsed; + const newWeight = element.isCollapsed ? 1 : element.weight; + const weightDelta = newWeight - oldWeight; + + this._weightAcrossRoots += weightDelta; + + let parentElement = this._idToElement.get(element.parentID); + while (parentElement != null) { + parentElement.weight += weightDelta; + parentElement = this._idToElement.get(parentElement.parentID); + } + + // The Tree context's search reducer expects an explicit list of ids for nodes that were added or removed. + // In this case, we can pass it empty arrays since nodes in a collapsed tree are still there (just hidden). + // Updating the selected search index later may require auto-expanding a collapsed subtree though. + this.emit('mutated', [[], []]); + } + } + _captureScreenshot = throttle( memoize((commitIndex: number) => { this._bridge.send('captureScreenshot', { commitIndex }); @@ -518,6 +543,7 @@ export default class Store extends EventEmitter { depth: -1, displayName: null, id, + isCollapsed: false, key: null, ownerID: 0, parentID: 0, @@ -566,6 +592,7 @@ export default class Store extends EventEmitter { depth: parentElement.depth + 1, displayName, id, + isCollapsed: false, key, ownerID, parentID: parentElement.id, @@ -715,14 +742,27 @@ export default class Store extends EventEmitter { throw Error(`Unsupported Bridge operation ${operation}`); } - this._numElements += weightDelta; + let isInsideCollapsedSubTree = false; while (parentElement != null) { parentElement.weight += weightDelta; + + // Additions and deletions within a collapsed subtree should not bubble beyond the collapsed parent. + // Their weight will bubble up when the parent is expanded. + if (parentElement.isCollapsed) { + isInsideCollapsedSubTree = true; + break; + } + parentElement = ((this._idToElement.get( parentElement.parentID ): any): Element); } + + // Additions and deletions within a collapsed subtree should not affect the overall number of elements. + if (!isInsideCollapsedSubTree) { + this._weightAcrossRoots += weightDelta; + } } this._revision++; diff --git a/src/devtools/views/ButtonIcon.js b/src/devtools/views/ButtonIcon.js index 7aa08b3c6c..c27e63183c 100644 --- a/src/devtools/views/ButtonIcon.js +++ b/src/devtools/views/ButtonIcon.js @@ -7,8 +7,10 @@ export type IconType = | 'back' | 'cancel' | 'close' + | 'collapsed' | 'copy' | 'down' + | 'expanded' | 'export' | 'filter' | 'import' @@ -40,12 +42,18 @@ export default function ButtonIcon({ type }: Props) { case 'close': pathData = PATH_CLOSE; break; + case 'collapsed': + pathData = PATH_COLLAPSED; + break; case 'copy': pathData = PATH_COPY; break; case 'down': pathData = PATH_DOWN; break; + case 'expanded': + pathData = PATH_EXPANDED; + break; case 'export': pathData = PATH_EXPORT; break; @@ -121,6 +129,8 @@ 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_COLLAPSED = 'M10 17l5-5-5-5v10z'; + 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 @@ -128,6 +138,8 @@ const PATH_COPY = ` const PATH_DOWN = 'M7.41 8.59L12 13.17l4.59-4.58L18 10l-6 6-6-6 1.41-1.41z'; +const PATH_EXPANDED = 'M7 10l5 5 5-5z'; + const PATH_EXPORT = 'M15.82,2.14v7H21l-9,9L3,9.18H8.18v-7ZM3,20.13H21v1.73H3Z'; const PATH_FILTER = 'M10 18h4v-2h-4v2zM3 6v2h18V6H3zm3 7h12v-2H6v2z'; diff --git a/src/devtools/views/Components/Element.css b/src/devtools/views/Components/Element.css index 4f1748c9c1..72f6d68a2f 100644 --- a/src/devtools/views/Components/Element.css +++ b/src/devtools/views/Components/Element.css @@ -7,6 +7,8 @@ align-items: center; cursor: default; user-select: none; + + --color-expand-collapse-toggle: var(--color-dim); } .Element:hover { background-color: var(--color-hover-background); @@ -21,6 +23,7 @@ --color-jsx-arrow-brackets: var(--color-jsx-arrow-brackets-inverted); --color-attribute-name: var(--color-hover-background); --color-attribute-value: var(--color-component-name-inverted); + --color-expand-collapse-toggle: var(--color-component-name-inverted); } .DollarR { @@ -55,3 +58,10 @@ .CurrentHighlight { background-color: var(--color-search-match-current); } + +.ExpandCollapseToggle { + display: inline-flex; + width: 1rem; + height: 1rem; + color: var(--color-expand-collapse-toggle); +} diff --git a/src/devtools/views/Components/Element.js b/src/devtools/views/Components/Element.js index 01f983c217..f696a1e3cd 100644 --- a/src/devtools/views/Components/Element.js +++ b/src/devtools/views/Components/Element.js @@ -9,6 +9,8 @@ import React, { useRef, } from 'react'; import { ElementTypeClass, ElementTypeFunction } from 'src/devtools/types'; +import Store from 'src/devtools/store'; +import ButtonIcon from '../ButtonIcon'; import { createRegExp } from '../utils'; import { TreeContext } from './TreeContext'; import { BridgeContext, StoreContext } from '../context'; @@ -28,6 +30,7 @@ export default function ElementView({ data, index, style }: Props) { const { baseDepth, getElementAtIndex, + ownerStack, selectOwner, selectedElementID, selectElementByID, @@ -75,8 +78,6 @@ export default function ElementView({ data, index, style }: Props) { } }, [id, isSelected, lastScrolledIDRef]); - // TODO Add click and key handlers for toggling element open/close state. - const handleMouseDown = useCallback( ({ metaKey }) => { if (id !== null) { @@ -114,8 +115,6 @@ export default function ElementView({ data, index, style }: Props) { const showDollarR = isSelected && (type === ElementTypeClass || type === ElementTypeFunction); - // TODO styles.SelectedElement is 100% width but it doesn't take horizontal overflow into account. - return (
+ {ownerStack.length === 0 ? ( + + ) : null} {key && ( @@ -152,6 +154,45 @@ export default function ElementView({ data, index, style }: Props) { ); } +// Prevent double clicks on toggle from drilling into the owner list. +const swallowDoubleClick = event => { + event.preventDefault(); + event.stopPropagation(); +}; + +type ExpandCollapseToggleProps = {| + element: Element, + store: Store, +|}; + +function ExpandCollapseToggle({ element, store }: ExpandCollapseToggleProps) { + const { children, id, isCollapsed } = element; + + const toggleCollapsed = useCallback( + event => { + event.preventDefault(); + event.stopPropagation(); + + store.toggleIsCollapsed(id, !isCollapsed); + }, + [id, isCollapsed, store] + ); + + if (children.length === 0) { + return
; + } + + return ( +
+ +
+ ); +} + type DisplayNameProps = {| displayName: string | null, id: number, diff --git a/src/devtools/views/Components/Tree.js b/src/devtools/views/Components/Tree.js index b97ff5a9cf..d5a2b31826 100644 --- a/src/devtools/views/Components/Tree.js +++ b/src/devtools/views/Components/Tree.js @@ -12,7 +12,7 @@ import AutoSizer from 'react-virtualized-auto-sizer'; import { FixedSizeList } from 'react-window'; import { TreeContext } from './TreeContext'; import { SettingsContext } from '../Settings/SettingsContext'; -import { BridgeContext } from '../context'; +import { BridgeContext, StoreContext } from '../context'; import ElementView from './Element'; import InspectHostNodesToggle from './InspectHostNodesToggle'; import OwnersStack from './OwnersStack'; @@ -37,12 +37,14 @@ export default function Tree(props: Props) { getElementAtIndex, numElements, ownerStack, + selectedElementID, selectedElementIndex, selectNextElementInTree, selectParentElementInTree, selectPreviousElementInTree, } = useContext(TreeContext); const bridge = useContext(BridgeContext); + const store = useContext(StoreContext); // $FlowFixMe https://github.com/facebook/flow/issues/7341 const listRef = useRef | null>(null); const treeRef = useRef(null); @@ -77,6 +79,8 @@ export default function Tree(props: Props) { return; } + let element; + // eslint-disable-next-line default-case switch (event.key) { case 'ArrowDown': @@ -84,10 +88,34 @@ export default function Tree(props: Props) { event.preventDefault(); break; case 'ArrowLeft': - selectParentElementInTree(); + element = + selectedElementID !== null + ? store.getElementByID(selectedElementID) + : null; + if ( + element !== null && + element.children.length > 0 && + !element.isCollapsed + ) { + store.toggleIsCollapsed(element.id, true); + } else { + selectParentElementInTree(); + } break; case 'ArrowRight': - selectNextElementInTree(); + element = + selectedElementID !== null + ? store.getElementByID(selectedElementID) + : null; + if ( + element !== null && + element.children.length > 0 && + element.isCollapsed + ) { + store.toggleIsCollapsed(element.id, false); + } else { + selectNextElementInTree(); + } event.preventDefault(); break; case 'ArrowUp': @@ -107,9 +135,11 @@ export default function Tree(props: Props) { ownerDocument.removeEventListener('keydown', handleKeyDown); }; }, [ + selectedElementID, selectNextElementInTree, selectParentElementInTree, selectPreviousElementInTree, + store, ]); // Let react-window know to re-render any time the underlying tree data changes. diff --git a/src/devtools/views/Components/TreeContext.js b/src/devtools/views/Components/TreeContext.js index 8e264f826f..915312d521 100644 --- a/src/devtools/views/Components/TreeContext.js +++ b/src/devtools/views/Components/TreeContext.js @@ -22,8 +22,10 @@ import React, { useCallback, useContext, useEffect, + useLayoutEffect, useMemo, useReducer, + useRef, } from 'react'; import { createRegExp } from '../utils'; import { BridgeContext, StoreContext } from '../context'; @@ -109,6 +111,8 @@ function reduceTreeState(store: Store, state: State, action: Action): State { selectedElementID, } = state; + let lookupIDForIndex = true; + // Base tree should ignore selected element changes when the owner's tree is active. if (ownerStack.length === 0) { switch (type) { @@ -127,6 +131,11 @@ function reduceTreeState(store: Store, state: State, action: Action): State { selectedElementIndex = ((payload: any): number | null); break; case 'SELECT_ELEMENT_BY_ID': + // Skip lookup in this case; it would be redundant. + // It might also cause problems if the specified element was inside of a (not yet expanded) subtree. + lookupIDForIndex = false; + + selectedElementID = payload; selectedElementIndex = payload === null ? null @@ -167,7 +176,7 @@ function reduceTreeState(store: Store, state: State, action: Action): State { } // Keep selected item ID and index in sync. - if (selectedElementIndex !== state.selectedElementIndex) { + if (lookupIDForIndex && selectedElementIndex !== state.selectedElementIndex) { if (selectedElementIndex === null) { selectedElementID = null; } else { @@ -686,6 +695,25 @@ function TreeContextController({ children, viewElementSource }: Props) { return () => bridge.removeListener('selectFiber', handleSelectFiber); }, [bridge, dispatch]); + // If a newly-selected search result or inspection selection is inside of a collapsed subtree, auto expand it. + // This needs to be a layout effect to avoid temporarily flashing an incorrect selection. + const prevSelectedElementID = useRef(null); + useLayoutEffect(() => { + if (state.selectedElementID !== prevSelectedElementID.current) { + prevSelectedElementID.current = state.selectedElementID; + + if (state.selectedElementID !== null) { + let element = store.getElementByID(state.selectedElementID); + while (element !== null && element.parentID > 0) { + element = ((store.getElementByID(element.parentID): any): Element); + if (element.isCollapsed) { + store.toggleIsCollapsed(element.id, false); + } + } + } + } + }, [state.selectedElementID, store]); + // Mutations to the underlying tree may impact this context (e.g. search results, selection state). useEffect(() => { const handleStoreMutated = ([ diff --git a/src/devtools/views/Components/types.js b/src/devtools/views/Components/types.js index fc71d5e9bb..ca3c7e3df0 100644 --- a/src/devtools/views/Components/types.js +++ b/src/devtools/views/Components/types.js @@ -14,6 +14,9 @@ export type Element = {| displayName: string | null, key: number | string | null, + // Should the elements children be visible in the tree? + isCollapsed: boolean, + // Owner (if available) ownerID: number, diff --git a/src/devtools/views/Profiler/ProfilerContext.js b/src/devtools/views/Profiler/ProfilerContext.js index d06b5dab73..c8a89c0d9e 100644 --- a/src/devtools/views/Profiler/ProfilerContext.js +++ b/src/devtools/views/Profiler/ProfilerContext.js @@ -79,7 +79,7 @@ type Props = {| function ProfilerContextController({ children }: Props) { const store = useContext(StoreContext); - const { selectElementAtIndex, selectedElementID } = useContext(TreeContext); + const { selectElementByID, selectedElementID } = useContext(TreeContext); const subscription = useMemo( () => ({ @@ -152,13 +152,14 @@ function ProfilerContextController({ children }: Props) { selectFiberID(id); selectFiberName(name); if (id !== null) { - const index = store.getIndexOfElementID(id); - if (index !== null) { - selectElementAtIndex(index); + // If this element is still in the store, then select it in the Components tab as well. + const element = store.getElementByID(id); + if (element !== null) { + selectElementByID(id); } } }, - [selectElementAtIndex, selectFiberID, selectFiberName, store] + [selectElementByID, selectFiberID, selectFiberName, store] ); if (isProfiling) { diff --git a/yarn.lock b/yarn.lock index 25a54310d8..4b0e19920e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7323,6 +7323,11 @@ mem@^4.0.0: mimic-fn "^1.0.0" p-is-promise "^2.0.0" +"memoize-one@>=3.1.1 <6": + version "5.0.4" + resolved "https://registry.yarnpkg.com/memoize-one/-/memoize-one-5.0.4.tgz#005928aced5c43d890a4dfab18ca908b0ec92cbc" + integrity sha512-P0z5IeAH6qHHGkJIXWw0xC2HNEgkx/9uWWBQw64FJj3/ol14VYdfVGWWr0fXfjhhv3TKVIqUq65os6O4GUNksA== + memoize-one@^3.1.1: version "3.1.1" resolved "https://registry.yarnpkg.com/memoize-one/-/memoize-one-3.1.1.tgz#ef609811e3bc28970eac2884eece64d167830d17" @@ -8799,13 +8804,13 @@ react-virtualized-auto-sizer@^1.0.2: resolved "https://registry.yarnpkg.com/react-virtualized-auto-sizer/-/react-virtualized-auto-sizer-1.0.2.tgz#a61dd4f756458bbf63bd895a92379f9b70f803bd" integrity sha512-MYXhTY1BZpdJFjUovvYHVBmkq79szK/k7V3MO+36gJkWGkrXKtyr4vCPtpphaTLRAdDNoYEYFZWE8LjN+PIHNg== -react-window@^1.5.1: - version "1.5.2" - resolved "https://registry.yarnpkg.com/react-window/-/react-window-1.5.2.tgz#39dbfd7aa47c1d80b37928f3269f7112a5900294" - integrity sha512-xGGKS9bR2y/XbkyQBk0qRO3y1mdENXVfksjAIn1fcbZ9qwiML52HryKfCaK50c1bIX3f0xqPgu8Q6FIxHKKbag== +react-window@^1.7.2: + version "1.7.2" + resolved "https://registry.yarnpkg.com/react-window/-/react-window-1.7.2.tgz#2e1528d5b9991e863302bfe74cb52d0ff7082e78" + integrity sha512-GK1gxSeGPLBDSQhPmYhCYrtf5MkGK8rwVjeyPgxZLvLRw0wvyzKZPMc/jfemiGNGfuJyW3kx1z6QR9uK7r2XdA== dependencies: "@babel/runtime" "^7.0.0" - memoize-one "^3.1.1" + memoize-one ">=3.1.1 <6" react@^16.8.4: version "16.8.4"