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

[dashboard][performance] dashboard grid item performs 2 DOM queries every render #199361

Closed
nreese opened this issue Nov 7, 2024 · 1 comment · Fixed by #199390
Closed

[dashboard][performance] dashboard grid item performs 2 DOM queries every render #199361

nreese opened this issue Nov 7, 2024 · 1 comment · Fixed by #199390
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas

Comments

@nreese
Copy link
Contributor

nreese commented Nov 7, 2024

#182535 updated src/plugins/dashboard/public/dashboard_container/component/grid/dashboard_grid_item.tsx with 2 document.querySelector queries every render. In a worse case, I logged this as a 50 millisecond delay but most delays are less then one millisecond.

Instead, these DOM elements should be passed to the DashboardGridItem as props. As an added benefit, props will decouple the DashboardGridItem component from internal selector implementations of the components that are getting searched for.

To see the effect I added some performance metrics around the code

    const start = performance.now();
    const dashboardContainerTopOffset =
      (document.querySelector('.dashboardContainer') as HTMLDivElement)?.offsetTop || 0;
    const globalNavTopOffset =
      (document.querySelector('#app-fixed-viewport') as HTMLDivElement)?.offsetTop || 0;
    const end = performance.now();

    const time = end - start;

    console.log('time (milliseconds)', time);

When rendering web logs sample data dashboard the output is
Image

There is a larger performance penalty when moving a panel around as seen below
Image

@nreese nreese added bug Fixes for quality problems that affect the customer experience Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas labels Nov 7, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@nreese nreese self-assigned this Nov 11, 2024
@nreese nreese closed this as completed in 9f54503 Nov 18, 2024
nreese added a commit to nreese/kibana that referenced this issue Nov 18, 2024
…199390)

Closes elastic#199361

While investigating, I found that fetching DOM element with id
`app-fixed-viewport` is a common pattern. I created the hook
`useAppFixedViewport` to consolidate this logic into a single location.
The hook only performs the DOM look-up on first render and then avoids
the DOM look-up on each additional render.

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
(cherry picked from commit 9f54503)

# Conflicts:
#	.github/CODEOWNERS
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this issue Dec 12, 2024
…199390)

Closes elastic#199361

While investigating, I found that fetching DOM element with id
`app-fixed-viewport` is a common pattern. I created the hook
`useAppFixedViewport` to consolidate this logic into a single location.
The hook only performs the DOM look-up on first render and then avoids
the DOM look-up on each additional render.

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants