[EuiDataGrid] Fix scrolling on focus return by tkajtoch · Pull Request #9453 · elastic/eui

tkajtoch · 2026-03-11T18:04:22Z

Summary

Fixes an issue in EuiDataGrid that caused the scroll position to jump back to the coordinates before starting to scroll. Originally reported by @agusruidiazgd in Slack.

Why are we making this change?

Because of a regression caused by #9276

Impact to users

No negative impact to users expected

QA

Remove or strikethrough items that do not apply to your PR.

General checklist

Browser QA
- Checked in Chrome, Safari, Edge, and Firefox
Docs site QA
- Added or updated jest and cypress tests
Release checklist
- A changelog entry exists and is marked appropriately

…ing the scrollbar

…urned ref

tkajtoch · 2026-03-16T09:09:21Z

packages/eui/src/components/datagrid/utils/scrolling.tsx

      });
    }
-  }, [focusedCell, isPointerDown, scrollCellIntoView]);
+  }, [focusedCell, scrollCellIntoView, isPointerDownRef]);


eslint complained that it really wants to have isPointerDownRef in the dependency array. It shouldn't impact anything, as this is a ref object that won't mutate itself.

…xport the undocumented and internal `useIsPointerDown`

weronikaolejniczak · 2026-03-16T11:34:49Z

packages/eui/src/services/hooks/index.ts

Good call 👍🏻

weronikaolejniczak

I compared staging Storybook with prod Storybook.

The reported issue of scroll position resets to the focused element after scrolling but there's regression that when we click on a cell, there's no scroll:

Kapture.2026-03-16.at.12.44.07.mp4

It's because of the isPointerDownRef.current in the early-return check. I checked with:

useEffect(() => {
  if (focusedCell) {
    if (window?.getSelection()?.type === 'Range') {
      return;
    }
    scrollCellIntoView({ rowIndex: focusedCell[1], colIndex: focusedCell[0] });
  }
}, [focusedCell, scrollCellIntoView]);

and it works as expected both for "clicking into a partially visible cell" and "scrolling data grid outside of the focused cell range":

Kapture.2026-03-16.at.13.03.23.mp4

tkajtoch · 2026-03-16T15:34:41Z

We're waiting for @acstll to discuss the current state. Turns out the fix to useIsPointerDown highlighted another issue that likely renders the whole usage of that hook unnecessary

acstll · 2026-03-16T16:14:37Z

We're waiting for @acstll to discuss the current state. Turns out the fix to useIsPointerDown highlighted another issue that likely renders the whole usage of that hook unnecessary

the hook was introduced because without it the fix was not good enough. You can see that my initial exploration (see 1770876) looks exactly like the snippet above…

I shared that first iteration in the issue (elastic/kibana#245368) and got some feedback:

I tried it out and it seems to partially work if I select fast enough, but a lot of the times I was too slow selecting and it still scrolled up. I know this is a tricky kind of interaction to handle reliably.

we then settled on that implementation of the "mouseup" idea with the useIsPointerDown hook.

I'm happy to remove the hook, but we'd need to replace it with something else if we want to keep the fix for the original elastic/kibana#245368 issue.

weronikaolejniczak · 2026-03-19T11:20:08Z

@tkajtoch I built on top of what you did because we do need to replace that useEffect with useRef as we've discussed. But I found a way to make this work for all 3 cases.

That useState like you pointed out triggers a re-render and re-runs the effect to scroll. Which is both causing the issue we're trying to fix (scroll snapping to focused el) AND is fixing the selection case not causing the scroll. The missing part here is some mechanism that actually executes the deferred scroll once the pointer is released - pointerup.

The gist is: when you click on a cell, the focus changes before you've lifted your finger. So we can't scroll yet because you might still be dragging to select the text. Instead, we flip the flag pendingScrollRef.current. When you lift your finger, we check the flag. If it's set, we then check whether you selected any text. If you didn't, we scroll. If you did, we don't scroll (otherwise the scroll will extend the selection). If you scroll the grid without clicking a cell, focus never changed, so pendingScrollRef.current is never set and pointerup does nothing.

Click to scroll into view	Scroll outside the focus view	Select the text in a partial cell

I checked in both axis. I didn't test in Kibana yet (worth doing in Discover and in this Windows + Firefox case before merging).

It would truly suck if this broke something else, so I'd highly appreciate it if you could review and test @acstll 🙏🏻 Let's aim to have a fix merged before EOW. If not what I wrote in 2a5065e then at least the revert.

elasticmachine · 2026-03-19T11:27:42Z

💚 Build Succeeded

History

💚 Build #2980 succeeded b6aab30
💔 Build #2977 failed 4f78db1
💚 Build #2974 succeeded 410daf6
💚 Build #2973 succeeded a1a7d6a
💚 Build #2942 succeeded 9bdcc8e

cc @tkajtoch

elasticmachine · 2026-03-19T11:48:51Z

💚 Build Succeeded

Buildkite Build
Commit: 2a5065e

History

💚 Build #6054 succeeded b6aab30
💚 Build #6048 succeeded 410daf6
💔 Build #6022 failed 9bdcc8e

cc @tkajtoch

acstll

🟢 I tested extensively, all 3 cases described in your comment here. Also tested in Kibana: was able to reproduce the bug, and then confirm the changes in this PR fixes it. (Also tested in Windows + Firefox, though i understand that's not really needed at this point.)

It would truly suck if this broke something else, so I'd highly appreciate it if you could review and test

looking at the original code, which had a simple if (focusedCell) check, it now looks a tad more complex, but still nothing in the current implementation would raise a concern for me in this regard…

Thanks for taking and tackling this one @weronikaolejniczak 💪

weronikaolejniczak · 2026-03-20T10:35:02Z

Thank you so much, @acstll 🙏🏻 💚

Also tested in Windows + Firefox, though i understand that's not really needed at this point.

Fortunately, not needed - that issue seems to be a completely separate one, it's related to EuiInMemoryTable (and likely something on the consumer side) and not EuiDataGrid.

looking at the original code, which had a simple if (focusedCell) check, it now looks a tad more complex

I agree, it's more complex and not to my liking. Our data grid component is huge, unoptimized, a ticking time bomb like we all know. But this is the way to make all 3 cases work at once 😄

## Dependency updates - `@elastic/eui` - v113.3.0 ⏩ v114.0.0 - `@elastic/eui-theme-borealis` - v6.2.0 ⏩ v7.0.0 - `@elastic/eslint-plugin-eui` - v2.10.0 ⏩ v2.11.0 --- ## Package updates ### [`v114.0.0`](https://github.com/elastic/eui/releases/v114.0.0) - Fixed the clipping of `EuiFlyout` overlay mask to the container bounds when the `container` prop is provided, so the mask no longer covers the full viewport for app-scoped flyouts. ([#9512](elastic/eui#9512)) - Updated `EuiFlyout` to support `pushAnimation` prop for `type="overlay"` ([#9428](elastic/eui#9428)) - Added `hasAnimation` prop on `EuiFlyout` (replaces `pushAnimation`) ([#9428](elastic/eui#9428)) - Added `hasAnimation` prop on `EuiOverlayMask` to conditionally add animation styles ([#9428](elastic/eui#9428)) - Added `historyKey` prop (type `symbol`) to `EuiFlyout` and the flyout manager API to support scoped flyout history. ([#9413](elastic/eui#9413)) - Only flyouts sharing the same `Symbol` reference share Back button navigation and history entries; omitting `historyKey` gives each session its own isolated history group. - `ACTION_CLOSE_ALL` now closes only the current history group rather than all open flyouts. **Bug fixes** - Fixed `EuiTreeView` expanded nodes clipping content and causing sibling overlap when children exceed viewport height ([#9510](elastic/eui#9510)) - Fixed `EuiDataGrid` scroll bouncing back to the focused element in certain cases ([#9453](elastic/eui#9453)) - Fixed support for intraword underscores in `EuiMarkdownFormat` ([#9408](elastic/eui#9408)) **Deprecations** - Deprecated `pushAnimation` prop on `EuiFlyout`. Use `hasAnimation` instead. ([#9428](elastic/eui#9428)) **Breaking changes** - Removed `severity.assistance` color token ([#9507](elastic/eui#9507)) - Removed assistance datavis color tokens: ([#9507](elastic/eui#9507)) - `vis.euiColorVisAssistance` - `vis.euiColorVis10` - `vis.euiColorVis11` - `vis.euiColorVisText10` - `vis.euiColorVisText11` - `vis.euiColorVisBehindText10` - `vis.euiColorVisBehindText11` - The positional signature of `FlyoutManagerApi.addFlyout` and the flyout store's `addFlyout` now includes `historyKey` before the `iconType`/`minWidth` arguments. Call sites that pass arguments positionally must be updated (or switched to named parameters) to account for this new parameter ordering. ([#9413](elastic/eui#9413)) **Accessibility** - Fixed `aria-label` not being applied to `EuiColorPicker`'s input element ([#9436](elastic/eui#9436)) ### @elastic/eui-theme-borealis v7.0.0 **Breaking changes** - Removed `severity.assistance` color token ([#9507](elastic/eui#9507)) - Removed assistance datavis color tokens: ([#9507](elastic/eui#9507)) - `vis.euiColorVisAssistance` - `vis.euiColorVis10` - `vis.euiColorVis11` - `vis.euiColorVisText10` - `vis.euiColorVisText11` - `vis.euiColorVisBehindText10` - `vis.euiColorVisBehindText11` ### @elastic/eslint-plugin-eui v2.11.0 - Updated `no-unnamed-interactive-element` to include checking `EuiColorPicker` ([#9436](elastic/eui#9436)) --------- Co-authored-by: Timothy Sullivan <tsullivan@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Weronika Olejniczak <32842468+weronikaolejniczak@users.noreply.github.com> Co-authored-by: Weronika Olejniczak <weronika.olejniczak@elastic.co>

## Dependency updates - `@elastic/eui` - v113.3.0 ⏩ v114.0.0 - `@elastic/eui-theme-borealis` - v6.2.0 ⏩ v7.0.0 - `@elastic/eslint-plugin-eui` - v2.10.0 ⏩ v2.11.0 --- ## Package updates ### [`v114.0.0`](https://github.com/elastic/eui/releases/v114.0.0) - Fixed the clipping of `EuiFlyout` overlay mask to the container bounds when the `container` prop is provided, so the mask no longer covers the full viewport for app-scoped flyouts. ([elastic#9512](elastic/eui#9512)) - Updated `EuiFlyout` to support `pushAnimation` prop for `type="overlay"` ([elastic#9428](elastic/eui#9428)) - Added `hasAnimation` prop on `EuiFlyout` (replaces `pushAnimation`) ([elastic#9428](elastic/eui#9428)) - Added `hasAnimation` prop on `EuiOverlayMask` to conditionally add animation styles ([elastic#9428](elastic/eui#9428)) - Added `historyKey` prop (type `symbol`) to `EuiFlyout` and the flyout manager API to support scoped flyout history. ([elastic#9413](elastic/eui#9413)) - Only flyouts sharing the same `Symbol` reference share Back button navigation and history entries; omitting `historyKey` gives each session its own isolated history group. - `ACTION_CLOSE_ALL` now closes only the current history group rather than all open flyouts. **Bug fixes** - Fixed `EuiTreeView` expanded nodes clipping content and causing sibling overlap when children exceed viewport height ([elastic#9510](elastic/eui#9510)) - Fixed `EuiDataGrid` scroll bouncing back to the focused element in certain cases ([elastic#9453](elastic/eui#9453)) - Fixed support for intraword underscores in `EuiMarkdownFormat` ([elastic#9408](elastic/eui#9408)) **Deprecations** - Deprecated `pushAnimation` prop on `EuiFlyout`. Use `hasAnimation` instead. ([elastic#9428](elastic/eui#9428)) **Breaking changes** - Removed `severity.assistance` color token ([elastic#9507](elastic/eui#9507)) - Removed assistance datavis color tokens: ([elastic#9507](elastic/eui#9507)) - `vis.euiColorVisAssistance` - `vis.euiColorVis10` - `vis.euiColorVis11` - `vis.euiColorVisText10` - `vis.euiColorVisText11` - `vis.euiColorVisBehindText10` - `vis.euiColorVisBehindText11` - The positional signature of `FlyoutManagerApi.addFlyout` and the flyout store's `addFlyout` now includes `historyKey` before the `iconType`/`minWidth` arguments. Call sites that pass arguments positionally must be updated (or switched to named parameters) to account for this new parameter ordering. ([elastic#9413](elastic/eui#9413)) **Accessibility** - Fixed `aria-label` not being applied to `EuiColorPicker`'s input element ([elastic#9436](elastic/eui#9436)) ### @elastic/eui-theme-borealis v7.0.0 **Breaking changes** - Removed `severity.assistance` color token ([elastic#9507](elastic/eui#9507)) - Removed assistance datavis color tokens: ([elastic#9507](elastic/eui#9507)) - `vis.euiColorVisAssistance` - `vis.euiColorVis10` - `vis.euiColorVis11` - `vis.euiColorVisText10` - `vis.euiColorVisText11` - `vis.euiColorVisBehindText10` - `vis.euiColorVisBehindText11` ### @elastic/eslint-plugin-eui v2.11.0 - Updated `no-unnamed-interactive-element` to include checking `EuiColorPicker` ([elastic#9436](elastic/eui#9436)) --------- Co-authored-by: Timothy Sullivan <tsullivan@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Weronika Olejniczak <32842468+weronikaolejniczak@users.noreply.github.com> Co-authored-by: Weronika Olejniczak <weronika.olejniczak@elastic.co>

Co-authored-by: Weronika Olejniczak <weronika.olejniczak@elastic.co>

tkajtoch self-assigned this Mar 11, 2026

tkajtoch added the skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) label Mar 11, 2026

tkajtoch added 2 commits March 16, 2026 09:44

fix(EuiDataGrid): prevent returning to focused cell when scrolling us…

bc66e96

…ing the scrollbar

test(EuiDataGrid): update useIsPointerDown tests to work with the ret…

a1a7d6a

…urned ref

tkajtoch force-pushed the fix/datagrid-scrolling-on-focus-return branch from 9bdcc8e to a1a7d6a Compare March 16, 2026 09:03

tkajtoch marked this pull request as ready for review March 16, 2026 09:07

tkajtoch requested a review from a team as a code owner March 16, 2026 09:07

tkajtoch commented Mar 16, 2026

View reviewed changes

tkajtoch changed the title ~~Fix/datagrid scrolling on focus return~~ [EuiDataGrid] Fix scrolling on focus return Mar 16, 2026

tkajtoch removed the skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) label Mar 16, 2026

chore: add changelog

410daf6

weronikaolejniczak self-requested a review March 16, 2026 09:35

tkajtoch added 2 commits March 16, 2026 11:49

chore: get rid of barrel exports around src/services/hooks and un-e…

4f78db1

…xport the undocumented and internal `useIsPointerDown`

fix: fix useIsPointerDown import in EuiDataGrid scrolling util

b6aab30

weronikaolejniczak reviewed Mar 16, 2026

View reviewed changes

packages/eui/src/services/hooks/index.ts

Copy link
Copy Markdown

Contributor

weronikaolejniczak Mar 16, 2026

Choose a reason for hiding this comment

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

Good call 👍🏻

weronikaolejniczak requested changes Mar 16, 2026

View reviewed changes

weronikaolejniczak added 2 commits March 19, 2026 11:59

chore(eui): remove bang operators and initialize ref

a97cf21

fix(eui): add a ref and pointerup event listener

2a5065e

weronikaolejniczak requested review from acstll and weronikaolejniczak March 19, 2026 15:49

acstll approved these changes Mar 20, 2026

View reviewed changes

weronikaolejniczak approved these changes Mar 20, 2026

View reviewed changes

weronikaolejniczak merged commit 0e37bab into elastic:main Mar 20, 2026
5 checks passed

acstll mentioned this pull request Mar 25, 2026

Update EUI to v114.0.0 elastic/kibana#259497

Merged

weronikaolejniczak added a commit to weronikaolejniczak/eui that referenced this pull request Apr 7, 2026

[EuiDataGrid] Fix scrolling on focus return (elastic#9453)

c3fe82a

Co-authored-by: Weronika Olejniczak <weronika.olejniczak@elastic.co>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiDataGrid] Fix scrolling on focus return#9453

[EuiDataGrid] Fix scrolling on focus return#9453
weronikaolejniczak merged 7 commits intoelastic:mainfrom
tkajtoch:fix/datagrid-scrolling-on-focus-return

tkajtoch commented Mar 11, 2026 •

edited

Loading

Uh oh!

tkajtoch Mar 16, 2026

Uh oh!

weronikaolejniczak Mar 16, 2026

Uh oh!

weronikaolejniczak left a comment

Uh oh!

tkajtoch commented Mar 16, 2026

Uh oh!

acstll commented Mar 16, 2026 •

edited

Loading

Uh oh!

weronikaolejniczak commented Mar 19, 2026 •

edited

Loading

Uh oh!

elasticmachine commented Mar 19, 2026

Uh oh!

elasticmachine commented Mar 19, 2026

Uh oh!

acstll left a comment

Uh oh!

weronikaolejniczak commented Mar 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

tkajtoch commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why are we making this change?

Impact to users

QA

General checklist

Uh oh!

tkajtoch Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

weronikaolejniczak Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

weronikaolejniczak left a comment

Choose a reason for hiding this comment

Uh oh!

tkajtoch commented Mar 16, 2026

Uh oh!

acstll commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

weronikaolejniczak commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Mar 19, 2026

💚 Build Succeeded

History

Uh oh!

elasticmachine commented Mar 19, 2026

💚 Build Succeeded

History

Uh oh!

acstll left a comment

Choose a reason for hiding this comment

Uh oh!

weronikaolejniczak commented Mar 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tkajtoch commented Mar 11, 2026 •

edited

Loading

acstll commented Mar 16, 2026 •

edited

Loading

weronikaolejniczak commented Mar 19, 2026 •

edited

Loading