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

Performance Issue: Load time increases when multiple components using sl-popup are added #2179

Closed
Shraddha-V22 opened this issue Sep 23, 2024 · 1 comment · May be fixed by TaylorBundy/swiper#3
Labels
bug Things that aren't working right in the library.

Comments

@Shraddha-V22
Copy link

The sl-popup component includes a function called handleAnchorChange, which listens for changes in the anchor slot. When triggered, this function checks if the anchor element exists. If it does, it calls the autoUpdate function from @floating-ui/dom to ensure the floating element remains anchored to its reference element, such as when scrolling and resizing the screen.

The primary issue is that the autoUpdate function is being called as soon as the sl-popup component is loaded into the DOM, even when the popup is not active. This unnecessary execution of the autoUpdate function is leading to delays, especially when multiple components using sl-popup are rendered on the page simultaneously.

Steps to reproduce

  • Clone shoelace repository
  • Add 50 to 100 sl-select component in select.md file
  • Check performance using Chrome Profiler

autoUpdate-execution-time

By modifying the handleAnchorChange function to include a condition that checks whether the dropdown is open, the execution time during page load significantly decreases when multiple sl-select components are present.

private async handleAnchorChange() {
    await this.stop(); // cleanup function
    this.anchorEl = this.querySelector<HTMLElement>('[slot="anchor"]');
    // If the anchor is valid, start it up
    if (this.anchorEl && this.active) { // Added condition to check if the dropdown is open (this.active)
      if (!this.anchorEl) {
        return;
      }
      this.cleanup = autoUpdate(this.anchorEl, this.popup, () => {
        this.reposition();
      });
    }
  }
  ...
  render() {
    return html`
      <slot name="anchor" @slotchange=${this.handleAnchorChange}></slot>
      ...
  }

handleAnchorChange-exection-on-active-false

@Shraddha-V22 Shraddha-V22 added the bug Things that aren't working right in the library. label Sep 23, 2024
claviska added a commit that referenced this issue Sep 23, 2024
@claviska
Copy link
Member

Nice catch! I've proposed a fix in #2179.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Things that aren't working right in the library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants