From 5a269e7ced1011885e3622c2bde8d30fcd8ad3ab Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 31 May 2019 11:24:34 -0700 Subject: [PATCH] Initial POC for compressing horizontal offset for wide/deep trees --- src/devtools/views/Components/Element.css | 25 ++-- src/devtools/views/Components/Element.js | 113 +++--------------- .../{Guidelines.css => Guideline.css} | 0 .../{Guidelines.js => Guideline.js} | 43 ++----- .../views/Components/SelectedElement.css | 6 +- src/devtools/views/Components/Tree.css | 8 ++ src/devtools/views/Components/Tree.js | 54 ++++++--- src/devtools/views/Settings/Settings.js | 26 +--- .../views/Settings/SettingsContext.js | 11 -- src/devtools/views/utils.js | 13 ++ 10 files changed, 102 insertions(+), 197 deletions(-) rename src/devtools/views/Components/{Guidelines.css => Guideline.css} (100%) rename src/devtools/views/Components/{Guidelines.js => Guideline.js} (76%) diff --git a/src/devtools/views/Components/Element.css b/src/devtools/views/Components/Element.css index 5833d462c8..dfa0b27d9d 100644 --- a/src/devtools/views/Components/Element.css +++ b/src/devtools/views/Components/Element.css @@ -9,6 +9,7 @@ align-items: center; cursor: default; user-select: none; + color: var(--color-component-name); } .HoveredElement { background-color: var(--color-background-hover); @@ -17,6 +18,10 @@ background-color: var(--color-background-inactive); } +.Bracket { + color: var(--color-jsx-arrow-brackets); +} + .ScrollAnchor { height: 100%; width: 0; @@ -37,24 +42,6 @@ --color-expand-collapse-toggle: var(--color-component-name-inverted); } -.DollarR { - color: var(--color-jsx-arrow-brackets); -} - -.Component { - color: var(--color-component-name); -} -.Component:before { - white-space: nowrap; - content: '<'; - color: var(--color-jsx-arrow-brackets); -} -.Component:after { - white-space: nowrap; - content: '>'; - color: var(--color-jsx-arrow-brackets); -} - .AttributeName { color: var(--color-attribute-name); } @@ -74,10 +61,12 @@ display: inline-flex; width: 1rem; height: 1rem; + flex: 0 0 1rem; color: var(--color-expand-collapse-toggle); } .Badge { + color: var(--color-text); background-color: var(--color-component-badge-background); padding: 0.125rem 0.25rem; line-height: normal; diff --git a/src/devtools/views/Components/Element.js b/src/devtools/views/Components/Element.js index 681b65a184..d7a03e9667 100644 --- a/src/devtools/views/Components/Element.js +++ b/src/devtools/views/Components/Element.js @@ -4,20 +4,13 @@ import React, { Fragment, useCallback, useContext, - useLayoutEffect, useMemo, - useRef, useState, } from 'react'; -import { - ElementTypeClass, - ElementTypeFunction, - ElementTypeMemo, - ElementTypeForwardRef, -} from 'src/types'; +import { ElementTypeMemo, ElementTypeForwardRef } from 'src/types'; import Store from 'src/devtools/store'; import ButtonIcon from '../ButtonIcon'; -import { createRegExp } from '../utils'; +import { createRegExp, truncateText } from '../utils'; import { TreeDispatcherContext, TreeStateContext } from './TreeContext'; import { StoreContext } from '../context'; @@ -46,13 +39,7 @@ export default function ElementView({ data, index, style }: Props) { const [isHovered, setIsHovered] = useState(false); - const { - lastScrolledIDRef, - isNavigatingWithKeyboard, - onElementMouseEnter, - showIndentLines, - treeFocused, - } = data; + const { isNavigatingWithKeyboard, onElementMouseEnter, treeFocused } = data; const id = element === null ? null : element.id; const isSelected = selectedElementID === id; @@ -62,49 +49,6 @@ export default function ElementView({ data, index, style }: Props) { } }, [dispatch, id]); - const scrollAnchorStartRef = useRef(null); - const scrollAnchorEndRef = useRef(null); - - // The tree above has its own autoscrolling, but it only works for rows. - // However, even when the row gets into the viewport, the component name - // might be too far left or right on the screen. Adjust it in this case. - useLayoutEffect(() => { - if (isSelected) { - // Don't select the same item twice. - // A row may appear and disappear just by scrolling: - // https://github.com/bvaughn/react-devtools-experimental/issues/67 - // It doesn't necessarily indicate a user action. - // TODO: we might want to revamp the autoscroll logic - // to only happen explicitly for user-initiated events. - if (lastScrolledIDRef.current === id) { - return; - } - lastScrolledIDRef.current = id; - - // We want to bring the whole name into view, - // including the expansion toggle and the "=== $r" hint. - // However, even calling scrollIntoView() on a wrapper parent node (e.g. ) - // wouldn't guarantee that it will be *fully* brought into view. - // As a workaround, we'll have two anchor spans, and scroll each into view. - if (scrollAnchorEndRef.current !== null) { - scrollAnchorEndRef.current.scrollIntoView({ - behavior: 'auto', - block: 'nearest', - inline: 'nearest', - }); - } - if (scrollAnchorStartRef.current !== null) { - // We scroll the start anchor last because it's - // more important for it to be in the view. - scrollAnchorStartRef.current.scrollIntoView({ - behavior: 'auto', - block: 'nearest', - inline: 'nearest', - }); - } - } - }, [id, isSelected, lastScrolledIDRef]); - const handleMouseDown = useCallback( ({ metaKey }) => { if (id !== null) { @@ -138,8 +82,6 @@ export default function ElementView({ data, index, style }: Props) { const { depth, displayName, key, type } = ((element: any): Element); - const showDollarR = - isSelected && (type === ElementTypeClass || type === ElementTypeFunction); const showBadge = type === ElementTypeMemo || type === ElementTypeForwardRef; let className = styles.Element; @@ -162,45 +104,26 @@ export default function ElementView({ data, index, style }: Props) { ...style, // "style" comes from react-window // Left padding presents the appearance of a nested tree structure. - paddingLeft: '0.25rem', - - // These style overrides enable the background color to fill the full visible width, - // when combined with the CSS tweaks in Tree. - // A lot of options were considered; this seemed the one that requires the least code. - // See https://github.com/bvaughn/react-devtools-experimental/issues/9 - width: undefined, - minWidth: '100%', - position: 'relative', - marginBottom: `-${style.height}px`, + // We must use padding rather than margin/left because of the selected background color. + paddingLeft: `calc(${depth} * var(--indentation-size))`, }} > -
- {ownerID === null ? ( ) : null} - - - {key && ( - -  key= - "{key}" - - )} - - {showDollarR &&  == $r} - + + < + + {key && ( + +  key= + + "{truncateText(`${key}`, 10)}" + + + )} + > + {showBadge && ( {type === ElementTypeMemo ? 'Memo' : 'ForwardRef'} diff --git a/src/devtools/views/Components/Guidelines.css b/src/devtools/views/Components/Guideline.css similarity index 100% rename from src/devtools/views/Components/Guidelines.css rename to src/devtools/views/Components/Guideline.css diff --git a/src/devtools/views/Components/Guidelines.js b/src/devtools/views/Components/Guideline.js similarity index 76% rename from src/devtools/views/Components/Guidelines.js rename to src/devtools/views/Components/Guideline.js index 30332ea9bf..3ba9760da0 100644 --- a/src/devtools/views/Components/Guidelines.js +++ b/src/devtools/views/Components/Guideline.js @@ -1,6 +1,6 @@ // @flow -import React, { Fragment, useContext, useMemo } from 'react'; +import React, { useContext, useMemo } from 'react'; import { TreeStateContext } from './TreeContext'; import TreeFocusedContext from './TreeFocusedContext'; import { SettingsContext } from '../Settings/SettingsContext'; @@ -8,23 +8,7 @@ import { StoreContext } from '../context'; import { useSubscription } from '../hooks'; import Store from '../../store'; -import styles from './Guidelines.css'; - -export default function Guidelines(_: {||}) { - const { selectedElementID } = useContext(TreeStateContext); - const treeFocused = useContext(TreeFocusedContext); - - return ( - - - - ); -} +import styles from './Guideline.css'; type Data = {| depth: number, @@ -32,23 +16,20 @@ type Data = {| stopIndex: number, |}; -type Props = {| - className: string, - elementID: number | null, -|}; - -function Guideline({ className, elementID }: Props) { - const store = useContext(StoreContext); +export default function Guideline(_: {||}) { const { lineHeight } = useContext(SettingsContext); + const store = useContext(StoreContext); + const { selectedElementID } = useContext(TreeStateContext); + const treeFocused = useContext(TreeFocusedContext); const subscription = useMemo( () => ({ getCurrentValue: () => { - if (elementID === null) { + if (selectedElementID === null) { return null; } - const element = store.getElementByID(elementID); + const element = store.getElementByID(selectedElementID); if ( element === null || element.isCollapsed || @@ -92,7 +73,7 @@ function Guideline({ className, elementID }: Props) { }; }, }), - [elementID, store] + [selectedElementID, store] ); const data = useSubscription(subscription); @@ -104,11 +85,13 @@ function Guideline({ className, elementID }: Props) { return (
diff --git a/src/devtools/views/Components/SelectedElement.css b/src/devtools/views/Components/SelectedElement.css index 9ad029099d..d9ae086b4b 100644 --- a/src/devtools/views/Components/SelectedElement.css +++ b/src/devtools/views/Components/SelectedElement.css @@ -33,11 +33,13 @@ .Component, .Owner { - overflow: hidden; - text-overflow: ellipsis; color: var(--color-component-name); font-family: var(--font-family-monospace); font-size: var(--font-size-monospace-normal); + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + max-width: 100%; } .Component:before, .Owner:before { diff --git a/src/devtools/views/Components/Tree.css b/src/devtools/views/Components/Tree.css index 3efc1bf863..e3ddfeb37d 100644 --- a/src/devtools/views/Components/Tree.css +++ b/src/devtools/views/Components/Tree.css @@ -7,6 +7,14 @@ border-top: 1px solid var(--color-border); } +.List { + overflow-x: hidden !important; +} + +.InnerElementType { + overflow-x: hidden; +} + .SearchInput { flex: 0 0 42px; display: flex; diff --git a/src/devtools/views/Components/Tree.js b/src/devtools/views/Components/Tree.js index 74972f7cd4..3e5b75f99e 100644 --- a/src/devtools/views/Components/Tree.js +++ b/src/devtools/views/Components/Tree.js @@ -22,7 +22,7 @@ import SearchInput from './SearchInput'; import { ComponentFiltersModalContextController } from './ComponentFiltersModalContext'; import ToggleComponentFiltersModalButton from './ToggleComponentFiltersModalButton'; import ComponentFiltersModal from './ComponentFiltersModal'; -import Guidelines from './Guidelines'; +import Guideline from './Guideline'; import TreeFocusedContext from './TreeFocusedContext'; import styles from './Tree.css'; @@ -32,7 +32,6 @@ export type ItemData = {| isNavigatingWithKeyboard: boolean, lastScrolledIDRef: { current: number | null }, onElementMouseEnter: (id: number) => void, - showIndentLines: boolean, treeFocused: boolean, |}; @@ -60,7 +59,7 @@ export default function Tree(props: Props) { const [treeFocused, setTreeFocused] = useState(false); - const { lineHeight, showIndentLines } = useContext(SettingsContext); + const { lineHeight } = useContext(SettingsContext); // Make sure a newly selected element is visible in the list. // This is helpful for things like the owners list and search. @@ -264,7 +263,6 @@ export default function Tree(props: Props) { isNavigatingWithKeyboard, onElementMouseEnter: handleElementMouseEnter, lastScrolledIDRef, - showIndentLines, treeFocused, }), [ @@ -272,7 +270,6 @@ export default function Tree(props: Props) { isNavigatingWithKeyboard, handleElementMouseEnter, lastScrolledIDRef, - showIndentLines, treeFocused, ] ); @@ -310,7 +307,6 @@ export default function Tree(props: Props) { itemCount={numElements} itemData={itemData} itemSize={lineHeight} - overscanCount={3} ref={listRef} width={width} > @@ -326,6 +322,31 @@ export default function Tree(props: Props) { ); } +function updateIndentationSizeVar(innerDiv: HTMLDivElement): void { + const list = ((innerDiv.parentElement: any): HTMLDivElement); + + let indentationSize = + parseFloat(getComputedStyle(list).getPropertyValue('--indentation-size')) || + 12; + + let maxChildWidth = 0; + for (let child of innerDiv.children) { + const { lastElementChild } = child; + // Skip over e.g. the guideline element + if (lastElementChild != null) { + const bounds = ((lastElementChild.getBoundingClientRect(): any): DOMRect); + maxChildWidth = Math.max(maxChildWidth, bounds.x + bounds.width); + } + } + + indentationSize = Math.min( + 12, + (list.clientWidth / maxChildWidth) * indentationSize + ); + + list.style.setProperty('--indentation-size', `${indentationSize}px`); +} + function InnerElementType({ children, style, ...rest }) { const { ownerID } = useContext(TreeStateContext); @@ -335,13 +356,12 @@ function InnerElementType({ children, style, ...rest }) { // and ensure that once we've grown to a new max size, we don't shrink below it. // This improves the user experience when scrolling between wide and narrow rows. const divRef = useRef(null); - const [minWidth, setMinWidth] = useState(null); + // TODO This is a valid warning, but we're ignoring it for the time being. // eslint-disable-next-line react-hooks/exhaustive-deps useEffect(() => { if (divRef.current !== null) { - const measuredWidth = divRef.current.offsetWidth; - setMinWidth(w => Math.max(w || 0, measuredWidth)); + updateIndentationSizeVar(divRef.current); } }); @@ -349,7 +369,11 @@ function InnerElementType({ children, style, ...rest }) { // This will cause a render with 100% min-width, a measurement // in an effect, and a second render where we know the width. useEffect(() => { - const invalidateMinWidth = () => setMinWidth(null); + const invalidateMinWidth = () => { + if (divRef.current !== null) { + updateIndentationSizeVar(divRef.current); + } + }; window.addEventListener('resize', invalidateMinWidth); return () => window.removeEventListener('resize', invalidateMinWidth); }, []); @@ -359,7 +383,6 @@ function InnerElementType({ children, style, ...rest }) { const [prevOwnerID, setPrevOwnerID] = useState(ownerID); if (ownerID !== prevOwnerID) { setPrevOwnerID(ownerID); - setMinWidth(null); } // This style override enables the background color to fill the full visible width, @@ -369,16 +392,11 @@ function InnerElementType({ children, style, ...rest }) { return (
- + {children}
); diff --git a/src/devtools/views/Settings/Settings.js b/src/devtools/views/Settings/Settings.js index 30b83803cc..645ee25422 100644 --- a/src/devtools/views/Settings/Settings.js +++ b/src/devtools/views/Settings/Settings.js @@ -11,14 +11,9 @@ import styles from './Settings.css'; function Settings(_: {||}) { const store = useContext(StoreContext); - const { - displayDensity, - setDisplayDensity, - showIndentLines, - setShowIndentLines, - theme, - setTheme, - } = useContext(SettingsContext); + const { displayDensity, setDisplayDensity, theme, setTheme } = useContext( + SettingsContext + ); const captureScreenshotsSubscription = useMemo( () => ({ @@ -62,13 +57,6 @@ function Settings(_: {||}) { [setTheme] ); - const updateShowIndentLines = useCallback( - ({ currentTarget }) => { - setShowIndentLines(currentTarget.checked); - }, - [setShowIndentLines] - ); - const updateCaptureScreenshotsWhileProfiling = useCallback( ({ currentTarget }) => { store.captureScreenshots = currentTarget.checked; @@ -155,14 +143,6 @@ function Settings(_: {||}) { />{' '} Collapse newly added components by default -
{store.supportsCaptureScreenshots && ( diff --git a/src/devtools/views/Settings/SettingsContext.js b/src/devtools/views/Settings/SettingsContext.js index ac371cbec7..4f79131554 100644 --- a/src/devtools/views/Settings/SettingsContext.js +++ b/src/devtools/views/Settings/SettingsContext.js @@ -16,9 +16,6 @@ type Context = {| // Specified as a separate prop so it can trigger a re-render of FixedSizeList. lineHeight: number, - showIndentLines: boolean, - setShowIndentLines: (value: boolean) => void, - theme: Theme, setTheme(value: Theme): void, |}; @@ -47,10 +44,6 @@ function SettingsContextController({ 'React::DevTools::displayDensity', 'compact' ); - const [showIndentLines, setShowIndentLines] = useLocalStorage( - 'React::DevTools::showIndentLines', - true - ); const [theme, setTheme] = useLocalStorage( 'React::DevTools::theme', 'auto' @@ -133,8 +126,6 @@ function SettingsContextController({ setDisplayDensity, theme, setTheme, - showIndentLines, - setShowIndentLines, lineHeight: displayDensity === 'compact' ? compactLineHeight @@ -144,9 +135,7 @@ function SettingsContextController({ comfortableLineHeight, compactLineHeight, displayDensity, - showIndentLines, setDisplayDensity, - setShowIndentLines, setTheme, theme, ] diff --git a/src/devtools/views/utils.js b/src/devtools/views/utils.js index 8cebefa319..c04f8bddd1 100644 --- a/src/devtools/views/utils.js +++ b/src/devtools/views/utils.js @@ -170,3 +170,16 @@ export function downloadFile(filename: string, text: string): void { ((document.body: any): HTMLBodyElement).removeChild(element); } + +export function truncateText(text: string, maxLength: number): string { + const { length } = text; + if (length > maxLength) { + return ( + text.substr(0, Math.floor(maxLength / 2)) + + '…' + + text.substr(length - Math.ceil(maxLength / 2) + 1) + ); + } else { + return text; + } +}