From 56c95467de98dd8f25ca56d3f471fad480e4c574 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 22 Sep 2022 15:08:14 +0100 Subject: [PATCH] Don't show feedback prompts when that UIFeature is disabled (#9305) --- src/components/structures/ThreadPanel.tsx | 4 +- src/components/views/beta/BetaCard.tsx | 3 +- .../dialogs/spotlight/SpotlightDialog.tsx | 4 +- .../views/spaces/SpaceCreateMenu.tsx | 5 +- .../UserOnboardingFeedback.tsx | 3 +- src/utils/Feedback.ts | 23 +++ .../structures/ThreadPanel-test.tsx | 86 +++++++---- .../__snapshots__/ThreadPanel-test.tsx.snap | 142 ++++++------------ test/components/views/beta/BetaCard-test.tsx | 80 ++++++++++ .../views/dialogs/SpotlightDialog-test.tsx | 32 ++++ test/test-utils/test-utils.ts | 2 + test/utils/Feedback-test.ts | 46 ++++++ 12 files changed, 303 insertions(+), 127 deletions(-) create mode 100644 src/utils/Feedback.ts create mode 100644 test/components/views/beta/BetaCard-test.tsx create mode 100644 test/utils/Feedback-test.ts diff --git a/src/components/structures/ThreadPanel.tsx b/src/components/structures/ThreadPanel.tsx index 8ccb7fbdd4..245c604c0a 100644 --- a/src/components/structures/ThreadPanel.tsx +++ b/src/components/structures/ThreadPanel.tsx @@ -33,7 +33,6 @@ import Measured from '../views/elements/Measured'; import PosthogTrackers from "../../PosthogTrackers"; import AccessibleButton, { ButtonEvent } from "../views/elements/AccessibleButton"; import { BetaPill } from '../views/beta/BetaCard'; -import SdkConfig from '../../SdkConfig'; import Modal from '../../Modal'; import BetaFeedbackDialog from '../views/dialogs/BetaFeedbackDialog'; import { Action } from '../../dispatcher/actions'; @@ -41,6 +40,7 @@ import { UserTab } from '../views/dialogs/UserTab'; import dis from '../../dispatcher/dispatcher'; import Spinner from "../views/elements/Spinner"; import Heading from '../views/typography/Heading'; +import { shouldShowFeedback } from "../../utils/Feedback"; interface IProps { roomId: string; @@ -234,7 +234,7 @@ const ThreadPanel: React.FC = ({ } }, [timelineSet, timelinePanel]); - const openFeedback = SdkConfig.get().bug_report_endpoint_url ? () => { + const openFeedback = shouldShowFeedback() ? () => { Modal.createDialog(BetaFeedbackDialog, { featureId: "feature_thread", }); diff --git a/src/components/views/beta/BetaCard.tsx b/src/components/views/beta/BetaCard.tsx index 3db52a0b69..82f00e1697 100644 --- a/src/components/views/beta/BetaCard.tsx +++ b/src/components/views/beta/BetaCard.tsx @@ -28,6 +28,7 @@ import SettingsFlag from "../elements/SettingsFlag"; import { useFeatureEnabled } from "../../../hooks/useSettings"; import InlineSpinner from "../elements/InlineSpinner"; import AccessibleTooltipButton from "../elements/AccessibleTooltipButton"; +import { shouldShowFeedback } from "../../../utils/Feedback"; // XXX: Keep this around for re-use in future Betas @@ -88,7 +89,7 @@ const BetaCard = ({ title: titleOverride, featureId }: IProps) => { } = info; let feedbackButton; - if (value && feedbackLabel && feedbackSubheading && SdkConfig.get().bug_report_endpoint_url) { + if (value && feedbackLabel && feedbackSubheading && shouldShowFeedback()) { feedbackButton = { Modal.createDialog(BetaFeedbackDialog, { featureId }); diff --git a/src/components/views/dialogs/spotlight/SpotlightDialog.tsx b/src/components/views/dialogs/spotlight/SpotlightDialog.tsx index 85f4e6ed50..b04299869c 100644 --- a/src/components/views/dialogs/spotlight/SpotlightDialog.tsx +++ b/src/components/views/dialogs/spotlight/SpotlightDialog.tsx @@ -60,7 +60,6 @@ import Modal from "../../../../Modal"; import { PosthogAnalytics } from "../../../../PosthogAnalytics"; import { getCachedRoomIDForAlias } from "../../../../RoomAliasCache"; import { showStartChatInviteDialog } from "../../../../RoomInvite"; -import SdkConfig from "../../../../SdkConfig"; import { SettingLevel } from "../../../../settings/SettingLevel"; import SettingsStore from "../../../../settings/SettingsStore"; import { BreadcrumbsStore } from "../../../../stores/BreadcrumbsStore"; @@ -93,6 +92,7 @@ import { RoomContextDetails } from "../../rooms/RoomContextDetails"; import { TooltipOption } from "./TooltipOption"; import { isLocalRoom } from "../../../../utils/localRoom/isLocalRoom"; import { useSlidingSyncRoomSearch } from "../../../../hooks/useSlidingSyncRoomSearch"; +import { shouldShowFeedback } from "../../../../utils/Feedback"; const MAX_RECENT_SEARCHES = 10; const SECTION_LIMIT = 50; // only show 50 results per section for performance reasons @@ -1171,7 +1171,7 @@ const SpotlightDialog: React.FC = ({ initialText = "", initialFilter = n } }; - const openFeedback = SdkConfig.get().bug_report_endpoint_url ? () => { + const openFeedback = shouldShowFeedback() ? () => { Modal.createDialog(FeedbackDialog, { feature: "spotlight", }); diff --git a/src/components/views/spaces/SpaceCreateMenu.tsx b/src/components/views/spaces/SpaceCreateMenu.tsx index 1e919981e2..f8c8e889ea 100644 --- a/src/components/views/spaces/SpaceCreateMenu.tsx +++ b/src/components/views/spaces/SpaceCreateMenu.tsx @@ -31,14 +31,13 @@ import AccessibleButton from "../elements/AccessibleButton"; import Field from "../elements/Field"; import withValidation from "../elements/Validation"; import RoomAliasField from "../elements/RoomAliasField"; -import SdkConfig from "../../../SdkConfig"; import Modal from "../../../Modal"; import GenericFeatureFeedbackDialog from "../dialogs/GenericFeatureFeedbackDialog"; import SettingsStore from "../../../settings/SettingsStore"; import { getKeyBindingsManager } from "../../../KeyBindingsManager"; import { KeyBindingAction } from "../../../accessibility/KeyboardShortcuts"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; -import { UIFeature } from "../../../settings/UIFeature"; +import { shouldShowFeedback } from "../../../utils/Feedback"; export const createSpace = async ( name: string, @@ -101,7 +100,7 @@ const nameToLocalpart = (name: string): string => { // XXX: Temporary for the Spaces release only export const SpaceFeedbackPrompt = ({ onClick }: { onClick?: () => void }) => { - if (!SdkConfig.get().bug_report_endpoint_url || !SettingsStore.getValue(UIFeature.Feedback)) return null; + if (!shouldShowFeedback()) return null; return
{ _t("Spaces are a new feature.") } diff --git a/src/components/views/user-onboarding/UserOnboardingFeedback.tsx b/src/components/views/user-onboarding/UserOnboardingFeedback.tsx index b6bd03dfe8..a0b19c2fd8 100644 --- a/src/components/views/user-onboarding/UserOnboardingFeedback.tsx +++ b/src/components/views/user-onboarding/UserOnboardingFeedback.tsx @@ -22,9 +22,10 @@ import SdkConfig from "../../../SdkConfig"; import AccessibleButton from "../../views/elements/AccessibleButton"; import Heading from "../../views/typography/Heading"; import FeedbackDialog from "../dialogs/FeedbackDialog"; +import { shouldShowFeedback } from "../../../utils/Feedback"; export function UserOnboardingFeedback() { - if (!SdkConfig.get().bug_report_endpoint_url) { + if (!shouldShowFeedback()) { return null; } diff --git a/src/utils/Feedback.ts b/src/utils/Feedback.ts new file mode 100644 index 0000000000..1662fe3fe6 --- /dev/null +++ b/src/utils/Feedback.ts @@ -0,0 +1,23 @@ +/* +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 SdkConfig from "../SdkConfig"; +import SettingsStore from "../settings/SettingsStore"; +import { UIFeature } from "../settings/UIFeature"; + +export function shouldShowFeedback(): boolean { + return SdkConfig.get().bug_report_endpoint_url && SettingsStore.getValue(UIFeature.Feedback); +} diff --git a/test/components/structures/ThreadPanel-test.tsx b/test/components/structures/ThreadPanel-test.tsx index 37928c560d..cad8331e36 100644 --- a/test/components/structures/ThreadPanel-test.tsx +++ b/test/components/structures/ThreadPanel-test.tsx @@ -15,68 +15,102 @@ limitations under the License. */ import React from 'react'; -// eslint-disable-next-line deprecate/import -import { shallow, mount } from "enzyme"; +import { render, screen, fireEvent } from "@testing-library/react"; +import { mocked } from "jest-mock"; import 'focus-visible'; // to fix context menus -import { - ThreadFilterType, - ThreadPanelHeader, - ThreadPanelHeaderFilterOptionItem, -} from '../../../src/components/structures/ThreadPanel'; -import { ContextMenuButton } from '../../../src/accessibility/context_menu/ContextMenuButton'; -import ContextMenu from '../../../src/components/structures/ContextMenu'; +import ThreadPanel, { ThreadFilterType, ThreadPanelHeader } from '../../../src/components/structures/ThreadPanel'; import { _t } from '../../../src/languageHandler'; +import ResizeNotifier from '../../../src/utils/ResizeNotifier'; +import { RoomPermalinkCreator } from '../../../src/utils/permalinks/Permalinks'; +import { createTestClient, mkStubRoom } from '../../test-utils'; +import { shouldShowFeedback } from "../../../src/utils/Feedback"; +import MatrixClientContext from "../../../src/contexts/MatrixClientContext"; + +jest.mock("../../../src/utils/Feedback"); describe('ThreadPanel', () => { + describe("Feedback prompt", () => { + const cli = createTestClient(); + const room = mkStubRoom("!room:server", "room", cli); + mocked(cli.getRoom).mockReturnValue(room); + + it("should show feedback prompt if feedback is enabled", () => { + mocked(shouldShowFeedback).mockReturnValue(true); + + render( + + ); + expect(screen.queryByText("Give feedback")).toBeTruthy(); + }); + + it("should hide feedback prompt if feedback is disabled", () => { + mocked(shouldShowFeedback).mockReturnValue(false); + + render( + + ); + expect(screen.queryByText("Give feedback")).toBeFalsy(); + }); + }); + describe('Header', () => { it('expect that All filter for ThreadPanelHeader properly renders Show: All threads', () => { - const wrapper = shallow( + const { asFragment } = render( undefined} />, ); - expect(wrapper).toMatchSnapshot(); + expect(asFragment()).toMatchSnapshot(); }); it('expect that My filter for ThreadPanelHeader properly renders Show: My threads', () => { - const wrapper = shallow( + const { asFragment } = render( undefined} />, ); - expect(wrapper).toMatchSnapshot(); + expect(asFragment()).toMatchSnapshot(); }); it('expect that ThreadPanelHeader properly opens a context menu when clicked on the button', () => { - const wrapper = mount( + const { container } = render( undefined} />, ); - const found = wrapper.find(ContextMenuButton); - expect(found).not.toBe(undefined); - expect(found).not.toBe(null); - expect(wrapper.exists(ContextMenu)).toEqual(false); - found.simulate('click'); - expect(wrapper.exists(ContextMenu)).toEqual(true); + const found = container.querySelector(".mx_ThreadPanel_dropdown"); + expect(found).toBeTruthy(); + expect(screen.queryByRole("menu")).toBeFalsy(); + fireEvent.click(found); + expect(screen.queryByRole("menu")).toBeTruthy(); }); it('expect that ThreadPanelHeader has the correct option selected in the context menu', () => { - const wrapper = mount( + const { container } = render( undefined} />, ); - wrapper.find(ContextMenuButton).simulate('click'); - const found = wrapper.find(ThreadPanelHeaderFilterOptionItem); - expect(found.length).toEqual(2); - const foundButton = found.find('[aria-checked=true]').first(); - expect(foundButton.text()).toEqual(`${_t("All threads")}${_t('Shows all threads from current room')}`); + fireEvent.click(container.querySelector(".mx_ThreadPanel_dropdown")); + const found = screen.queryAllByRole("menuitemradio"); + expect(found).toHaveLength(2); + const foundButton = screen.queryByRole("menuitemradio", { checked: true }); + expect(foundButton.textContent).toEqual(`${_t("All threads")}${_t('Shows all threads from current room')}`); expect(foundButton).toMatchSnapshot(); }); }); diff --git a/test/components/structures/__snapshots__/ThreadPanel-test.tsx.snap b/test/components/structures/__snapshots__/ThreadPanel-test.tsx.snap index 16b3300927..b6266893dd 100644 --- a/test/components/structures/__snapshots__/ThreadPanel-test.tsx.snap +++ b/test/components/structures/__snapshots__/ThreadPanel-test.tsx.snap @@ -1,106 +1,64 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`ThreadPanel Header expect that All filter for ThreadPanelHeader properly renders Show: All threads 1`] = ` -
- +
- Threads - - - Show: All threads - -
+

+ Threads +

+ +
+ `; exports[`ThreadPanel Header expect that My filter for ThreadPanelHeader properly renders Show: My threads 1`] = ` -
- +
- Threads - - - Show: My threads - -
+

+ Threads +

+ +
+ `; exports[`ThreadPanel Header expect that ThreadPanelHeader has the correct option selected in the context menu 1`] = ` - - - - All threads - - - Shows all threads from current room - -
, - } - } - onClick={[Function]} - onFocus={[Function]} - role="menuitemradio" - tabIndex={0} - > -
- - All threads - - - Shows all threads from current room - -
- - + + All threads + + + Shows all threads from current room + + `; diff --git a/test/components/views/beta/BetaCard-test.tsx b/test/components/views/beta/BetaCard-test.tsx new file mode 100644 index 0000000000..6ba57f99a8 --- /dev/null +++ b/test/components/views/beta/BetaCard-test.tsx @@ -0,0 +1,80 @@ +/* +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 { mocked } from "jest-mock"; +import { render, screen } from "@testing-library/react"; + +import { shouldShowFeedback } from "../../../../src/utils/Feedback"; +import BetaCard from "../../../../src/components/views/beta/BetaCard"; +import SettingsStore from "../../../../src/settings/SettingsStore"; + +jest.mock("../../../../src/utils/Feedback"); +jest.mock("../../../../src/settings/SettingsStore"); + +describe('', () => { + describe("Feedback prompt", () => { + const featureId = "featureId"; + + beforeEach(() => { + mocked(SettingsStore).getBetaInfo.mockReturnValue({ + title: "title", + caption: () => "caption", + feedbackLabel: "feedbackLabel", + feedbackSubheading: "feedbackSubheading", + }); + mocked(SettingsStore).getValue.mockReturnValue(true); + mocked(shouldShowFeedback).mockReturnValue(true); + }); + + it("should show feedback prompt", () => { + render(); + expect(screen.queryByText("Feedback")).toBeTruthy(); + }); + + it("should not show feedback prompt if beta is disabled", () => { + mocked(SettingsStore).getValue.mockReturnValue(false); + render(); + expect(screen.queryByText("Feedback")).toBeFalsy(); + }); + + it("should not show feedback prompt if label is unset", () => { + mocked(SettingsStore).getBetaInfo.mockReturnValue({ + title: "title", + caption: () => "caption", + feedbackSubheading: "feedbackSubheading", + }); + render(); + expect(screen.queryByText("Feedback")).toBeFalsy(); + }); + + it("should not show feedback prompt if subheading is unset", () => { + mocked(SettingsStore).getBetaInfo.mockReturnValue({ + title: "title", + caption: () => "caption", + feedbackLabel: "feedbackLabel", + }); + render(); + expect(screen.queryByText("Feedback")).toBeFalsy(); + }); + + it("should not show feedback prompt if feedback is disabled", () => { + mocked(shouldShowFeedback).mockReturnValue(false); + render(); + expect(screen.queryByText("Feedback")).toBeFalsy(); + }); + }); +}); diff --git a/test/components/views/dialogs/SpotlightDialog-test.tsx b/test/components/views/dialogs/SpotlightDialog-test.tsx index 3bb16bd54f..b4c0ac2f7e 100644 --- a/test/components/views/dialogs/SpotlightDialog-test.tsx +++ b/test/components/views/dialogs/SpotlightDialog-test.tsx @@ -29,6 +29,9 @@ import { LocalRoom, LOCAL_ROOM_ID_PREFIX } from "../../../../src/models/LocalRoo import { DirectoryMember, startDmOnFirstMessage } from "../../../../src/utils/direct-messages"; import DMRoomMap from "../../../../src/utils/DMRoomMap"; import { mkRoom, stubClient } from "../../../test-utils"; +import { shouldShowFeedback } from "../../../../src/utils/Feedback"; + +jest.mock("../../../../src/utils/Feedback"); jest.mock("../../../../src/utils/direct-messages", () => ({ // @ts-ignore @@ -138,6 +141,7 @@ describe("Spotlight Dialog", () => { getUserIdForRoomId: jest.fn(), } as unknown as DMRoomMap); }); + describe("should apply filters supplied via props", () => { it("without filter", async () => { const wrapper = mount( @@ -370,4 +374,32 @@ describe("Spotlight Dialog", () => { wrapper.unmount(); }); + + describe("Feedback prompt", () => { + it("should show feedback prompt if feedback is enabled", async () => { + mocked(shouldShowFeedback).mockReturnValue(true); + + const wrapper = mount( null} />); + await act(async () => { + await sleep(200); + }); + wrapper.update(); + + const content = wrapper.find(".mx_SpotlightDialog_footer"); + expect(content.childAt(0).text()).toBe("Results not as expected? Please give feedback."); + }); + + it("should hide feedback prompt if feedback is disabled", async () => { + mocked(shouldShowFeedback).mockReturnValue(false); + + const wrapper = mount( null} />); + await act(async () => { + await sleep(200); + }); + wrapper.update(); + + const content = wrapper.find(".mx_SpotlightDialog_footer"); + expect(content.text()).toBeFalsy(); + }); + }); }); diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index 2022d73a9c..aaf8bd95de 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -444,6 +444,8 @@ export function mkStubRoom(roomId: string = null, name: string, client: MatrixCl canInvite: jest.fn(), getThreads: jest.fn().mockReturnValue([]), eventShouldLiveIn: jest.fn().mockReturnValue({}), + createThreadsTimelineSets: jest.fn().mockReturnValue(new Promise(() => {})), + fetchRoomThreads: jest.fn().mockReturnValue(new Promise(() => {})), } as unknown as Room; } diff --git a/test/utils/Feedback-test.ts b/test/utils/Feedback-test.ts new file mode 100644 index 0000000000..64868b7131 --- /dev/null +++ b/test/utils/Feedback-test.ts @@ -0,0 +1,46 @@ +/* +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 { mocked } from "jest-mock"; + +import SdkConfig from "../../src/SdkConfig"; +import { shouldShowFeedback } from "../../src/utils/Feedback"; +import SettingsStore from "../../src/settings/SettingsStore"; + +jest.mock("../../src/SdkConfig"); +jest.mock("../../src/settings/SettingsStore"); + +describe("shouldShowFeedback", () => { + it("should return false if bug_report_endpoint_url is falsey", () => { + mocked(SdkConfig).get.mockReturnValue({ + bug_report_endpoint_url: null, + }); + expect(shouldShowFeedback()).toBeFalsy(); + }); + + it("should return false if UIFeature.Feedback is disabled", () => { + mocked(SettingsStore).getValue.mockReturnValue(false); + expect(shouldShowFeedback()).toBeFalsy(); + }); + + it("should return true if bug_report_endpoint_url is set and UIFeature.Feedback is true", () => { + mocked(SdkConfig).get.mockReturnValue({ + bug_report_endpoint_url: "https://rageshake.server", + }); + mocked(SettingsStore).getValue.mockReturnValue(true); + expect(shouldShowFeedback()).toBeTruthy(); + }); +});