Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TreeView: Improve performance when rendering lots of items #2460

Merged
merged 7 commits into from
Oct 20, 2022

Conversation

colebemis
Copy link
Contributor

@colebemis colebemis commented Oct 20, 2022

This PR dramatically improves the rendering performance of TreeView. We accomplished this in two ways:

  1. We don't render subtrees of items that are collapsed (not visible)
  2. We don't generate unique styles for each item

Stress test

TreeView with 1000 directories containing 100 files each:

Before

Page fails to load:

CleanShot 2022-10-20 at 11 50 11

After

👉 Try it out

CleanShot.2022-10-20.at.11.47.04.mp4

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

@changeset-bot
Copy link

changeset-bot bot commented Oct 20, 2022

🦋 Changeset detected

Latest commit: 3d4566b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Oct 20, 2022

size-limit report 📦

Path Size
dist/browser.esm.js 77.99 KB (0%)
dist/browser.umd.js 78.64 KB (0%)

@colebemis colebemis temporarily deployed to github-pages October 20, 2022 18:09 Inactive
@colebemis colebemis temporarily deployed to github-pages October 20, 2022 18:30 Inactive
@colebemis colebemis marked this pull request as ready for review October 20, 2022 18:56
@colebemis colebemis requested review from a team and josepmartins October 20, 2022 18:56
@colebemis colebemis temporarily deployed to github-pages October 20, 2022 18:57 Inactive
@colebemis colebemis requested review from joshblack and removed request for josepmartins October 20, 2022 19:21
Copy link
Member

@joshblack joshblack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super slick sx find 🔥

@colebemis
Copy link
Contributor Author

I'm going to wait for confirmation from @jdrush89 that this improves rendering performance in Repos.

@colebemis
Copy link
Contributor Author

Super slick sx find 🔥

That was all @siddharthkp 🙇

@colebemis colebemis temporarily deployed to github-pages October 20, 2022 22:49 Inactive
@colebemis
Copy link
Contributor Author

@jdrush89 confirmed that these changes sped up the rendering performance of TreeView in dotcom ✅ 1000 directories now render in 833 ms (previously it was 3888 ms). 100 items render in ~90 ms (previously it was 1450 ms)

@colebemis colebemis merged commit 1f25c90 into main Oct 20, 2022
@colebemis colebemis deleted the treeview-lots-of-items branch October 20, 2022 23:01
@primer-css primer-css mentioned this pull request Oct 20, 2022
josepmartins pushed a commit that referenced this pull request Oct 21, 2022
* Avoid unecessary style recalculation

* Add stress test story

* Only render subtree if it's expanded

* Don't expand current item by default

* Update controlled story

* Create popular-taxis-yawn.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants