Components tree changes to improve HOCs and browsing deep trees

[bvaughn]

This PR introduces a drastically new UX behavior to the Componts tab: Horizontal scrolling is removed entirely in favor of dynamically changing indentation so that all rows fit within the viewport. The change also enabled me to simplify the DOM structure of each row in the tree, since most of the complexity before was only needed to support background color of overflowing rows.

The bulk of how that change works is outlined here:

react-devtools-experimental/src/devtools/views/Components/Tree.js

Lines 325 to 365 in 6b6e571

	// Indentation size can be adjusted but child width is fixed.
	// We need to adjust indentations so the widest child can fit without overflowing.
	// Sometimes the widest child is also the deepest in the tree:
	// ┏----------------------┓
	// ┆ <Foo> ┆
	// ┆ ••••<Foobar> ┆
	// ┆ ••••••••<Baz> ┆
	// ┗----------------------┛
	//
	// But this is not always the case.
	// Even with the above example, a change in indentation may change the overall widest child:
	// ┏----------------------┓
	// ┆ <Foo> ┆
	// ┆ ••<Foobar> ┆
	// ┆ ••••<Baz> ┆
	// ┗----------------------┛
	//
	// In extreme cases this difference can be important:
	// ┏----------------------┓
	// ┆ <ReallyLongName> ┆
	// ┆ ••<Foo> ┆
	// ┆ ••••<Bar> ┆
	// ┆ ••••••<Baz> ┆
	// ┆ ••••••••<Qux> ┆
	// ┗----------------------┛
	//
	// In the above example, the current indentation is fine,
	// but if we naively assumed that the widest element is also the deepest element,
	// we would end up compressing the indentation unnecessarily:
	// ┏----------------------┓
	// ┆ <ReallyLongName> ┆
	// ┆ •<Foo> ┆
	// ┆ ••<Bar> ┆
	// ┆ •••<Baz> ┆
	// ┆ ••••<Qux> ┆
	// ┗----------------------┛
	//
	// The way we deal with this is to compute the max indentation size that can fit each child,
	// given the child's fixed width and depth within the tree.
	// Then we take the smallest of these indentation sizes...
	function updateIndentationSizeVar(

To support this change and avoid odd UI/UX interactions, I also made the following adjustments:

• HOCs are parsed out of display names and shown as separate badges (see below for more).
• Keys are truncated to show a max of 10 characters (e.g. "abcdefghijklmn" becomes "abcde…lmn"). This avoids an edge case of really long keys causing indentation to shrink drastically.
• The "== $r" suffix is no longer shown for selected elements, since showing it would change the width of the element (which could cause the dynamic indentation to jump as new elements are selected). We can add this label back in the right hand property if we think it's important, although we also now have the debug button that logs things to the console as well (so maybe it's less important than it once was).
• Recently added indent lines have been removed because they looked bad when indentation shrinks below a certain threshold. These have been replaced with a new background color that highlights which elements are descendants of the selected element. (See below for a demo video.)
• Added a new component filter type for HOCs. (See below for a demo video.)

What does "dynamic indentation" mean?

Let's take a look at Twitter for an example. As we scroll deeper into the component tree, it's easy to get "lost" because of the empty horizontal space. After this change, scrolling deeper into the tree will cause the indentation size to be adjusted so the deepest/widest components fit fully within the viewport.

Here is a side by side before and after:

Remove HOCs from display names and show as badges

HOCs make component names longer and harder to parse. (They also require indentation to be compressed more aggressively.) This diff tries a new approach with them- removing the HOC portions of the display name and pulling them out into badges.

For example, here's what a really long HOC display name might look like before this change:

And here's what it looks like afterward:

There is one downside for this change: Search no longer finds matches based on the HOC portion of the display name (since that bit is parsed out). We could change the search behavior to also include HOCs if we think it's important, but I don't think it's necessary for now.

New HOC component filter

Since I've committed to identifying HOCs as display names that contain parenthesis, it seemed worth going one step further and adding a convenience HOC filter:

Replace indent lines with background color

Indent lines looked bad when combined with dynamic indentation:

The primary purpose these lines served is showing the hierarchy. I think we can achieve the same thing using background color to group descendants of the selected component.

Here is a side by side before and after:

Should we debounce/animate indentation changes?

I experimented with this (as can be seen in the commit history) but ultimately decided against it. It feels more natural and fluid for indentation to adjust as soon as new rows are added or removed. Initial feedback from the few people I shared an early build with seems to support this decision.

Why not truncate long display names as well?

There are two basic approaches to truncating long display names:

• CSS based (e.g. max-width: x%). The downside of this is that browsers more aggressively truncate text that's near the right edge of its container. This means that names might be truncated in an extreme way as the tree gets deeper (e.g. "Foobar" might be truncated to "F..." unnecessarily).
• JS-based truncation would not have the above flaw, but would potentially break our inline search match highlighting logic. (It would also be a bit harder to scale with the size of the viewport.)

I think in most cases, long names are due to HOCs that get added to the display name of what they're wrapping. I think we may be able to address this problem separately, by pulling out the HOC portion of the name.

[gaearon]

Why? I added this because of visual artifacts on fast scrolling. I don’t think it’s solved yet. Maybe we need to look into why it happens though.

[bvaughn]

Couple of reasons:

1. I don't think overscan actually helps much with artifacts. It only helps with the first paint or two after a fast scroll. After that, react-window is racing to keep up with a passive "scroll" event regardless.
2. With dynamic indentation, I didn't want the size of hidden/offscreen rows visually impacting the indentation size too much, because it felt a bit awkward in some cases.

I don't feel very strongly about this, but it did seem like a positive change.

[bvaughn]

Okay, I think I'm done with this batch of changes. Code review and/or testing welcome if you're interested!

[gaearon]

A few things I've noticed:

• Navigating the tree with arrows (e.g. "right" to expand) doesn't seem to adjust the indentation. So eventually things go offscreen until you start scrolling.
• Same for using "select via DOM node".
• Sometimes the indentation jumps too much over a short period of time if you're unlucky enough to have a wide element just above and just below the current screen. That can be very jarring and disorienting. I wonder if there's a threshold of either time or scroll distance (or combination) that should avoid adjusting the indentation.
• Pressing "expand" with mouse causes the arrow to "jump" from under the cursor. Feels a bit unexpected.

[bvaughn]

• Navigating the tree with arrows (e.g. "right" to expand) doesn't seem to adjust the indentation. So eventually things go offscreen until you start scrolling.
• Same for using "select via DOM node".

I could not reproduce this, but conceptually I believe it was due to the fact that I am caching measurements per DOM item with a WeakMap and I was keying rows by index rather than ID (d'oh!) which has been fixed in dc08e80.

• Sometimes the indentation jumps too much over a short period of time if you're unlucky enough to have a wide element just above and just below the current screen. That can be very jarring and disorienting. I wonder if there's a threshold of either time or scroll distance (or combination) that should avoid adjusting the indentation.

No immediate ideas come to mind. Having tried several forms of animation and/or debouncing over the past few days, I think I've decided that I dislike them all- but maybe we can brainstorm this and come up with something this week.

• Pressing "expand" with mouse causes the arrow to "jump" from under the cursor. Feels a bit unexpected.

Yeah, I see why this is not ideal, but I'm also not sure how to fix it given the constraints we're working under. Maybe we can brainstorm it some this week while you're in town. Although I also don't think it's a show-stopper, when compared to the previous horizontal scrolling.

[gaearon]

Few more thoughts:

• I think we may be able to solve horizontal jumps by being less rigorous about moving things to the right. It's understandable we always want to move them to the left if necessary to avoid clipping. However, moving them to the right doesn't always solve an immediate need. Instead, we do it mostly to avoid drift. This means we should be able to be a bit more selective about when to move to the right (e.g. if we drifted too far and are ready to make a small jump), and when to avoid or "soften" it (e.g. if the horizontal jump would be too large).

• As discussed, the white padding breaks the illusion of scrolling.

[bvaughn]

• I think we may be able to solve horizontal jumps by being less rigorous about moving things to the right...

This sounds potentially promising. Let's pair on it tomorrow.

• As discussed, the white padding breaks the illusion of scrolling.

Padding removed in 8261a3f.

Should we want to add it back in a way that also scrolls out of view, I added an entry to the react-window FAQs detailing how we could do it.

[bvaughn]

• I think we may be able to solve horizontal jumps by being less rigorous about moving things to the right...

f713265 takes a shot at this!

react-devtools-experimental/src/devtools/views/Components/Tree.js

Lines 411 to 421 in f713265

	// It's very important to shrink indentation so that nothing gets clipped.
	// But it is less important to increase indentation when something wide is scrolled out of view.
	// In fact, increasing too much leads to visual "jumping" which can be unpleasant.
	// To avoid this, we only increase by a maximum of some threshold (MAX_INDENTATION_SIZE_INCREASE).
	const newIndentationSize =
	indentationSizeRef.current > maxIndentationSize
	? maxIndentationSize
	: Math.min(
	maxIndentationSize,
	indentationSizeRef.current + MAX_INDENTATION_SIZE_INCREASE
	);

Before

0.5 pixel max

0.25 pixel max

[bvaughn]

Commit 78cf211 tries to address the concern of "jumping" in a more extreme way: We decrease indentation to fit wider/deeper trees, but we do not increase it again (unless the window resizes). I think this also helps with cases where clicking to collapse a row might otherwise jump horizontally beneath your cursor.

[bvaughn]

Since I've committed to identifying HOCs as display names containing parenthesis, we might as well go one step further and add a convenience filter?

[SavePointSam]

I'm not too sure why someone would want to filter out HOC wrapped components. Though it might be nice to have an inverse filter option in the case that an engineer is working to clean them up. Or maybe even filter by HOC name so that it would be easy to find all occurrences of an HOC that a team might be wanting to remove?

[bvaughn]

I'm not too sure why someone would want to filter out HOC wrapped components.

People have asked for this feature in the past.

Though it might be nice to have an inverse filter option in the case that an engineer is working to clean them up. Or maybe even filter by HOC name so that it would be easy to find all occurrences of an HOC that a team might be wanting to remove?

Filters are exclusive/reductive. It was a conscious decision not to mix and match inclusive and exclusive filters because I thought it would result in a confusing UX.

That being said, you could do what you're asking for (only showing HOCs) with an exclusive "name" filter of ^[^(]+$.

[SavePointSam]

People have asked for this feature in the past.

Great!

Filters are exclusive/reductive. It was a conscious decision not to mix and match inclusive and exclusive filters

Sounds good! Also a great decision, supporting inclusive and exclusive is a nightmare. I've had to support it in the past, haha.

[bvaughn]

I'm going to roll forward with this. We can iterate on it more as needed.

[SavePointSam]

💯 🎉

[chuckdries]

As a relatively experienced react user where many of my projects have lots of react-redux connected components, I absolutely love the decision to hide HOCs. That being said, when I was a beginner just learning, I relied really heavily on (somewhat blindly) making changes and seeing what they did in devtools to learn how things worked. Are we worried that hiding HOCs from the tree might obscure what they really are to learners? (Is the fact that they exist in the tree even important information for users of HOCs or just an implementation detail?)

[bvaughn]

HOCs are not hidden from the tree (unless you explicitly add a component filter to hide them). This PR just changes the way they are presented to (hopefully) be more readable. The same tree structure (HOC outside with decorated component inside) still exists.