feat: gray out non-matches when searching by starpit · Pull Request #554 · webpack/webpack-bundle-analyzer

starpit · 2023-01-30T16:04:06Z

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 · 2023-01-31T07:48:25Z

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 · 2023-01-31T14:14:43Z

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 · 2023-01-31T14:26:55Z

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

starpit · 2023-01-31T15:03:19Z

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 · 2023-02-01T07:29:44Z

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.

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

starpit · 2023-02-02T17:07:23Z

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 · 2023-02-03T10:59:19Z

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:

https://github.com/webpack-contrib/webpack-bundle-analyzer/blob/38930418af195b5ecbc857ce1db43bf83256d726/client/components/Search.jsx#L42-L44

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 · 2023-02-13T08:39:21Z

This is now in v4.8.0! 🚀

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [webpack-bundle-analyzer](https://github.com/webpack-contrib/webpack-bundle-analyzer) | devDependencies | minor | [`4.7.0` -> `4.8.0`](https://renovatebot.com/diffs/npm/webpack-bundle-analyzer/4.7.0/4.8.0) | --- ### Release Notes <details> <summary>webpack-contrib/webpack-bundle-analyzer</summary> ### [`v4.8.0`](https://github.com/webpack-contrib/webpack-bundle-analyzer/blob/HEAD/CHANGELOG.md#480) [Compare Source](webpack/webpack-bundle-analyzer@v4.7.0...v4.8.0) - **Improvement** - Support reading large (>500MB) stats.json files ([#423](webpack/webpack-bundle-analyzer#423) by [@henry-alakazhang](https://github.com/henry-alakazhang)) - Improve search UX by graying out non-matches ([#554](webpack/webpack-bundle-analyzer#554) by [@starpit](https://github.com/starpit)) - **Internal** - Add Node.js v16.x to CI and update GitHub actions ([#539](webpack/webpack-bundle-analyzer#539) by [@amareshsm](https://github.com/amareshsm)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [x] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1783 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

starpit force-pushed the gray-out-search-non-matches branch from 7c2d117 to aa96b63 Compare January 31, 2023 14:14

starpit force-pushed the gray-out-search-non-matches branch from aa96b63 to 8680d12 Compare January 31, 2023 18:06

feat: gray out non-matches when searching

0ee0c48

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

starpit force-pushed the gray-out-search-non-matches branch from 8680d12 to 0ee0c48 Compare February 2, 2023 17:03

Merge branch 'master' into gray-out-search-non-matches

9c129d8

valscion approved these changes Feb 13, 2023

View reviewed changes

Add changelog entry

320ec80

valscion merged commit f959f9c into webpack:master Feb 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: gray out non-matches when searching#554

feat: gray out non-matches when searching#554
valscion merged 3 commits intowebpack:masterfrom
starpit:gray-out-search-non-matches

starpit commented Jan 30, 2023 •

edited by valscion

Loading

Uh oh!

valscion commented Jan 31, 2023

Uh oh!

starpit commented Jan 31, 2023 •

edited

Loading

Uh oh!

valscion commented Jan 31, 2023

Uh oh!

starpit commented Jan 31, 2023 •

edited

Loading

Uh oh!

valscion commented Feb 1, 2023

Uh oh!

starpit commented Feb 2, 2023 •

edited

Loading

Uh oh!

valscion commented Feb 3, 2023

Uh oh!

valscion left a comment

Uh oh!

valscion commented Feb 13, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

starpit commented Jan 30, 2023 • edited by valscion Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

FoamTree bug?

Uh oh!

valscion commented Jan 31, 2023

Uh oh!

starpit commented Jan 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

valscion commented Jan 31, 2023

Uh oh!

starpit commented Jan 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

valscion commented Feb 1, 2023

Uh oh!

starpit commented Feb 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

valscion commented Feb 3, 2023

Uh oh!

valscion left a comment

Choose a reason for hiding this comment

Uh oh!

valscion commented Feb 13, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

starpit commented Jan 30, 2023 •

edited by valscion

Loading

starpit commented Jan 31, 2023 •

edited

Loading

starpit commented Jan 31, 2023 •

edited

Loading

starpit commented Feb 2, 2023 •

edited

Loading