From eee715c83a257c2337af466870aa99c6bc133e33 Mon Sep 17 00:00:00 2001 From: Shyamal Date: Thu, 25 Apr 2024 14:51:20 -0300 Subject: [PATCH 1/6] [EuiTabbedContent] Fixed issue of TypeError #7350 --- .../tabbed_content.test.tsx.snap | 60 +++++++++---------- .../tabs/tabbed_content/tabbed_content.tsx | 23 +++++-- 2 files changed, 48 insertions(+), 35 deletions(-) diff --git a/src/components/tabs/tabbed_content/__snapshots__/tabbed_content.test.tsx.snap b/src/components/tabs/tabbed_content/__snapshots__/tabbed_content.test.tsx.snap index 7d16b9bd716..cfba1be1fa7 100644 --- a/src/components/tabs/tabbed_content/__snapshots__/tabbed_content.test.tsx.snap +++ b/src/components/tabs/tabbed_content/__snapshots__/tabbed_content.test.tsx.snap @@ -8,8 +8,8 @@ exports[`EuiTabbedContent behavior when selected tab state isn't controlled by t > - - -
-

- Elasticsearch content -

-
- -`; - -exports[`EuiTabbedContent behavior when uncontrolled, the selected tab should update if it receives new content 1`] = ` -
-
- - -
-
-

- updated Kibana content -

-
-
-`; - exports[`EuiTabbedContent is rendered with required props and tabs 1`] = `
{ describe('onTabClick', () => { test('is called when a tab is clicked', () => { const onTabClickHandler = jest.fn(); - const component = mount( + const { getByTestSubject } = render( ); - findTestSubject(component, 'kibanaTab').simulate('click'); + fireEvent.click(getByTestSubject('kibanaTab')); expect(onTabClickHandler).toBeCalledTimes(1); expect(onTabClickHandler).toBeCalledWith(kibanaTab); }); @@ -94,33 +94,32 @@ describe('EuiTabbedContent', () => { }); describe('behavior', () => { - test("when selected tab state isn't controlled by the owner, select the first tab by default", () => { - const { container } = render(); - expect(container.firstChild).toMatchSnapshot(); - }); - - test('when uncontrolled, the selected tab should update if it receives new content', () => { - const tabs = [ - elasticsearchTab, - { - ...kibanaTab, - }, - ]; - const component = mount(); - - component.find('EuiTab[id="kibana"] button').first().simulate('click'); - - component.setProps({ - tabs: [ - elasticsearchTab, - { - ...kibanaTab, - content:

updated Kibana content

, - }, - ], + describe('when selectedTab state is uncontrolled', () => { + it('selects the first tab by default', () => { + const { getAllByRole } = render(); + const tabElements = getAllByRole('tab'); + expect(tabElements[0]).toHaveAttribute('aria-selected', 'true'); + expect(tabElements[1]).toHaveAttribute('aria-selected', 'false'); }); + }); - expect(component.render()).toMatchSnapshot(); + it("does not reset the existing selected tab if the tab's contents update", () => { + const { rerender, getAllByRole, getByTestSubject } = render( + + ); + + fireEvent.click(getByTestSubject('kibanaTab')); + expect(getAllByRole('tab')[1]).toHaveAttribute('aria-selected', 'true'); + + rerender( + pdated Kibana content

}, + ]} + /> + ); + expect(getAllByRole('tab')[1]).toHaveAttribute('aria-selected', 'true'); }); }); }); From 7bb41368835566eafac9f61b4ea558c0bfbb90f2 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Mon, 29 Apr 2024 13:54:45 -0700 Subject: [PATCH 4/6] Simplify selected tab logic + add a test - 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 --- .../tabbed_content/tabbed_content.test.tsx | 27 +++++++++++++++++++ .../tabs/tabbed_content/tabbed_content.tsx | 18 +++---------- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/src/components/tabs/tabbed_content/tabbed_content.test.tsx b/src/components/tabs/tabbed_content/tabbed_content.test.tsx index 79522ff9e42..c809b6842df 100644 --- a/src/components/tabs/tabbed_content/tabbed_content.test.tsx +++ b/src/components/tabs/tabbed_content/tabbed_content.test.tsx @@ -121,5 +121,32 @@ describe('EuiTabbedContent', () => { ); expect(getAllByRole('tab')[1]).toHaveAttribute('aria-selected', 'true'); }); + + it('resets the selected tab to the first if the `tabs` content completely changes', () => { + const { rerender, getAllByRole, getByTestSubject } = render( + + ); + + fireEvent.click(getByTestSubject('kibanaTab')); + expect(getAllByRole('tab')[1]).toHaveAttribute('aria-selected', 'true'); + + rerender( + Hello

, + }, + { + id: 'world', + name: 'New tab 2', + content:

World

, + }, + ]} + /> + ); + expect(getAllByRole('tab')[0]).toHaveAttribute('aria-selected', 'true'); + }); }); }); diff --git a/src/components/tabs/tabbed_content/tabbed_content.tsx b/src/components/tabs/tabbed_content/tabbed_content.tsx index 4ed1f74727c..d8f4c736825 100644 --- a/src/components/tabs/tabbed_content/tabbed_content.tsx +++ b/src/components/tabs/tabbed_content/tabbed_content.tsx @@ -137,19 +137,6 @@ export class EuiTabbedContent extends Component< } }; - static getDerivedStateFromProps( - nextProps: EuiTabbedContentProps, - currentState: EuiTabbedContentState - ) { - if (!nextProps.tabs?.find((tab) => tab.id === currentState.selectedTabId)) { - return { - ...currentState, - selectedTabId: nextProps?.tabs[0]?.id, - }; - } - return null; - } - render() { const { className, @@ -168,9 +155,10 @@ export class EuiTabbedContent extends Component< externalSelectedTab || tabs.find( (tab: EuiTabbedContentTab) => tab.id === this.state.selectedTabId - ); + ) || + tabs[0]; // Fall back to the first tab if a selected tab can't be found - const { content: selectedTabContent, id: selectedTabId } = selectedTab!; + const { content: selectedTabContent, id: selectedTabId } = selectedTab; return (
From c50ee12297b3faf00bd5f6a8c684b42a5c3ef958 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Mon, 29 Apr 2024 14:02:20 -0700 Subject: [PATCH 5/6] changelog --- changelogs/upcoming/7713.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelogs/upcoming/7713.md diff --git a/changelogs/upcoming/7713.md b/changelogs/upcoming/7713.md new file mode 100644 index 00000000000..1f53a7e94ff --- /dev/null +++ b/changelogs/upcoming/7713.md @@ -0,0 +1,3 @@ +**Bug fixes** + +- Fixed an `EuiTabbedContent` edge case bug that occurred when updated with a completely different set of `tabs` From ec15d3ebbda4e1b9a7ace78133055597c3c86511 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Mon, 29 Apr 2024 14:07:37 -0700 Subject: [PATCH 6/6] Remove unnecessary types - typescript already infers this for us --- src/components/tabs/tabbed_content/tabbed_content.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/components/tabs/tabbed_content/tabbed_content.tsx b/src/components/tabs/tabbed_content/tabbed_content.tsx index d8f4c736825..7a81e25ea25 100644 --- a/src/components/tabs/tabbed_content/tabbed_content.tsx +++ b/src/components/tabs/tabbed_content/tabbed_content.tsx @@ -153,9 +153,7 @@ export class EuiTabbedContent extends Component< // Allow the consumer to control tab selection. const selectedTab = externalSelectedTab || - tabs.find( - (tab: EuiTabbedContentTab) => tab.id === this.state.selectedTabId - ) || + tabs.find((tab) => tab.id === this.state.selectedTabId) || tabs[0]; // Fall back to the first tab if a selected tab can't be found const { content: selectedTabContent, id: selectedTabId } = selectedTab; @@ -169,7 +167,7 @@ export class EuiTabbedContent extends Component< onFocus={this.initializeFocus} onBlur={this.removeFocus} > - {tabs.map((tab: EuiTabbedContentTab) => { + {tabs.map((tab) => { const { id, name,