Only shrink indentation. Don't increase it again. This avoids 'jumping'.
Brian Vaughn committed
Jun 3, 2019 at 08:01 UTC
78cf211286107ba178d163655ea53a0d2274c761
1 file changed
+30
-32
src/devtools/views/Components/Tree.js
+30
-32
@@ -27,6 +27,9 @@ import TreeFocusedContext from './TreeFocusedContext';
27
28
import styles from './Tree.css';
29
30
+// Never indent more than this number of pixels (even if we have the room).
31
+const DEFAULT_INDENTATION_SIZE = 12;
32
+
33
export type ItemData = {|
34
numElements: number,
35
isNavigatingWithKeyboard: boolean,
@@ -35,13 +38,6 @@ export type ItemData = {|
38
treeFocused: boolean,
39
|};
40
38
-// Never indent more than this number of pixels (even if we have the room).
39
-const DEFAULT_INDENTATION_SIZE = 12;
40
-
41
-// Never increase indentation by more than this number of pixels in a single adjustment.
42
-// This is to prevent things from "jumping" when a wide item is scrolled out of view.
43
-const MAX_INDENTATION_SIZE_INCREASE = 0.25;
44
-
41
type Props = {||};
42
43
export default function Tree(props: Props) {
@@ -378,12 +374,19 @@ export default function Tree(props: Props) {
374
function updateIndentationSizeVar(
375
innerDiv: HTMLDivElement,
376
cachedChildWidths: WeakMap<HTMLElement, number>,
381
- indentationSizeRef: {| current: number |}
377
+ indentationSizeRef: {| current: number |},
378
+ prevListWidthRef: {| current: number |}
379
): void {
380
const list = ((innerDiv.parentElement: any): HTMLDivElement);
381
const listWidth = list.clientWidth;
382
386
- let maxIndentationSize: number = DEFAULT_INDENTATION_SIZE;
383
+ // Reset the max indentation size if the width of the tree has increased.
384
+ if (listWidth > prevListWidthRef.current) {
385
+ indentationSizeRef.current = DEFAULT_INDENTATION_SIZE;
386
+ }
387
+ prevListWidthRef.current = listWidth;
388
+
389
+ let maxIndentationSize: number = indentationSizeRef.current;
390
391
for (let child of innerDiv.children) {
392
const depth = parseInt(child.getAttribute('data-depth'), 10) || 0;
@@ -408,21 +411,9 @@ function updateIndentationSizeVar(
411
maxIndentationSize = Math.min(maxIndentationSize, remainingWidth / depth);
412
}
413
411
- // It's very important to shrink indentation so that nothing gets clipped.
412
- // But it is less important to increase indentation when something wide is scrolled out of view.
413
- // In fact, increasing too much leads to visual "jumping" which can be unpleasant.
414
- // To avoid this, we only increase by a maximum of some threshold (MAX_INDENTATION_SIZE_INCREASE).
415
- const newIndentationSize =
416
- indentationSizeRef.current > maxIndentationSize
417
- ? maxIndentationSize
418
- : Math.min(
419
- maxIndentationSize,
420
- indentationSizeRef.current + MAX_INDENTATION_SIZE_INCREASE
421
- );
422
-
423
- list.style.setProperty('--indentation-size', `${newIndentationSize}px`);
424
-
425
- indentationSizeRef.current = newIndentationSize;
414
+ indentationSizeRef.current = maxIndentationSize;
415
+
416
+ list.style.setProperty('--indentation-size', `${maxIndentationSize}px`);
417
}
418
419
function InnerElementType({ children, style, ...rest }) {
@@ -432,23 +423,30 @@ function InnerElementType({ children, style, ...rest }) {
423
() => new WeakMap(),
424
[]
425
);
435
- const indentationSizeRef = useRef<number>(DEFAULT_INDENTATION_SIZE);
426
437
- // The list may need to scroll horizontally due to deeply nested elements.
438
- // We don't know the maximum scroll width up front, because we're windowing.
439
- // What we can do instead, is passively measure the width of the current rows,
440
- // and ensure that once we've grown to a new max size, we don't shrink below it.
441
- // This improves the user experience when scrolling between wide and narrow rows.
427
+ // This ref tracks the current indentation size.
428
+ // We decrease indentation to fit wider/deeper trees.
429
+ // We indentionally do not increase it again afterward, to avoid the perception of content "jumping"
430
+ // e.g. clicking to toggle/collapse a row might otherwise jump horizontally beneath your cursor,
431
+ // e.g. scrolling a wide row off screen could cause narrower rows to jump to the right some.
432
+ //
433
+ // The one exception for this is when the width of the tree increases.
434
+ // The user may have resized the window specifically to make more room for DevTools.
435
+ // In either case, this should reset our max indentation size logic.
436
+ const indentationSizeRef = useRef<number>(DEFAULT_INDENTATION_SIZE);
437
+ const prevListWidthRef = useRef<number>(0);
438
const divRef = useRef<HTMLDivElement | null>(null);
439
444
- // TODO This is a valid warning, but we're ignoring it for the time being.
440
+ // When we render new content, measure to see if we need to shrink indentation to fit it.
441
+ // TODO The lint warning is valid, but we are intentionally ignoring it for now.
442
// eslint-disable-next-line react-hooks/exhaustive-deps
443
useEffect(() => {
444
if (divRef.current !== null) {
445
updateIndentationSizeVar(
446
divRef.current,
447
cachedChildWidths,
451
- indentationSizeRef
448
+ indentationSizeRef,
449
+ prevListWidthRef
450
);
451
}
452
});