[Dashboard] Skip automatic scroll when panel is visible by cqliu1 · Pull Request #233226 · elastic/kibana

cqliu1 · 2025-08-27T22:39:24Z

Summary

This adds a check in dashboardApi.scrollToPanel comparing the panel's position and viewport and skips the automatic scroll if the panel is already in view. This improves the UX by removing the slightly jarring auto scroll whenever you edit any panel, especially noticeable with any panel that supports inline editing in Dashboard.

Before

After

Per @nreese feedback, I add a dashboardContainerRef to the dashboardInternalApi so we don't have to rely on classes to retrieve the dashboard container DOM element. This way we also avoid prop drilling the container ref from the dashboardRenderer component and make it accessible from anywhere the dashboardApi is available.

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
Flaky Test Runner was used on any tests changed
The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines
Review the backport guidelines and apply applicable backport:* labels.

Identify risks

Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.

See some risk examples
...

…ally

elasticmachine · 2025-08-27T22:41:05Z

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

nreese · 2025-08-27T22:55:42Z

src/platform/plugins/shared/dashboard/public/dashboard_api/track_panel.ts

+          const clientBottom = window.innerHeight;
+          const { top: panelTop, bottom: panelBottom } = panelRef.getBoundingClientRect();
+
+          console.log(


Lets remove console.log

Removed in 013211e

nreese · 2025-08-27T23:00:04Z

src/platform/plugins/shared/dashboard/public/dashboard_api/track_panel.ts

          scrollPosition = undefined;
        } else {
-          panelRef.scrollIntoView({ block: 'start', behavior: 'smooth' });
+          const dashboardTop =


Can we add a unit test for this?

Added in 38c09f6

looks like the test file did not make the commit

Oops, they're in 17b1d3d

…rd/skip-scroll-to-panel-in-view

…no-cache --fix'

…ub.com/cqliu1/kibana into dashboard/skip-scroll-to-panel-in-view

…rd/skip-scroll-to-panel-in-view

nreese · 2025-08-29T18:41:33Z

src/platform/plugins/shared/dashboard/public/dashboard_renderer/dashboard_renderer.tsx

        data-test-subj="dashboardContainer"
        css={styles.renderer}
-        ref={(e) => (dashboardContainerRef.current = e)}
+        ref={dashboardInternalApi.setDashboardContainerRef}


This will cause problems. dashboardInternalApi is loaded async and may not be available when ref is provided. To avoid race conditions, how about

ref={(e) => { if (dashboardInternalApi) { dashboardInternalApi.setDashboardContainerRef(dashboardInternalApi); return; } dashboardContainerRef.current = e; }}

Then when dashboardInternalApi is set, if dashboardContainerRef.current call setDashboardContainerRef

nreese · 2025-08-29T20:19:17Z

src/platform/plugins/shared/dashboard/public/dashboard_renderer/dashboard_renderer.tsx

+    if (dashboardInternalApi) {
+      dashboardInternalApi.setDashboardContainerRef(dashboardContainerRef.current);
+    }
+  }, [dashboardInternalApi, dashboardContainerRef]);


refs do not trigger re-renders so lets remove dashboardContainerRef from this useEffect dependency array.

dashboardContainerRef$ may already be this value if ref callback fires first. So how about checking equality before calling set.

Removed in c9ef5db

and added the equality check here ba933b9

Heenawter

FYI We no longer scroll to the bottom of the dashboard when adding a new section:

Screen.Recording.2025-09-02.at.12.23.00.PM.mov

…rd/skip-scroll-to-panel-in-view

Heenawter · 2025-09-11T20:08:20Z

@cqliu1 Oops! Looks like I had the following in my config/kibana.dev.yml and that was impacting the scroll behaviour 😬

feature_flags.overrides:
  core.chrome.layoutType: 'grid'

Ignore my above comment 🤦

Heenawter · 2025-09-11T20:16:57Z

src/platform/plugins/shared/dashboard/public/dashboard_renderer/dashboard_renderer.tsx

+          if (dashboardInternalApi) {
+            dashboardInternalApi.setDashboardContainerRef(e);
+          }


Why are we setting dashboardContainerRef twice? Do we need this if we have the useEffect above?

The useEffect is a back up to account for a race condition where dashboardInternalApi may not be initialized when the ref is set initially. This might be redundant though. I can remove this, and we can just rely on the useEffect

Why not just keep this and then listen for changes to dashboardContainerRef$ via usePublishingSubjects? Rather than declaring dashboardContainerRef locally - that feels confusing to me

From earlier feedback, @nreese wanted to avoid a race condition where the dashboardInternalApi potentially isn't available when setting the ref. #233226 (comment)

Let's just use the useEffect to set dashboardContainerRef (dependant on dashboardInternalApi) then - the double setting seems redundant to me

…rd/skip-scroll-to-panel-in-view

src/platform/plugins/shared/dashboard/public/dashboard_renderer/dashboard_renderer.tsx

Heenawter · 2025-09-15T17:40:24Z

src/platform/plugins/shared/dashboard/public/dashboard_renderer/dashboard_renderer.tsx

+  const dashboardContainerRef = useStateFromPublishingSubject(
+    dashboardInternalApi?.dashboardContainerRef$ || new BehaviorSubject(null)
+  );


sorry - I was wrong but didn't delete my original comment in time :) see the updated comment here: #233226 (comment)

This reverts commit 04360de.

nreese · 2025-09-15T19:48:00Z

src/platform/plugins/shared/dashboard/public/dashboard_renderer/dashboard_renderer.tsx

+            return;
+          }
+
+          // only store ref locally if dashboardInternalApi is not yet available


I think this is going to cause some problems when savedObjectId changes and useEffect that loads dashboard re-runs. Here is the scenario

load dashboard returns dashboardInternalApi and setDashboardApi is called.

dashboardInternalApi useEffect calls dashboardInternalApi.setDashboardContainerRef(undefined)

ref returns. dashboardInternalApi.setDashboardContainerRef(newRef) is called. dashboardContainer ref is not set.

saved object id changes, causing load dashboard to run again. Now when dashboardInternalApi is returned and setDashboardApi is called triggering dashboardInternalApi useEffect

dashboardContainer ref is undefined so dashboardInternalApi.setDashboardContainerRef is not called and dashboardInteralApi does not have dashboard container ref.

Regardless of whether dashboardInteralApi has the right ref, this component should always set dashboardContainerRef.current.

Applied in afa173d

elasticmachine · 2025-09-16T01:46:46Z

💚 Build Succeeded

Buildkite Build
Commit: afa173d

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`dashboard`	646.1KB	646.5KB	+388.0B

History

💔 Build #339534 failed af265fa
💚 Build #338980 succeeded d624a3a
💔 Build #334305 failed ba933b9
💚 Build #334280 succeeded c9ef5db
💚 Build #334171 succeeded 83e4b6c

Heenawter

LGTM - tested locally + code review. 💃 This makes the editing experience so much better! I'm excited to see it land 🎉

nreese

kibana-presentation changes LGTM. Thanks for taking the extra time to clean up dashboardContainerRef usage and consolidating this into a single location

Check panel visibility in current viewport before scrolling automatic…

9799846

…ally

cqliu1 requested a review from a team as a code owner August 27, 2025 22:39

cqliu1 added the v9.2.0 label Aug 27, 2025

nreese reviewed Aug 27, 2025

View reviewed changes

cqliu1 added 2 commits August 27, 2025 16:01

Remove console log

013211e

Add unit tests for track panel

38c09f6

cqliu1 added release_note:fix backport:skip This PR does not require backporting labels Aug 29, 2025

cqliu1 and others added 9 commits August 28, 2025 20:23

Merge branch 'main' of https://github.com/elastic/kibana into dashboa…

358596e

…rd/skip-scroll-to-panel-in-view

add tests

17b1d3d

[CI] Auto-commit changed files from 'node scripts/eslint_all_files --…

83e4b6c

…no-cache --fix'

Add dashboardContainerRef to dashboardInternalApi

6652373

Merge branch 'dashboard/skip-scroll-to-panel-in-view' of https://gith…

a264b09

…ub.com/cqliu1/kibana into dashboard/skip-scroll-to-panel-in-view

Merge branch 'main' of https://github.com/elastic/kibana into dashboa…

e47f403

…rd/skip-scroll-to-panel-in-view

fix bugs

770f034

Remove console logs

a14811f

Fix test

0e2e3ec

nreese reviewed Aug 29, 2025

View reviewed changes

Add useeffect to set dashboard container ref

401c793

nreese reviewed Aug 29, 2025

View reviewed changes

cqliu1 added 2 commits August 29, 2025 13:27

Remove containerDashboardRef dep from useEffect

c9ef5db

Check container ref equality before setting new container ref

ba933b9

Heenawter requested changes Sep 2, 2025

View reviewed changes

Merge branch 'main' of https://github.com/elastic/kibana into dashboa…

4547197

…rd/skip-scroll-to-panel-in-view

Merge branch 'main' of https://github.com/elastic/kibana into dashboa…

605d125

…rd/skip-scroll-to-panel-in-view

Heenawter reviewed Sep 11, 2025

View reviewed changes

cqliu1 added 3 commits September 12, 2025 12:06

Remove dashboardInternalApi call from dashboard container ref

d624a3a

Clean up

302d929

Merge branch 'main' of https://github.com/elastic/kibana into dashboa…

71735fc

…rd/skip-scroll-to-panel-in-view

Heenawter reviewed Sep 15, 2025

View reviewed changes

src/platform/plugins/shared/dashboard/public/dashboard_renderer/dashboard_renderer.tsx Outdated Show resolved Hide resolved

Apply feedback

04360de

Heenawter reviewed Sep 15, 2025

View reviewed changes

cqliu1 added 2 commits September 15, 2025 10:44

Revert "Apply feedback"

dcd0865

This reverts commit 04360de.

Remove dup useEffect

af265fa

nreese reviewed Sep 15, 2025

View reviewed changes

Always store dashboardContainerRef

afa173d

Heenawter approved these changes Sep 16, 2025

View reviewed changes

nreese approved these changes Sep 16, 2025

View reviewed changes

cqliu1 merged commit db30388 into elastic:main Sep 16, 2025
12 checks passed

CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Sep 24, 2025

[Dashboard] Skip automatic scroll when panel is visible (elastic#233226)

b3aea33

niros1 pushed a commit that referenced this pull request Sep 30, 2025

[Dashboard] Skip automatic scroll when panel is visible (#233226)

f82f915

rylnd pushed a commit to rylnd/kibana that referenced this pull request Oct 17, 2025

[Dashboard] Skip automatic scroll when panel is visible (elastic#233226)

4783418

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Dashboard] Skip automatic scroll when panel is visible#233226

[Dashboard] Skip automatic scroll when panel is visible#233226
cqliu1 merged 24 commits intoelastic:mainfrom
cqliu1:dashboard/skip-scroll-to-panel-in-view

cqliu1 commented Aug 27, 2025 •

edited

Loading

elasticmachine commented Aug 27, 2025

nreese Aug 27, 2025

cqliu1 Aug 29, 2025

nreese Aug 27, 2025

cqliu1 Aug 29, 2025

nreese Aug 29, 2025

cqliu1 Aug 29, 2025

nreese Aug 29, 2025

nreese Aug 29, 2025

cqliu1 Aug 29, 2025 •

edited

Loading

Heenawter left a comment

Heenawter commented Sep 11, 2025 •

edited

Loading

Heenawter Sep 11, 2025 •

edited

Loading

cqliu1 Sep 12, 2025

Heenawter Sep 12, 2025

cqliu1 Sep 15, 2025

Heenawter Sep 15, 2025 •

edited

Loading

Uh oh!

Heenawter Sep 15, 2025

nreese Sep 15, 2025 •

edited

Loading

cqliu1 Sep 16, 2025

elasticmachine commented Sep 16, 2025

Heenawter left a comment

nreese left a comment

Uh oh!

Labels

5 participants

Conversation

cqliu1 commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Before

After

Checklist

Identify risks

elasticmachine commented Aug 27, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cqliu1 Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Heenawter left a comment

Choose a reason for hiding this comment

Heenawter commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Heenawter Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Heenawter Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

nreese Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Sep 16, 2025

💚 Build Succeeded

Metrics [docs]

Async chunks

History

Heenawter left a comment

Choose a reason for hiding this comment

nreese left a comment

Choose a reason for hiding this comment

Uh oh!

Labels

5 participants

cqliu1 commented Aug 27, 2025 •

edited

Loading

cqliu1 Aug 29, 2025 •

edited

Loading

Heenawter commented Sep 11, 2025 •

edited

Loading

Heenawter Sep 11, 2025 •

edited

Loading

Heenawter Sep 15, 2025 •

edited

Loading

nreese Sep 15, 2025 •

edited

Loading