powerhome/playbook

Performance Issue (Possibly in the Icon Kit)

gregblake opened this issue · 6 comments

gregblake commented

Describe the bug

@powerhome/rebel-alliance recently noticed performance issues in the Connect v3 web client. We think it could be related to the Icon kit.

To Reproduce

We encountered a performance issue on our local environments, while working on a Connect v3 web client feature. These comments describe the issue in more detail: comment 1, comment 2.

After troubleshooting the issue, I was able to recreate the slowness with only this change. Checking out this branch and testing with/without the changes on 5fc4db40addbb496076d3d7ddae4b10964e46abe is likely the easiest way to reproduce the issue.

The code that introduces the slow down only renders the <InlineLoading/> component (which, in turn, renders an <Icon/>) and the spinner icon from Font Awesome. This change also caused a slow down in the list of the v3 web client that renders an <Icon/>.

Shortly after that, @codemonium experienced something similar when adding a new feature to the web client. He added something that renders an <Icon/>, and the feature became extremely slow on his local environment. Igor confirmed that the component works perfectly in a brand new create-react-app, and he eventually narrowed it down to the <Icon/> component.

We also compared the performance in Chrome and Firefox. The above performance issues are not happening to us when we use Firefox on our local development environments.

In addition, we have not noticed a slow down in deployed environments.

Expected behavior

The same behavior, without a performance hit.

Screenshots

Here are screen recordings that @codemonium made:

Desktop (please complete the following information):

  • OS: MacOS
  • Browser: Chrome
  • Version: 95.0.4638.54 (Official Build) (x86_64)
samanthar8 commented

Hi Greg! We might have this tracked in Runway-- do you mind taking a look at this ticket, and seeing if it lines up with the issue you're seeing?

gregblake commented

Hey @samanthar8! Yes, I believe that ticket describes the same issue we've seen.

The only difference is that we notice the performance issue with fewer than 150 icons in a given view (at least on our local development environments).

samanthar8 commented

@gregblake thanks! On that story, is the solution that Stephen G commented something applicable to Connect?

"A quick update on this: this issue seems to mostly be related to the i2svg() functionality that we have implemented in Nitro. The animations when loading our flyouts have been very choppy due to this operation and it has also affected the overall loading times on our schedule page.

To get around this, I've created a PR that uses SVGs directly for the icons, negating the need for i2svg() to run for those elements. https://github.com/powerhome/nitro-web/pull/18346"

samanthar8 commented

Additionally, Jasper told me that "A current workaround that Stephen greer used at one point was to just use an <i> element with fontawesome as declared in their docs", which may not be the same thing referenced in the prior comment?

gregblake commented

@samanthar8 Thanks for mentioning those potential changes. I agree that if we made those changes, it would help to speed up Connect.

But, I believe making the change in Connect would have two downsides:

  1. Feature drift: at some point in the future, changes will be made to the <Icon/> component in Playbook (e.g. maybe a styling change will be made). Depending on the change, we may also need to make a similar change to Connect.
  2. It wouldn't solve the root cause of the performance issue. And, this issue likely effects many parts of our application, to varying degrees of impact. Generally speaking, performance issues like this happen on a sliding scale. If a page only has a few icons, the performance impact might not be noticeable to the user. But each additional icon on the view makes the performance incrementally slower. Almost every page in Nitro uses the top and left navigation menus, which contain roughly 60 icons (which doesn't include the icons in the main content of any given page). So, I believe making this performance improvement in Playbook would improve page load times on nearly every page in Nitro. In other words, the performance change would be more noticeable on pages with several hundred icons (like Connect v3). But it would also speed up performance on pages that have fewer than 150 icons (just to a lesser degree).

That's why I believe the best solution is to make one of the changes suggested in NUXE-36 directly in Playbook.

  • 👍1
co1in-1ewis commented

Closing and moving to PLAY-850 for follow-up.