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

[EuiSelectable] Fix non-virtualized lists causing Jest errors #7618

Merged
merged 2 commits into from
Mar 25, 2024

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented Mar 25, 2024

Summary

This is happening on activeOptionIndex change only. jsdom doesn't support or stub scrollIntoView (and we don't need it to be called in Jest tests) so adding a conditional check for it fixes several Kibana Jest failures:

QA

N/A, tests only

General checklist

  • Browser QA - N/A
  • Docs site QA - N/A
  • Code quality checklist
    • [ ] Added or updated jest and cypress tests Skipping this to speed up the patch/upgrade process, unless someone thinks it's really important to do
  • 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)
  • Designer checklist - N/A

@cee-chen cee-chen marked this pull request as ready for review March 25, 2024 19:06
@cee-chen cee-chen requested a review from a team as a code owner March 25, 2024 19:06
Copy link
Contributor

@mgadewoll mgadewoll left a comment

Choose a reason for hiding this comment

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

🚢 🐈‍⬛ LGTM! I guess if we ever do needed the stub we can still add it, but the conditional chaining seems a good solution for now 👍

@cee-chen
Copy link
Member Author

Thanks Lene!

@cee-chen cee-chen enabled auto-merge (squash) March 25, 2024 19:24
@elasticmachine
Copy link
Collaborator

elasticmachine commented Mar 25, 2024

💚 Build Succeeded

History

@kibanamachine
Copy link

Preview staging links for this PR:

@cee-chen cee-chen merged commit ac3396e into elastic:main Mar 25, 2024
8 checks passed
@cee-chen cee-chen deleted the selectable-patch branch March 25, 2024 19:57
cee-chen added a commit to elastic/kibana that referenced this pull request Mar 26, 2024
`v93.4.0` ⏩ `v93.5.1`

---

## [`v93.5.1`](https://github.com/elastic/eui/releases/v93.5.1)

**Bug fixes**

- Fixed unvirtualized `EuiSelectable`s to not cause Jest/jsdom errors on
active option change ([#7618](elastic/eui#7618))

## [`v93.5.0`](https://github.com/elastic/eui/releases/v93.5.0)

- `EuiHeaderLinks` now accepts a `children` render function that will be
passed a `closeMobilePopover` callback, allowing consumers to close the
mobile popover by its content
([#7603](elastic/eui#7603))
- Updated `EuiSelectable` to support scrolling list containers when
`listProps.isVirtualization` is set to `false`
([#7609](elastic/eui#7609))

**Bug fixes**

- Fixed `EuiIconTip`'s default `aria-label` text to be i18n tokenizable
([#7606](elastic/eui#7606))
- Fixed `EuiTextArea`'s CSS box model to no longer render a few extra
pixels of strut height
([#7607](elastic/eui#7607))

**Dependency updates**

- Updated `@types/refractor` to v3.4.0
([#7590](elastic/eui#7590))
- Updated `@types/lodash` to v4.14.202
([#7591](elastic/eui#7591))
- Removed `@types/resize-observer-browser` dependency. `ResizeObserver`
types should already be baked in to Typescript as of 4.2+
([#7592](elastic/eui#7592))
- Updated `classnames` to v2.5.1
([#7593](elastic/eui#7593))
- Updated `@types/numeral` to v2.0.5
([#7594](elastic/eui#7594))
- Updated `@types/react-window` to 1.8.8
([#7597](elastic/eui#7597))
- Updated `prop-types` to v15.18.1
([#7602](elastic/eui#7602))
- Removed `prop-types` as a peer dependency, per package recommendation
([#7602](elastic/eui#7602))

**Accessibility**

- `EuiIcons` no longer apply `aria-hidden` to empty icons, as long as a
valid title or label is provided to the icon. In particular, this is
intended to improve the accessibility of loading `EuiIconTip`s.
([#7606](elastic/eui#7606))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants