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

[EuiTabbedContent] Fix error when updated with a completely new tabs prop #7713

Merged
merged 6 commits into from
Apr 29, 2024

Conversation

shyamal2411
Copy link
Contributor

@shyamal2411 shyamal2411 commented Apr 25, 2024

Summary

closes #7350. Adds a simple workaround/fallback to prevent the entire component from throwing an error if tabs updates and a matching selected tab ID can't be found.

Also updates and improves component tests

General checklist

  • Browser QA - N/A, JS logic bugfix
  • Docs site QA - N/A, bugfix
  • Code quality checklist
  • Changelog

@shyamal2411 shyamal2411 requested a review from a team as a code owner April 25, 2024 18:29
Copy link

cla-checker-service bot commented Apr 25, 2024

💚 CLA has been signed

Copy link

👋 Since this is a community submitted pull request, a Buildkite build has not been started automatically. Would an Elastic organization member please verify the contents of this pull request and kick off a build manually?

@cee-chen
Copy link
Member

buildkite test this

@cee-chen
Copy link
Member

cee-chen commented Apr 25, 2024

@shyamal2411 Thank you for the PR! 🎉 As a heads up, you'll need to sign the contributor agreement (link) with the email address associated with your git commits! You can git log to check what that email address is.

shyamal2411 and others added 4 commits April 29, 2024 13:58
- remove Enzyme usage completely, reorder imports

- switch from snapshot assertions to actual assertions
- a basic fallback to `tabs[0]` suffices for this edge case and doesn't require extra logic/processing

- add a unit test to confirm the expected/desired behavior
@cee-chen cee-chen changed the title [EuiTabbedContent] Fixed issue of TypeError #7350 [EuiTabbedContent] Fix error when updated with a completely new tabs prop Apr 29, 2024
Copy link
Member

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @shyamal2411! I've pushed up a changelog, test improvements, and simplified the proposed getDerivedStateFromProps solution to a || tabs[0] fallback since the bug being solved is an edge case and doesn't need to add more logic overhead.

@cee-chen cee-chen enabled auto-merge (squash) April 29, 2024 21:06
- typescript already infers this for us
@cee-chen
Copy link
Member

buildkite test this

@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

@cee-chen cee-chen merged commit 22ca8c6 into elastic:main Apr 29, 2024
7 checks passed
cee-chen added a commit to elastic/kibana that referenced this pull request May 3, 2024
`v94.1.0-backport.0` ⏩ `v94.2.1-backport.0`

_[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_

---

##
[`v94.2.1-backport.0`](https://github.com/elastic/eui/releases/v94.2.1-backport.0)

**This is a backport release only intended for use by Kibana.**

- Reverted the `EuiFlexGroup`/`EuiFlexItem` `component` prop feature due
to Kibana typing issues

## [`v94.2.1`](https://github.com/elastic/eui/releases/v94.2.1)

**Bug fixes**

- Fixed an `EuiTabbedContent` edge case bug that occurred when updated
with a completely different set of `tabs`
([#7713](elastic/eui#7713))
- Fixed the `@storybook/test` dependency to be listed in
`devDependencies` and not `dependencies`
([#7719](elastic/eui#7719))

## [`v94.2.0`](https://github.com/elastic/eui/releases/v94.2.0)

- Updated `getDefaultEuiMarkdownPlugins()` to allow excluding the
following plugins in addition to `tooltip`:
([#7676](elastic/eui#7676))
  - `checkbox`
  - `linkValidator`
  - `lineBreaks`
  - `emoji`
- Updated `EuiSelectable`'s `isPreFiltered` prop to allow passing a
configuration object, which allows disabling search highlighting in
addition to search filtering
([#7683](elastic/eui#7683))
- Updated `EuiFlexGroup` and `EuiFlexItem` prop types to support passing
any valid React component type to the `component` prop and ensure proper
type checking of the extra props forwarded to the `component`.
([#7688](elastic/eui#7688))
- Updated `EuiSearchBar` to allow the `@` special character in query
string searches ([#7702](elastic/eui#7702))
- Added a new, optional `optionMatcher` prop to `EuiSelectable` and
`EuiComboBox` allowing passing a custom option matcher function to these
components and controlling option filtering for given search string
([#7709](elastic/eui#7709))

**Bug fixes**

- Fixed an `EuiPageTemplate` bug where prop updates would not cascade
down to child sections
([#7648](elastic/eui#7648))
- To cascade props down to the sidebar, `EuiPageTemplate` now explicitly
requires using the `EuiPageTemplate.Sidebar` rather than
`EuiPageSidebar`
- Fixed `EuiFieldNumber`'s typing to accept an icon configuration shape
([#7666](elastic/eui#7666))
- Fixed `EuiFieldText` and `EuiFieldNumber` to render the correct
paddings for icon shapes set to `side: 'right'`
([#7666](elastic/eui#7666))
- Fixed `EuiFieldText` and `EuiFieldNumber` to fully ignore
`icon`/`prepend`/`append` when `controlOnly` is set to true
([#7666](elastic/eui#7666))
- Fixed `EuiColorPicker`'s input not setting the correct right padding
for the number of icons displayed
([#7666](elastic/eui#7666))
- Visual fixes for `EuiRange`s with `showInput`:
([#7678](elastic/eui#7678))
  - Longer `append`/`prepend` labels no longer cause a background bug
  - Inputs can no longer overwhelm the actual range in width
- Fixed a visual text alignment regression in `EuiTableRowCell`s with
the `row` header scope
([#7681](elastic/eui#7681))
- Fixed `toolTipProps` type on `EuiSuperUpdateButton` to use
`Partial<EuiToolTipProps>`
([#7692](elastic/eui#7692))
- Fixes missing prop type for `popperProps` on `EuiDatePicker`
([#7694](elastic/eui#7694))
- Fixed a focus bug with `EuiDataGrid`s with `leadingControlColumns`
when moving columns to the left/right
([#7701](elastic/eui#7701))
([#7698](elastic/eui#7698))
- Fixed `EuiSuperDatePicker` to validate date string with respect of
locale on `EuiAbsoluteTab`.
([#7705](elastic/eui#7705))
- Fixed a visual bug with `EuiSuperDatePicker`'s absolute tab on small
mobile screens ([#7708](elastic/eui#7708))
- Fixed i18n of empty and loading state messages for the
`FieldValueSelectionFilter` component
([#7718](elastic/eui#7718))

**Dependency updates**

- Updated `@hello-pangea/dnd` to v16.6.0
([#7599](elastic/eui#7599))
- Updated `remark-rehype` to v8.1.0
([#7601](elastic/eui#7601))

**Accessibility**

- Improved `EuiBasicTable` and `EuiInMemoryTable`'s selection checkboxes
to have unique aria-labels per row
([#7672](elastic/eui#7672))
- Added `aria-valuetext` attributes to `EuiRange`s with tick labels for
improved screen reader UX
([#7675](elastic/eui#7675))
- Updated `EuiAccordion` to keep focus on accordion trigger instead of
moving to content on click/keypress
([#7696](elastic/eui#7696))
- Added `aria-disabled` attribute to `EuiHorizontalSteps` when status is
"disabled" ([#7699](elastic/eui#7699))

---------

Co-authored-by: Tomasz Kajtoch <tomasz.kajtoch@elastic.co>
mgadewoll pushed a commit to mgadewoll/eui that referenced this pull request May 3, 2024
…` prop (elastic#7713)

Co-authored-by: Cee Chen <constance.chen@elastic.co>
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.

[EuiTabbedContent] TypeError: Cannot read property 'content' of undefined
4 participants