[ES|QL] LOOKUP JOIN - Keyboard accessibility improvements by sddonne · Pull Request #229907 · elastic/kibana

sddonne · 2025-07-30T07:13:24Z

Part of #207330

⚠️ Note for reviewers: This PR is targeting the feature
branch

Summary

Improves the grid accessibility by:

Allowing the user to modify cell values using arrows, enter and scape.
- The inputs are no longer displayed in the body of the cell, they are now shown as a custom popup.
- The dataGrid ref has been forwarded so it can be used for UnifiedDataTable, this ref exposes a method to programatically select the focused cell, it's used to keep the focus in the correct place after editing a cell.
- WithRestorableState HOC now forwards the ref of the component it's wrapping. (Needed as UnifiedDataTable is wrapped with it.)
Allow the user to modify columns using keyboard.
- Unfortunately dataGrid does not expose a way to set focus on column headers, and does not allow to display popups on them. So making them keyboard navegable has been more tricky. It requires to set a data property in the html element that wraps the header and to manually put focus on it when required.
- We still need a double enter to edit it.
Don't move the placeholder columns when filled.
- Previous implementation was pushing the placeholders at the end of the columns array, now they preserve its place, this is done by treating the placeholders as normal unsaved columns, the user now edits these columns, instead of adding a new one. This requires now to ignore the placeholders columns in some parts of the code.

Screen.Recording.2025-07-30.at.10.20.21.mov

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

Unit or functional tests were updated or added to match the most common scenarios

sddonne · 2025-07-30T07:30:03Z

...kages/private/kbn-index-editor/src/components/grid_custom_renderers/column_input_control.tsx

    ...column,
-    display: <AddColumnHeader initialColumnName={initialColumnName} />,
+    display: <AddColumnHeader initialColumnName={columnName} containerId={column.id} />,
+    displayHeaderCellProps: {


DataGrid exposes the method setFocusedCell for cells, but not way of focusing headers.
The way of achieving this is using a data prop and then select the html element manually.

sddonne · 2025-07-30T07:33:33Z

...ckages/private/kbn-index-editor/src/components/grid_custom_renderers/value_input_control.tsx


 export type OnCellValueChange = (docId: string, update: any) => void;

+export const getValueInputPopover =


The input for the cell is no longer in the cell body, now it's displayed as a popup with this method.

sddonne · 2025-07-30T08:16:33Z

src/platform/packages/private/kbn-index-editor/src/index_update_service.ts

-            }
-            return acc;
-          }, [])
+          take(1),


Now we set the placeholders as unsaved columns initial value.

sddonne · 2025-07-30T08:18:31Z

src/platform/packages/shared/kbn-restorable-state/src/restorable_state_provider.tsx

          onInitialStateChange={onInitialStateChange}
        >
-          <ComponentMemoized {...(props as TProps)} />
+          {/* @ts-ignore */}


Couldn't pass through this type check..

If you have no other options but using ts-ignore, we should also mentioned the suppressing error. But I wonder if you can make it work with generics?

solved: 25d95be

…com/sddonne/kibana into lookup-join/accesibility-new-branch

...kages/private/kbn-index-editor/src/components/grid_custom_renderers/column_input_control.tsx

...ckages/private/kbn-index-editor/src/components/grid_custom_renderers/value_input_control.tsx

darnautov · 2025-07-30T16:58:06Z

src/platform/packages/private/kbn-index-editor/src/components/value_inputs_factory.tsx


-export const StringInput: React.FC<ValueInputProps> = ({ onError, ...restOfProps }) => (
-  <EuiFieldText compressed {...restOfProps} />
+export const StringInput = React.forwardRef<HTMLInputElement, ValueInputProps>(


did you consider using React.forwardRef in getInputComponentForType?

Took the forwardRefs out, changed the implentation to use a focus trap as you suggested ❤️ 94c9364

src/platform/packages/shared/kbn-restorable-state/src/restorable_state_provider.tsx

darnautov · 2025-07-30T17:13:56Z

src/platform/packages/shared/kbn-restorable-state/src/restorable_state_provider.tsx

          onInitialStateChange={onInitialStateChange}
        >
-          <ComponentMemoized {...(props as TProps)} />
+          {/* @ts-ignore */}


If you have no other options but using ts-ignore, we should also mentioned the suppressing error. But I wonder if you can make it work with generics?

darnautov · 2025-07-30T17:19:14Z

I understand the intention to fix keyboard accessibility, but with that level of effort, I wonder if it might be better to address the issue on the EUI side instead?

jughosta

Data Discovery changes LGTM 👍
Nice idea on merging together the restorable state ref and the grid ref methods!

elasticmachine · 2025-07-31T13:32:56Z

💔 Build Failed

Buildkite Build
Commit: 5288912

Failed CI Steps

Post-Build

Metrics [docs]

‼️ ERROR: no builds found for mergeBase sha [4cde6bc]

History

💔 Build #324640 failed 3b11b51

darnautov

LGTM

Part of #207330 > ⚠️ Built on top of #229907, that needs to be merged first. ## Summary * Both for creating an index from scratch as well as editing the behaviour for adding a new row will be to push a new row in the grid in the first position. The focus will be changed to the first cell of the row, and if the user is in a different page than the first one, it will be directed to the first page. * For adding a new column, now the "Add field" button will push a new placeholder column into the grid (in the last position), if the new column is out of the viewport the grid will be auto scrolled until it's visible. * "Add row" button has been renamed as "Add document". * Added an illustration to the empty state. <img width="1726" height="874" alt="image" src="https://github.com/user-attachments/assets/0bbd3476-e4e2-4e16-9060-38f5955d6e0f" /> --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

Part of #207330 > ⚠️ Built on top of #229907, that needs to be merged first. ## Summary Now when making an edit in a cell or column, the change will be saved if clicking outside the cell. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

sddonne added 4 commits July 29, 2025 17:05

accesibility improvements

8b48962

add data table tests

94e9dc8

clean

b6bc021

refactor: avoid using an effect for calculating columns

0fc2fa1

sddonne commented Jul 30, 2025

View reviewed changes

sddonne requested a review from darnautov July 30, 2025 08:21

sddonne marked this pull request as ready for review July 30, 2025 08:24

sddonne requested review from a team as code owners July 30, 2025 08:24

kibanamachine and others added 4 commits July 30, 2025 08:36

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

ab0aca1

fix: not discarding unsaved columns

b3f0608

Merge branch 'lookup-join/accesibility-new-branch' of https://github.…

f0b151d

…com/sddonne/kibana into lookup-join/accesibility-new-branch

fix: placeholder columns after importing a file

3b11b51

sddonne mentioned this pull request Jul 30, 2025

[ES|QL] LOOKUP JOIN - Unify add row and add column behaviours #229954

Merged

darnautov reviewed Jul 30, 2025

View reviewed changes

PhilippeOberti approved these changes Jul 31, 2025

View reviewed changes

sddonne added 6 commits July 31, 2025 11:18

use focus trap to autofocus cell input

94c9364

don't focus column header after edition

256c997

new error text

a64a78b

cell value input improvements

576dce6

fix: type issues

25d95be

improve comment

5288912

jughosta approved these changes Jul 31, 2025

View reviewed changes

sddonne mentioned this pull request Jul 31, 2025

[ES|QL] LOOKUP JOIN - Allow to edit cells and columns on blur #230074

Merged

darnautov approved these changes Jul 31, 2025

View reviewed changes

sddonne merged commit f9014f1 into elastic:feature/lookup-join-ux Jul 31, 2025
11 of 12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ES|QL] LOOKUP JOIN - Keyboard accessibility improvements#229907

[ES|QL] LOOKUP JOIN - Keyboard accessibility improvements#229907
sddonne merged 14 commits intoelastic:feature/lookup-join-uxfrom
sddonne:lookup-join/accesibility-new-branch

sddonne commented Jul 30, 2025 •

edited

Loading

sddonne Jul 30, 2025

sddonne Jul 30, 2025

sddonne Jul 30, 2025

sddonne Jul 30, 2025

darnautov Jul 30, 2025

sddonne Jul 31, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

darnautov Jul 30, 2025

sddonne Jul 31, 2025

Uh oh!

Uh oh!

darnautov Jul 30, 2025

darnautov commented Jul 30, 2025

jughosta left a comment

elasticmachine commented Jul 31, 2025

darnautov left a comment

Uh oh!

Labels

6 participants


		export type OnCellValueChange = (docId: string, update: any) => void;

		export const getValueInputPopover =

Conversation

sddonne commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

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

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

darnautov commented Jul 30, 2025

jughosta left a comment

Choose a reason for hiding this comment

elasticmachine commented Jul 31, 2025

💔 Build Failed

Failed CI Steps

Metrics [docs]

History

darnautov left a comment

Choose a reason for hiding this comment

Uh oh!

Labels

6 participants

sddonne commented Jul 30, 2025 •

edited

Loading