feat: gray out non-matches when searching

[starpit]

This PR reduces the saturation (from 60 to 10) for non-matches.

Note: there seems to be a bug in foamtree? It does not redraw some of the nodes until you hover over them. This seems to be a preexisting issue.

Fixes #553

Click to see the outdated details about the .redraw() behavior we've discussed in this PR

FoamTree bug?

This gif shows how randomly some of the nodes are not redrawn (by foamtree) until you hover over them. I think this bug is masked by the current highlighting behavior, which does not change the color of non-matching nodes.

[valscion]

This gif shows how randomly some of the nodes are not redrawn (by foamtree) until you hover over them. I think this bug is masked by the current highlighting behavior, which does not change the color of non-matching nodes.

Hmm, if this bug wasn't active earlier but is now visible, we will need to fix it before we can ship this feature.

Are you able to debug further on why this happens and if updating our version of FoamTree would help fix the situation?

Ever since #412 we've got FoamTree from an NPM package. I don't recall right now on how updating this package will happen so it needs some thought behind it. Looking through the changelogs of FoamTree could show if this bug has existed on FoamTree side and has now been fixed.

[starpit]

Hmm, I don't think FoamTree is open source? I don't even see a way to file an issue. You are using the latest version (3.5.0).

[valscion]

Yeah FoamTree is not open source but they do distribute it via npm

[starpit]

Interesting. If i change the .redraw(false, groupsToRedraw) to .redraw(), then the bug goes away. So either: a) the groupsToRedraw computation is incorrect on your side; or b) foamtree does not handle this properly.

Is it viable to work around this for now by always redrawing all groups, i.e. .redraw()?

---

Edit: Thinking this through more: if the highlight groups changes (which is where this redraw() is called)... then with this PR, we will always be redrawing almost all of the chunks... since the colors are likely to change for almost all groups, when the search term changes.

I have updated the PR along these lines.

[valscion]

Thanks for looking more into why this case happened 😊. I'm not that familiar with the treemap interaction code so it is very helpful to have you figure this out, too.

It sounds like the original code could've used the false argument as some sort of render optimization. Is the rendering now noticeably slower than it used to be? I wonder if we need to somehow have some sort of a debounce for the handling of search input value changes if redrawing the treemap becomes slow.

[starpit]

the false of .redraw(false) is the default behavior. it tells foamtree not to animate the update. the second argument is a hint to foamtree as to which groups may have changed. the bug is either in bundle analyzer's presentation of changed groups to foamtree, or in foamtree's algorithm that interprets this hint.

i have not noticed any rendering lag for the example above, which has maybe 100 top-level groups, and 600 total groups.

[valscion]

Okay thanks for the extra information!

That sounds like a useful optimization to have. I suspect that operation is costly so with users having even more groups and maybe a less powerful machine could get performance issues they weren't seeing earlier.

However, we are indeed debouncing changes done to the search input here:

webpack-bundle-analyzer/client/components/Search.jsx

Lines 42 to 44 in 3893041

	handleValueChange = _.debounce((event) => {
	this.informChange(event.target.value);
	}, 400)

so maybe the performance issue wouldn't be that bad. I don't know what other code paths call to this method that now calls .redraw()? I hope none related to cursor movements or some other commonly used events.

[valscion]

Ok I took this for a spin, too, and even with a CPU slowdown of 6x I can't see any performance issues with this code.

Thank you for the contribution, this looks ace! I'll create a changelog entry for this and merge after ☺️

[valscion]

This is now in v4.8.0! 🚀