这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@emma-sg
Copy link
Member

@emma-sg emma-sg commented Sep 2, 2025

Context

Because of the getComputedStyle call on every shoelace menu item, rendering menu items for each item of a list is slow: each call to getComputedStyle triggers a reflow, and this happens for each menu item on each menu on each list item. There's some discussion of this here: shoelace-style/shoelace#773 (comment).

Because of this, rendering the whole org list when there are hundreds of orgs locks up the main thread for sometimes several seconds. While pagination is possible for this, it seems a little pointless given we're already doing client-side filtering/search, so instead I've implemented virtualized scrolling.

Changes

This PR uses TanStack Virtual to implement virtualized scrolling for the superadmin org list. Rather than rendering all of the items all of the time, this only renders the items visible on screen, and updates when scrolling. This significantly improves the initial rendering time, as well as responsiveness when updating search and filters.

This does break subgrid rendering, which means we can't adjust the table spacing to match its contents automatically — but we've already got an overflow scroller on this table, so we can just set some sensible minimum column widths and amounts they expand when given space with minmax(<fixed_width>, <n>fr), and then handle long titles with truncation and title attributes.1

It also adds a patch to @shoelace-style/shoelace that swaps out getComputedStyle for LocalizeController to determine whether or not to display menu items as RTL. This significantly improves performance when rendering this list, as well as all other lists of items where each item has a dropdown menu. It also removes an older unused/unneeded patch for an older version of shoelace — shoelace now is now packaged slightly differently, and it seems the older patch isn't necessary anymore.

Finally, it adds debounced updates when entering search values, which improves the responsiveness of the search input, since searching many items with Fuse.js can take a bit of time. In the future, maybe we want to look into PartyTown, which would let us run Fuse.js in a web worker separate from the main thread and therefore not block user input while searching at all, but for now this reduces how often searches cause user-input-blocking main thread hangs.

Testing

Log into an instance with many orgs as superadmin (I tested with prod, but be careful!), and poke around the superadmin org list. Try entering search terms, switching filters, and scrolling.

Ensure that:

  • Links to orgs work (i.e. I haven't broken the table layout)
  • Dropdown menus work (i.e. I haven't broken menu items)

Footnotes

  1. We could use Shoelace tooltips for this, but given that a) native title tooltips are likely much faster to render, and b) this is mostly internal-facing anyways, I think title is the right approach here.

@emma-sg emma-sg requested a review from SuaYoo September 2, 2025 21:04
@socket-security
Copy link

socket-security bot commented Sep 2, 2025

@socket-security
Copy link

socket-security bot commented Sep 2, 2025

Warning

Review the following alerts detected in dependencies.

According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.

Action Severity Alert  (click "▶" to expand/collapse)
Warn High
safer-buffer@2.1.2 has Obfuscated code.

Confidence: 0.94

Location: Package overview

From: frontend/yarn.locknpm/safer-buffer@2.1.2

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/safer-buffer@2.1.2. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

updates SubmenuController to use `LocalizeController` instead of
`getComputedStyle` when determining RTL state, which saves an entire
layout per call. on long lists, this adds up quickly.
@SuaYoo SuaYoo self-requested a review September 8, 2025 19:12
Copy link
Member

@SuaYoo SuaYoo left a comment

Choose a reason for hiding this comment

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

We should implement org list backend pagination and filtering at some point, but agreed that this is a nice interim solution. Thanks!

@emma-sg emma-sg merged commit 50cdaa9 into main Sep 9, 2025
29 checks passed
@emma-sg emma-sg deleted the frontend-superadmin-org-list-perf branch September 9, 2025 05:23
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.

3 participants