use stable reference for active tab in tabbedView (#9145)

src/components/structures/TabbedView.tsx

@@ -60,21 +60,16 @@ interface IProps {
 }
 
 interface IState {
-    activeTabIndex: number;
+    activeTabId: string;
 }
 
 export default class TabbedView extends React.Component<IProps, IState> {
     constructor(props: IProps) {
         super(props);
 
-        let activeTabIndex = 0;
+        const initialTabIdIsValid = props.tabs.find(tab => tab.id === props.initialTabId);
-        if (props.initialTabId) {
-            const tabIndex = props.tabs.findIndex(t => t.id === props.initialTabId);
-            if (tabIndex >= 0) activeTabIndex = tabIndex;
-        }
-
         this.state = {
-            activeTabIndex,
+            activeTabId: initialTabIdIsValid ? props.initialTabId : props.tabs[0]?.id,
         };
     }
 
@@ -82,9 +77,8 @@ export default class TabbedView extends React.Component<IProps, IState> {
         tabLocation: TabLocation.LEFT,
     };
 
-    private getActiveTabIndex() {
+    private getTabById(id: string): Tab | undefined {
-        if (!this.state || !this.state.activeTabIndex) return 0;
+        return this.props.tabs.find(tab => tab.id === id);
-        return this.state.activeTabIndex;
     }
 
     /**
@@ -93,10 +87,10 @@ export default class TabbedView extends React.Component<IProps, IState> {
      * @private
      */
     private setActiveTab(tab: Tab) {
-        const idx = this.props.tabs.indexOf(tab);
+        // make sure this tab is still in available tabs
-        if (idx !== -1) {
+        if (!!this.getTabById(tab.id)) {
             if (this.props.onChange) this.props.onChange(tab.id);
-            this.setState({ activeTabIndex: idx });
+            this.setState({ activeTabId: tab.id });
         } else {
             logger.error("Could not find tab " + tab.label + " in tabs");
         }
@@ -105,8 +99,7 @@ export default class TabbedView extends React.Component<IProps, IState> {
     private renderTabLabel(tab: Tab) {
         let classes = "mx_TabbedView_tabLabel ";
 
-        const idx = this.props.tabs.indexOf(tab);
+        if (this.state.activeTabId === tab.id) classes += "mx_TabbedView_tabLabel_active";
-        if (idx === this.getActiveTabIndex()) classes += "mx_TabbedView_tabLabel_active";
 
         let tabIcon = null;
         if (tab.icon) {
@@ -143,8 +136,8 @@ export default class TabbedView extends React.Component<IProps, IState> {
 
     public render(): React.ReactNode {
         const labels = this.props.tabs.map(tab => this.renderTabLabel(tab));
-        const tab = this.props.tabs[this.getActiveTabIndex()];
+        const tab = this.getTabById(this.state.activeTabId);
-        const panel = this.renderTabPanel(tab);
+        const panel = tab ? this.renderTabPanel(tab) : null;
 
         const tabbedViewClasses = classNames({
             'mx_TabbedView': true,

test/components/structures/TabbedView-test.tsx

@@ -0,0 +1,133 @@
+/*
+Copyright 2022 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import React from 'react';
+import { fireEvent, render } from "@testing-library/react";
+import { act } from 'react-dom/test-utils';
+
+import TabbedView, { Tab, TabLocation } from "../../../src/components/structures/TabbedView";
+
+describe('<TabbedView />', () => {
+    const generalTab = new Tab(
+        'GENERAL',
+        'General',
+        'general',
+        <div>general</div>,
+    );
+    const labsTab = new Tab(
+        'LABS',
+        'Labs',
+        'labs',
+        <div>labs</div>,
+    );
+    const securityTab = new Tab(
+        'SECURITY',
+        'Security',
+        'security',
+        <div>security</div>,
+    );
+    const defaultProps = {
+        tabLocation: TabLocation.LEFT,
+        tabs: [generalTab, labsTab, securityTab],
+    };
+    const getComponent = (props = {}): React.ReactElement => <TabbedView {...defaultProps} {...props} />;
+
+    const getTabTestId = (tab: Tab): string => `settings-tab-${tab.id}`;
+    const getActiveTab = (container: HTMLElement): Element | undefined =>
+        container.getElementsByClassName('mx_TabbedView_tabLabel_active')[0];
+    const getActiveTabBody = (container: HTMLElement): Element | undefined =>
+        container.getElementsByClassName('mx_TabbedView_tabPanel')[0];
+
+    it('renders tabs', () => {
+        const { container } = render(getComponent());
+        expect(container).toMatchSnapshot();
+    });
+
+    it('renders first tab as active tab when no initialTabId', () => {
+        const { container } = render(getComponent());
+        expect(getActiveTab(container).textContent).toEqual(generalTab.label);
+        expect(getActiveTabBody(container).textContent).toEqual('general');
+    });
+
+    it('renders first tab as active tab when initialTabId is not valid', () => {
+        const { container } = render(getComponent({ initialTabId: 'bad-tab-id' }));
+        expect(getActiveTab(container).textContent).toEqual(generalTab.label);
+        expect(getActiveTabBody(container).textContent).toEqual('general');
+    });
+
+    it('renders initialTabId tab as active when valid', () => {
+        const { container } = render(getComponent({ initialTabId: securityTab.id }));
+        expect(getActiveTab(container).textContent).toEqual(securityTab.label);
+        expect(getActiveTabBody(container).textContent).toEqual('security');
+    });
+
+    it('renders without error when there are no tabs', () => {
+        const { container } = render(getComponent({ tabs: [] }));
+        expect(container).toMatchSnapshot();
+    });
+
+    it('sets active tab on tab click', () => {
+        const { container, getByTestId } = render(getComponent());
+
+        act(() => {
+            fireEvent.click(getByTestId(getTabTestId(securityTab)));
+        });
+
+        expect(getActiveTab(container).textContent).toEqual(securityTab.label);
+        expect(getActiveTabBody(container).textContent).toEqual('security');
+    });
+
+    it('calls onchange on on tab click', () => {
+        const onChange = jest.fn();
+        const { getByTestId } = render(getComponent({ onChange }));
+
+        act(() => {
+            fireEvent.click(getByTestId(getTabTestId(securityTab)));
+        });
+
+        expect(onChange).toHaveBeenCalledWith(securityTab.id);
+    });
+
+    it('keeps same tab active when order of tabs changes', () => {
+        // start with middle tab active
+        const { container, rerender } = render(getComponent({ initialTabId: labsTab.id }));
+
+        expect(getActiveTab(container).textContent).toEqual(labsTab.label);
+
+        rerender(getComponent({ tabs: [labsTab, generalTab, securityTab] }));
+
+        // labs tab still active
+        expect(getActiveTab(container).textContent).toEqual(labsTab.label);
+    });
+
+    it('does not reactivate inititalTabId on rerender', () => {
+        const { container, getByTestId, rerender } = render(getComponent());
+
+        expect(getActiveTab(container).textContent).toEqual(generalTab.label);
+
+        // make security tab active
+        act(() => {
+            fireEvent.click(getByTestId(getTabTestId(securityTab)));
+        });
+        expect(getActiveTab(container).textContent).toEqual(securityTab.label);
+
+        // rerender with new tab location
+        rerender(getComponent({ tabLocation: TabLocation.TOP }));
+
+        // still security tab
+        expect(getActiveTab(container).textContent).toEqual(securityTab.label);
+    });
+});

test/components/structures/__snapshots__/TabbedView-test.tsx.snap

@@ -0,0 +1,83 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`<TabbedView /> renders tabs 1`] = `
+<div>
+  <div
+    class="mx_TabbedView mx_TabbedView_tabsOnLeft"
+  >
+    <div
+      class="mx_TabbedView_tabLabels"
+    >
+      <div
+        class="mx_AccessibleButton mx_TabbedView_tabLabel mx_TabbedView_tabLabel_active"
+        data-testid="settings-tab-GENERAL"
+        role="button"
+        tabindex="0"
+      >
+        <span
+          class="mx_TabbedView_maskedIcon general"
+        />
+        <span
+          class="mx_TabbedView_tabLabel_text"
+        >
+          General
+        </span>
+      </div>
+      <div
+        class="mx_AccessibleButton mx_TabbedView_tabLabel "
+        data-testid="settings-tab-LABS"
+        role="button"
+        tabindex="0"
+      >
+        <span
+          class="mx_TabbedView_maskedIcon labs"
+        />
+        <span
+          class="mx_TabbedView_tabLabel_text"
+        >
+          Labs
+        </span>
+      </div>
+      <div
+        class="mx_AccessibleButton mx_TabbedView_tabLabel "
+        data-testid="settings-tab-SECURITY"
+        role="button"
+        tabindex="0"
+      >
+        <span
+          class="mx_TabbedView_maskedIcon security"
+        />
+        <span
+          class="mx_TabbedView_tabLabel_text"
+        >
+          Security
+        </span>
+      </div>
+    </div>
+    <div
+      class="mx_TabbedView_tabPanel"
+    >
+      <div
+        class="mx_AutoHideScrollbar mx_TabbedView_tabPanelContent"
+        tabindex="-1"
+      >
+        <div>
+          general
+        </div>
+      </div>
+    </div>
+  </div>
+</div>
+`;
+
+exports[`<TabbedView /> renders without error when there are no tabs 1`] = `
+<div>
+  <div
+    class="mx_TabbedView mx_TabbedView_tabsOnLeft"
+  >
+    <div
+      class="mx_TabbedView_tabLabels"
+    />
+  </div>
+</div>
+`;