Skip to content

Conversation

@mgadewoll
Copy link
Contributor

@mgadewoll mgadewoll commented Nov 12, 2025

Summary

Related to elastic/kibana#242481

This PR is a follow-up to #9030 which ensured that columns are sorted according to visibleColumns. While this generally works fine, it does not account for the edge case that columns could in theory have duplicate columns that have the same id. In that case, the added functionality to sort the columns would run into a out-of-bounds value and pass undefined as column (code).

While it's generally not valid or expected to pass duplicated columns, this is not prevented and could potentially happen, as it recently did in Kibana here. The underlying issue is that Kibana passed duplicate columns, which they fixed in elastic/kibana#242557.

To make our functionality a bit more robust for this edge case, this PR updates the column_selector to dedupe duplicate columns and prevent app crashes if consumers pass unexpected duplicated columns.

Why are we making this change?

💪 Component robustness: Provide a safety net for misuse to prevent crashes.

Screenshots #

Passing duplicate columns

before after
Screenshot 2025-11-12 at 16 07 01 Screenshot 2025-11-12 at 16 07 12

Impact to users

🟢 There are no updates required on consumer side.

ℹ️ The changes have been tested against Kibana CI here.

QA

🧪 Testing story

  • verify that changing the visibleColumns order and amount correctly renders columns
  • verify that updating column order (via dragging, via column selector and via action menu "move left/right") work as expected and have no regression
  • verify that adding duplicate items in columns renders correctly and without error (visible columns are rendered only, duplicate columns are removed completely and are also not included in the column selector)

General checklist

  • Browser QA
    • Checked in both light and dark modes
    • Checked in both MacOS and Windows high contrast modes
    • Checked in mobile
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes
  • Docs site QA
  • Code quality checklist
  • Release checklist
    • A changelog entry exists and is marked appropriately
    • If applicable, added the breaking change issue label (and filled out the breaking change checklist)
    • If the changes unblock an issue in a different repo, smoke tested carefully (see Testing EUI features in Kibana ahead of time)
  • Designer checklist
    • If applicable, file an issue to update EUI's Figma library with any corresponding UI changes. (This is an internal repo, if you are external to Elastic, ask a maintainer to submit this request)

- columns with duplicate id are not expected so it happening is an edge case. but  previously when duplicate ids were  passed it would lead to undefined values returned as column content which could crash applications. While consumers should not pass duplicate ids, we can also filter them out as fallback.
- visually confirms correct column render
@mgadewoll mgadewoll self-assigned this Nov 12, 2025
@mgadewoll mgadewoll marked this pull request as ready for review November 12, 2025 16:27
@mgadewoll mgadewoll requested a review from a team as a code owner November 12, 2025 16:27
Copy link
Contributor

@weronikaolejniczak weronikaolejniczak left a comment

Choose a reason for hiding this comment

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

I tested the visibleColumns and it works as expected 👍🏻 I have one doubt that I think could potentially cause confusion for the consumers so it's worth tackling, that being said everything else is fine so I'll leave an approve. 🟢

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @mgadewoll

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @mgadewoll

@mgadewoll mgadewoll merged commit a4b3e60 into elastic:main Nov 14, 2025
7 checks passed
acstll added a commit to elastic/kibana that referenced this pull request Nov 20, 2025
- `@elastic/eui`: `v109.0.0` ⏩ `v109.1.0`
- `@elastic/eslint-plugin-eui`: `v2.5.0` ⏩ `v2.6.0`

---

## Changes

- Updated i18n EUI mapping 6cc95b0
- Updated test in Unified Search
668948f

## Package updates

### `@elastic/eui`
[`v109.1.0`](https://github.com/elastic/eui/releases/v109.1.0)

- Added `--euiBottomBarOffset` CSS variable to `EuiBottomBar` for
positioning other fixed elements relative to the bottom bar's height
([#9211](elastic/eui#9211))
- Updated `boxesVertical` icon and restored `checkInCircleFilled`,
`errorFilled`, and `warningFilled` icons.
([#9194](elastic/eui#9194))
- Updated `EuiSuperDatePicker` with new time zone information, opt-in
via `timeZoneDisplayProps`.
([#9191](elastic/eui#9191))
- Updated the position of `EuiModal` by removing bottom padding in
`EuiOverlayMask` ([#9190](elastic/eui#9190))
- Added `EuiPopover` and `EuiToolTip`'s `repositionOnScroll` to
`componentDefaults` ([#9152](elastic/eui#9152))
- Updated `EuiSuperDatePicker` with new time window buttons for time
shifting and zoom out, opt-in via `showTimeWindowButtons` boolean prop.
([#9151](elastic/eui#9151))
- Added beta prop `hasAriaDisabled` to all base button components:
`EuiButton`, `EuiButtonEmpty`, `EuiButtonIcon`, `EuibuttonGroup`,
`EuiFilterButton` ([#9201](elastic/eui#9201))
- Added `euiDisabledSelector` variable that combines CSS selectors
`:disabled` and `[aria-disabled="true"]`
([#9201](elastic/eui#9201))
- Added custom test matchers that check for both `disabled` and
`aria-disabled` attributes:
([#9201](elastic/eui#9201))
  - React testing Library: `.toBeEuiDisabled()`
  - Enzyme: `.toHaveEuiDisabledProp()`
  - Cypress: `should('be.euiDisabled)`

**Bug fixes**

- Fixed unexpected duplicate columns in `EuiDataGrid` crashing the
column sorting by removing duplicate columns entirely
([#9209](elastic/eui#9209))
- Fixed a visual bug in `EuiTable` where long table row content would be
cut off on mobile screens
([#9206](elastic/eui#9206))
- Fixed virtualized `EuiCodeBlock` rendering blank lines when content
updates if scrolled. ([#9196](elastic/eui#9196))
- Fixed `EuiButtonGroup` button sizing to ensure square buttons when
used with `isIconOnly=true`
([#9170](elastic/eui#9170))

**Accessibility**

- Fixed an issue where portalled components like `EuiPopover` were not
included in `EuiFlyout`'s focus trap through
`includeSelectorInFocusTrap`, making them inaccessible to keyboard users
([#9103](elastic/eui#9103))

### `@elastic/eslint-plugin-eui`
[`v2.6.0`](https://github.com/elastic/eui/blob/main/packages/eslint-plugin/changelogs/CHANGELOG_2025.md#v260)

- Added new `require-table-caption` rule.
([#9168](elastic/eui#9168))

---------

Co-authored-by: Elastic Machine <[email protected]>
andrimal pushed a commit to andrimal/kibana that referenced this pull request Nov 20, 2025
- `@elastic/eui`: `v109.0.0` ⏩ `v109.1.0`
- `@elastic/eslint-plugin-eui`: `v2.5.0` ⏩ `v2.6.0`

---

## Changes

- Updated i18n EUI mapping 6cc95b0
- Updated test in Unified Search
668948f

## Package updates

### `@elastic/eui`
[`v109.1.0`](https://github.com/elastic/eui/releases/v109.1.0)

- Added `--euiBottomBarOffset` CSS variable to `EuiBottomBar` for
positioning other fixed elements relative to the bottom bar's height
([elastic#9211](elastic/eui#9211))
- Updated `boxesVertical` icon and restored `checkInCircleFilled`,
`errorFilled`, and `warningFilled` icons.
([elastic#9194](elastic/eui#9194))
- Updated `EuiSuperDatePicker` with new time zone information, opt-in
via `timeZoneDisplayProps`.
([elastic#9191](elastic/eui#9191))
- Updated the position of `EuiModal` by removing bottom padding in
`EuiOverlayMask` ([elastic#9190](elastic/eui#9190))
- Added `EuiPopover` and `EuiToolTip`'s `repositionOnScroll` to
`componentDefaults` ([elastic#9152](elastic/eui#9152))
- Updated `EuiSuperDatePicker` with new time window buttons for time
shifting and zoom out, opt-in via `showTimeWindowButtons` boolean prop.
([elastic#9151](elastic/eui#9151))
- Added beta prop `hasAriaDisabled` to all base button components:
`EuiButton`, `EuiButtonEmpty`, `EuiButtonIcon`, `EuibuttonGroup`,
`EuiFilterButton` ([elastic#9201](elastic/eui#9201))
- Added `euiDisabledSelector` variable that combines CSS selectors
`:disabled` and `[aria-disabled="true"]`
([elastic#9201](elastic/eui#9201))
- Added custom test matchers that check for both `disabled` and
`aria-disabled` attributes:
([elastic#9201](elastic/eui#9201))
  - React testing Library: `.toBeEuiDisabled()`
  - Enzyme: `.toHaveEuiDisabledProp()`
  - Cypress: `should('be.euiDisabled)`

**Bug fixes**

- Fixed unexpected duplicate columns in `EuiDataGrid` crashing the
column sorting by removing duplicate columns entirely
([elastic#9209](elastic/eui#9209))
- Fixed a visual bug in `EuiTable` where long table row content would be
cut off on mobile screens
([elastic#9206](elastic/eui#9206))
- Fixed virtualized `EuiCodeBlock` rendering blank lines when content
updates if scrolled. ([elastic#9196](elastic/eui#9196))
- Fixed `EuiButtonGroup` button sizing to ensure square buttons when
used with `isIconOnly=true`
([elastic#9170](elastic/eui#9170))

**Accessibility**

- Fixed an issue where portalled components like `EuiPopover` were not
included in `EuiFlyout`'s focus trap through
`includeSelectorInFocusTrap`, making them inaccessible to keyboard users
([elastic#9103](elastic/eui#9103))

### `@elastic/eslint-plugin-eui`
[`v2.6.0`](https://github.com/elastic/eui/blob/main/packages/eslint-plugin/changelogs/CHANGELOG_2025.md#v260)

- Added new `require-table-caption` rule.
([elastic#9168](elastic/eui#9168))

---------

Co-authored-by: Elastic Machine <[email protected]>
eokoneyo pushed a commit to eokoneyo/kibana that referenced this pull request Dec 2, 2025
- `@elastic/eui`: `v109.0.0` ⏩ `v109.1.0`
- `@elastic/eslint-plugin-eui`: `v2.5.0` ⏩ `v2.6.0`

---

## Changes

- Updated i18n EUI mapping 6cc95b0
- Updated test in Unified Search
668948f

## Package updates

### `@elastic/eui`
[`v109.1.0`](https://github.com/elastic/eui/releases/v109.1.0)

- Added `--euiBottomBarOffset` CSS variable to `EuiBottomBar` for
positioning other fixed elements relative to the bottom bar's height
([elastic#9211](elastic/eui#9211))
- Updated `boxesVertical` icon and restored `checkInCircleFilled`,
`errorFilled`, and `warningFilled` icons.
([elastic#9194](elastic/eui#9194))
- Updated `EuiSuperDatePicker` with new time zone information, opt-in
via `timeZoneDisplayProps`.
([elastic#9191](elastic/eui#9191))
- Updated the position of `EuiModal` by removing bottom padding in
`EuiOverlayMask` ([elastic#9190](elastic/eui#9190))
- Added `EuiPopover` and `EuiToolTip`'s `repositionOnScroll` to
`componentDefaults` ([elastic#9152](elastic/eui#9152))
- Updated `EuiSuperDatePicker` with new time window buttons for time
shifting and zoom out, opt-in via `showTimeWindowButtons` boolean prop.
([elastic#9151](elastic/eui#9151))
- Added beta prop `hasAriaDisabled` to all base button components:
`EuiButton`, `EuiButtonEmpty`, `EuiButtonIcon`, `EuibuttonGroup`,
`EuiFilterButton` ([elastic#9201](elastic/eui#9201))
- Added `euiDisabledSelector` variable that combines CSS selectors
`:disabled` and `[aria-disabled="true"]`
([elastic#9201](elastic/eui#9201))
- Added custom test matchers that check for both `disabled` and
`aria-disabled` attributes:
([elastic#9201](elastic/eui#9201))
  - React testing Library: `.toBeEuiDisabled()`
  - Enzyme: `.toHaveEuiDisabledProp()`
  - Cypress: `should('be.euiDisabled)`

**Bug fixes**

- Fixed unexpected duplicate columns in `EuiDataGrid` crashing the
column sorting by removing duplicate columns entirely
([elastic#9209](elastic/eui#9209))
- Fixed a visual bug in `EuiTable` where long table row content would be
cut off on mobile screens
([elastic#9206](elastic/eui#9206))
- Fixed virtualized `EuiCodeBlock` rendering blank lines when content
updates if scrolled. ([elastic#9196](elastic/eui#9196))
- Fixed `EuiButtonGroup` button sizing to ensure square buttons when
used with `isIconOnly=true`
([elastic#9170](elastic/eui#9170))

**Accessibility**

- Fixed an issue where portalled components like `EuiPopover` were not
included in `EuiFlyout`'s focus trap through
`includeSelectorInFocusTrap`, making them inaccessible to keyboard users
([elastic#9103](elastic/eui#9103))

### `@elastic/eslint-plugin-eui`
[`v2.6.0`](https://github.com/elastic/eui/blob/main/packages/eslint-plugin/changelogs/CHANGELOG_2025.md#v260)

- Added new `require-table-caption` rule.
([elastic#9168](elastic/eui#9168))

---------

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants