diff --git a/src/components/structures/TabbedView.tsx b/src/components/structures/TabbedView.tsx index d6b88b1d4f..fa717126e5 100644 --- a/src/components/structures/TabbedView.tsx +++ b/src/components/structures/TabbedView.tsx @@ -18,7 +18,6 @@ limitations under the License. import * as React from "react"; import classNames from "classnames"; -import { logger } from "matrix-js-sdk/src/logger"; import { _t, TranslationKey } from "../../languageHandler"; import AutoHideScrollbar from "./AutoHideScrollbar"; @@ -47,6 +46,18 @@ export class Tab { ) {} } +export function useActiveTabWithDefault( + tabs: NonEmptyArray>, + defaultTabID: T, + initialTabID?: T, +): [T, (tabId: T) => void] { + const [activeTabId, setActiveTabId] = React.useState( + initialTabID && tabs.some((t) => t.id === initialTabID) ? initialTabID : defaultTabID, + ); + + return [activeTabId, setActiveTabId]; +} + export enum TabLocation { LEFT = "left", TOP = "top", @@ -113,12 +124,12 @@ function TabLabel({ tab, isActive, onClick }: ITabLabelProps { // An array of objects representign tabs that the tabbed view will display. tabs: NonEmptyArray>; - // The ID of the tab to display initially. - initialTabId?: T; + // The ID of the tab to show + activeTabId: T; // The location of the tabs, dictating the layout of the TabbedView. tabLocation?: TabLocation; - // A callback that is called when the active tab changes. - onChange?: (tabId: T) => void; + // A callback that is called when the active tab should change + onChange: (tabId: T) => void; // The screen name to report to Posthog. screenName?: ScreenName; } @@ -130,39 +141,19 @@ interface IProps { export default function TabbedView(props: IProps): JSX.Element { const tabLocation = props.tabLocation ?? TabLocation.LEFT; - const [activeTabId, setActiveTabId] = React.useState((): T => { - const initialTabIdIsValid = props.tabs.find((tab) => tab.id === props.initialTabId); - // unfortunately typescript doesn't infer the types coorectly if the null check is included above - return initialTabIdIsValid && props.initialTabId ? props.initialTabId : props.tabs[0].id; - }); - const getTabById = (id: T): Tab | undefined => { return props.tabs.find((tab) => tab.id === id); }; - /** - * Shows the given tab - * @param {Tab} tab the tab to show - */ - const setActiveTab = (tab: Tab): void => { - // make sure this tab is still in available tabs - if (!!getTabById(tab.id)) { - props.onChange?.(tab.id); - setActiveTabId(tab.id); - } else { - logger.error("Could not find tab " + tab.label + " in tabs"); - } - }; - const labels = props.tabs.map((tab) => ( setActiveTab(tab)} + isActive={tab.id === props.activeTabId} + onClick={() => props.onChange(tab.id)} /> )); - const tab = getTabById(activeTabId); + const tab = getTabById(props.activeTabId); const panel = tab ? : null; const tabbedViewClasses = classNames({ diff --git a/src/components/views/dialogs/InviteDialog.tsx b/src/components/views/dialogs/InviteDialog.tsx index 2b3c7af8db..58d2d28a11 100644 --- a/src/components/views/dialogs/InviteDialog.tsx +++ b/src/components/views/dialogs/InviteDialog.tsx @@ -1538,7 +1538,7 @@ export default class InviteDialog extends React.PureComponent diff --git a/src/components/views/dialogs/RoomSettingsDialog.tsx b/src/components/views/dialogs/RoomSettingsDialog.tsx index a58cef95a7..213ee94aca 100644 --- a/src/components/views/dialogs/RoomSettingsDialog.tsx +++ b/src/components/views/dialogs/RoomSettingsDialog.tsx @@ -56,11 +56,12 @@ export const enum RoomSettingsTab { interface IProps { roomId: string; onFinished: (success?: boolean) => void; - initialTabId?: string; + initialTabId?: RoomSettingsTab; } interface IState { room: Room; + activeTabId: RoomSettingsTab; } class RoomSettingsDialog extends React.Component { @@ -70,7 +71,7 @@ class RoomSettingsDialog extends React.Component { super(props); const room = this.getRoom(); - this.state = { room }; + this.state = { room, activeTabId: props.initialTabId || RoomSettingsTab.General }; } public componentDidMount(): void { @@ -128,6 +129,10 @@ class RoomSettingsDialog extends React.Component { if (event.getType() === EventType.RoomJoinRules) this.forceUpdate(); }; + private onTabChange = (tabId: RoomSettingsTab): void => { + this.setState({ activeTabId: tabId }); + }; + private getTabs(): NonEmptyArray> { const tabs: Tab[] = []; @@ -246,8 +251,9 @@ class RoomSettingsDialog extends React.Component {
diff --git a/src/components/views/dialogs/SpacePreferencesDialog.tsx b/src/components/views/dialogs/SpacePreferencesDialog.tsx index 8e8e4ce398..2ba3a7bec6 100644 --- a/src/components/views/dialogs/SpacePreferencesDialog.tsx +++ b/src/components/views/dialogs/SpacePreferencesDialog.tsx @@ -33,7 +33,6 @@ import SettingsSubsection, { SettingsSubsectionText } from "../settings/shared/S interface IProps { space: Room; - initialTabId?: SpacePreferenceTab; onFinished(): void; } @@ -68,7 +67,7 @@ const SpacePreferencesAppearanceTab: React.FC> = ({ space ); }; -const SpacePreferencesDialog: React.FC = ({ space, initialTabId, onFinished }) => { +const SpacePreferencesDialog: React.FC = ({ space, onFinished }) => { const tabs: NonEmptyArray> = [ new Tab( SpacePreferenceTab.Appearance, @@ -90,7 +89,7 @@ const SpacePreferencesDialog: React.FC = ({ space, initialTabId, onFinis
- + {}} />
); diff --git a/src/components/views/dialogs/SpaceSettingsDialog.tsx b/src/components/views/dialogs/SpaceSettingsDialog.tsx index 0318e1af62..016307d899 100644 --- a/src/components/views/dialogs/SpaceSettingsDialog.tsx +++ b/src/components/views/dialogs/SpaceSettingsDialog.tsx @@ -82,6 +82,8 @@ const SpaceSettingsDialog: React.FC = ({ matrixClient: cli, space, onFin ].filter(Boolean) as NonEmptyArray>; }, [cli, space, onFinished]); + const [activeTabId, setActiveTabId] = React.useState(SpaceSettingsTab.General); + return ( = ({ matrixClient: cli, space, onFin fixedWidth={false} >
- +
); diff --git a/src/components/views/dialogs/UserSettingsDialog.tsx b/src/components/views/dialogs/UserSettingsDialog.tsx index a6c82d50c2..2b28e423ed 100644 --- a/src/components/views/dialogs/UserSettingsDialog.tsx +++ b/src/components/views/dialogs/UserSettingsDialog.tsx @@ -17,7 +17,7 @@ limitations under the License. import React from "react"; -import TabbedView, { Tab } from "../../structures/TabbedView"; +import TabbedView, { Tab, useActiveTabWithDefault } from "../../structures/TabbedView"; import { _t, _td } from "../../../languageHandler"; import GeneralUserSettingsTab from "../settings/tabs/user/GeneralUserSettingsTab"; import SettingsStore from "../../../settings/SettingsStore"; @@ -173,6 +173,8 @@ export default function UserSettingsDialog(props: IProps): JSX.Element { return tabs as NonEmptyArray>; }; + const [activeTabId, setActiveTabId] = useActiveTabWithDefault(getTabs(), UserTab.General, props.initialTabId); + return ( // XXX: SDKContext is provided within the LoggedInView subtree. // Modals function outside the MatrixChat React tree, so sdkContext is reprovided here to simulate that. @@ -185,7 +187,12 @@ export default function UserSettingsDialog(props: IProps): JSX.Element { title={_t("common|settings")} >
- +
diff --git a/src/components/views/elements/DesktopCapturerSourcePicker.tsx b/src/components/views/elements/DesktopCapturerSourcePicker.tsx index e4d52a8104..18cb9e6f4e 100644 --- a/src/components/views/elements/DesktopCapturerSourcePicker.tsx +++ b/src/components/views/elements/DesktopCapturerSourcePicker.tsx @@ -85,8 +85,6 @@ export interface PickerIProps { onFinished(source?: DesktopCapturerSource): void; } -type TabId = "screen" | "window"; - export default class DesktopCapturerSourcePicker extends React.Component { public interval?: number; @@ -127,15 +125,15 @@ export default class DesktopCapturerSourcePicker extends React.Component { - this.setState({ selectedSource: undefined }); + private onTabChange = (tab: Tabs): void => { + this.setState({ selectedSource: undefined, selectedTab: tab }); }; private onCloseClick = (): void => { this.props.onFinished(); }; - private getTab(type: TabId, label: TranslationKey): Tab { + private getTab(type: Tabs, label: TranslationKey): Tab { const sources = this.state.sources .filter((source) => source.id.startsWith(type)) .map((source) => { @@ -153,9 +151,9 @@ export default class DesktopCapturerSourcePicker extends React.Component> = [ - this.getTab("screen", _td("voip|screenshare_monitor")), - this.getTab("window", _td("voip|screenshare_window")), + const tabs: NonEmptyArray> = [ + this.getTab(Tabs.Screens, _td("voip|screenshare_monitor")), + this.getTab(Tabs.Windows, _td("voip|screenshare_window")), ]; return ( @@ -164,7 +162,12 @@ export default class DesktopCapturerSourcePicker extends React.Component - + ", () => { const defaultProps = { tabLocation: TabLocation.LEFT, tabs: [generalTab, labsTab, securityTab] as NonEmptyArray>, + onChange: () => {}, }; - const getComponent = (props = {}): React.ReactElement => ; + const getComponent = ( + props: { + activeTabId: "GENERAL" | "LABS" | "SECURITY"; + onChange?: () => any; + tabs?: NonEmptyArray>; + } = { + activeTabId: "GENERAL", + }, + ): React.ReactElement => ; const getTabTestId = (tab: Tab): string => `settings-tab-${tab.id}`; const getActiveTab = (container: HTMLElement): Element | undefined => @@ -42,38 +51,15 @@ describe("", () => { expect(container).toMatchSnapshot(); }); - it("renders first tab as active tab when no initialTabId", () => { - const { container } = render(getComponent()); - expect(getActiveTab(container)?.textContent).toEqual(_t(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(_t(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(_t(securityTab.label)); - expect(getActiveTabBody(container)?.textContent).toEqual("security"); - }); - - it("sets active tab on tab click", () => { - const { container, getByTestId } = render(getComponent()); - - act(() => { - fireEvent.click(getByTestId(getTabTestId(securityTab))); - }); - + it("renders activeTabId tab as active when valid", () => { + const { container } = render(getComponent({ activeTabId: securityTab.id })); expect(getActiveTab(container)?.textContent).toEqual(_t(securityTab.label)); expect(getActiveTabBody(container)?.textContent).toEqual("security"); }); it("calls onchange on on tab click", () => { const onChange = jest.fn(); - const { getByTestId } = render(getComponent({ onChange })); + const { getByTestId } = render(getComponent({ activeTabId: "GENERAL", onChange })); act(() => { fireEvent.click(getByTestId(getTabTestId(securityTab))); @@ -84,31 +70,13 @@ describe("", () => { it("keeps same tab active when order of tabs changes", () => { // start with middle tab active - const { container, rerender } = render(getComponent({ initialTabId: labsTab.id })); + const { container, rerender } = render(getComponent({ activeTabId: labsTab.id })); expect(getActiveTab(container)?.textContent).toEqual(_t(labsTab.label)); - rerender(getComponent({ tabs: [labsTab, generalTab, securityTab] })); + rerender(getComponent({ tabs: [labsTab, generalTab, securityTab], activeTabId: labsTab.id })); // labs tab still active expect(getActiveTab(container)?.textContent).toEqual(_t(labsTab.label)); }); - - it("does not reactivate inititalTabId on rerender", () => { - const { container, getByTestId, rerender } = render(getComponent()); - - expect(getActiveTab(container)?.textContent).toEqual(_t(generalTab.label)); - - // make security tab active - act(() => { - fireEvent.click(getByTestId(getTabTestId(securityTab))); - }); - expect(getActiveTab(container)?.textContent).toEqual(_t(securityTab.label)); - - // rerender with new tab location - rerender(getComponent({ tabLocation: TabLocation.TOP })); - - // still security tab - expect(getActiveTab(container)?.textContent).toEqual(_t(securityTab.label)); - }); }); diff --git a/test/components/views/elements/DesktopCapturerSourcePicker-test.tsx b/test/components/views/elements/DesktopCapturerSourcePicker-test.tsx new file mode 100644 index 0000000000..aafa356a3d --- /dev/null +++ b/test/components/views/elements/DesktopCapturerSourcePicker-test.tsx @@ -0,0 +1,100 @@ +/* +Copyright 2024 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 { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; + +import DesktopCapturerSourcePicker from "../../../../src/components/views/elements/DesktopCapturerSourcePicker"; +import PlatformPeg from "../../../../src/PlatformPeg"; +import BasePlatform from "../../../../src/BasePlatform"; + +const SOURCES = [ + { + id: "screen1", + name: "Screen 1", + thumbnailURL: "data:image/png;base64,", + }, + { + id: "window1", + name: "Window 1", + thumbnailURL: "data:image/png;base64,", + }, +]; + +describe("DesktopCapturerSourcePicker", () => { + beforeEach(() => { + const plaf = { + getDesktopCapturerSources: jest.fn().mockResolvedValue(SOURCES), + supportsSetting: jest.fn().mockReturnValue(false), + }; + jest.spyOn(PlatformPeg, "get").mockReturnValue(plaf as unknown as BasePlatform); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it("should render the component", () => { + render( {}} />); + expect(screen.getByRole("button", { name: "Cancel" })).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Share" })).toBeInTheDocument(); + }); + + it("should disable share button until a source is selected", () => { + render( {}} />); + expect(screen.getByRole("button", { name: "Share" })).toBeDisabled(); + }); + + it("should contain a screen source in the default tab", async () => { + render( {}} />); + + const screen1Button = await screen.findByRole("button", { name: "Screen 1" }); + + expect(screen1Button).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Window 1" })).not.toBeInTheDocument(); + }); + + it("should contain a window source in the window tab", async () => { + render( {}} />); + + await userEvent.click(screen.getByRole("tab", { name: "Application window" })); + + const window1Button = await screen.findByRole("button", { name: "Window 1" }); + + expect(window1Button).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Screen 1" })).not.toBeInTheDocument(); + }); + + it("should call onFinished with no arguments if cancelled", async () => { + const onFinished = jest.fn(); + render(); + + await userEvent.click(screen.getByRole("button", { name: "Cancel" })); + expect(onFinished).toHaveBeenCalledWith(); + }); + + it("should call onFinished with the selected source when share clicked", async () => { + const onFinished = jest.fn(); + render(); + + const screen1Button = await screen.findByRole("button", { name: "Screen 1" }); + + await userEvent.click(screen1Button); + await userEvent.click(screen.getByRole("button", { name: "Share" })); + expect(onFinished).toHaveBeenCalledWith(SOURCES[0]); + }); +});