Don't show feedback prompts when that UIFeature is disabled (#9305)

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<IProps> = ({
         }
     }, [timelineSet, timelinePanel]);
 
-    const openFeedback = SdkConfig.get().bug_report_endpoint_url ? () => {
+    const openFeedback = shouldShowFeedback() ? () => {
         Modal.createDialog(BetaFeedbackDialog, {
             featureId: "feature_thread",
         });

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 = <AccessibleButton
             onClick={() => {
                 Modal.createDialog(BetaFeedbackDialog, { featureId });

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<IProps> = ({ initialText = "", initialFilter = n
         }
     };
 
-    const openFeedback = SdkConfig.get().bug_report_endpoint_url ? () => {
+    const openFeedback = shouldShowFeedback() ? () => {
         Modal.createDialog(FeedbackDialog, {
             feature: "spotlight",
         });

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 <div className="mx_SpaceFeedbackPrompt">
         <span className="mx_SpaceFeedbackPrompt_text">{ _t("Spaces are a new feature.") }</span>

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;
     }
 

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);
+}

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 { render, screen, fireEvent } from "@testing-library/react";
-import { shallow, mount } from "enzyme";
+import { mocked } from "jest-mock";
 import 'focus-visible'; // to fix context menus
 
-import {
+import ThreadPanel, { ThreadFilterType, ThreadPanelHeader } from '../../../src/components/structures/ThreadPanel';
-    ThreadFilterType,
-    ThreadPanelHeader,
-    ThreadPanelHeaderFilterOptionItem,
-} from '../../../src/components/structures/ThreadPanel';
-import { ContextMenuButton } from '../../../src/accessibility/context_menu/ContextMenuButton';
-import ContextMenu from '../../../src/components/structures/ContextMenu';
 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(<MatrixClientContext.Provider value={cli}>
+                <ThreadPanel
+                    roomId="!room:server"
+                    onClose={jest.fn()}
+                    resizeNotifier={new ResizeNotifier()}
+                    permalinkCreator={new RoomPermalinkCreator(room)}
+                />
+            </MatrixClientContext.Provider>);
+            expect(screen.queryByText("Give feedback")).toBeTruthy();
+        });
+
+        it("should hide feedback prompt if feedback is disabled", () => {
+            mocked(shouldShowFeedback).mockReturnValue(false);
+
+            render(<MatrixClientContext.Provider value={cli}>
+                <ThreadPanel
+                    roomId="!room:server"
+                    onClose={jest.fn()}
+                    resizeNotifier={new ResizeNotifier()}
+                    permalinkCreator={new RoomPermalinkCreator(room)}
+                />
+            </MatrixClientContext.Provider>);
+            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(
                 <ThreadPanelHeader
                     empty={false}
                     filterOption={ThreadFilterType.All}
                     setFilterOption={() => 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(
                 <ThreadPanelHeader
                     empty={false}
                     filterOption={ThreadFilterType.My}
                     setFilterOption={() => 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(
                 <ThreadPanelHeader
                     empty={false}
                     filterOption={ThreadFilterType.All}
                     setFilterOption={() => undefined} />,
             );
-            const found = wrapper.find(ContextMenuButton);
+            const found = container.querySelector(".mx_ThreadPanel_dropdown");
-            expect(found).not.toBe(undefined);
+            expect(found).toBeTruthy();
-            expect(found).not.toBe(null);
+            expect(screen.queryByRole("menu")).toBeFalsy();
-            expect(wrapper.exists(ContextMenu)).toEqual(false);
+            fireEvent.click(found);
-            found.simulate('click');
+            expect(screen.queryByRole("menu")).toBeTruthy();
-            expect(wrapper.exists(ContextMenu)).toEqual(true);
         });
 
         it('expect that ThreadPanelHeader has the correct option selected in the context menu', () => {
-            const wrapper = mount(
+            const { container } = render(
                 <ThreadPanelHeader
                     empty={false}
                     filterOption={ThreadFilterType.All}
                     setFilterOption={() => undefined} />,
             );
-            wrapper.find(ContextMenuButton).simulate('click');
+            fireEvent.click(container.querySelector(".mx_ThreadPanel_dropdown"));
-            const found = wrapper.find(ThreadPanelHeaderFilterOptionItem);
+            const found = screen.queryAllByRole("menuitemradio");
-            expect(found.length).toEqual(2);
+            expect(found).toHaveLength(2);
-            const foundButton = found.find('[aria-checked=true]').first();
+            const foundButton = screen.queryByRole("menuitemradio", { checked: true });
-            expect(foundButton.text()).toEqual(`${_t("All threads")}${_t('Shows all threads from current room')}`);
+            expect(foundButton.textContent).toEqual(`${_t("All threads")}${_t('Shows all threads from current room')}`);
             expect(foundButton).toMatchSnapshot();
         });
     });

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`] = `
-<div
+<DocumentFragment>
-  className="mx_BaseCard_header_title"
+  <div
->
+    class="mx_BaseCard_header_title"
-  <Heading
-    className="mx_BaseCard_header_title_heading"
-    size="h4"
   >
-    Threads
+    <h4
-  </Heading>
+      class="mx_Heading_h4 mx_BaseCard_header_title_heading"
-  <ContextMenuButton
+    >
-    className="mx_ThreadPanel_dropdown"
+      Threads
-    inputRef={
+    </h4>
-      Object {
+    <div
-        "current": null,
+      aria-expanded="false"
-      }
+      aria-haspopup="true"
-    }
+      class="mx_AccessibleButton mx_ThreadPanel_dropdown"
-    isExpanded={false}
+      role="button"
-    onClick={[Function]}
+      tabindex="0"
-  >
+    >
-    Show: All threads
+      Show: All threads
-  </ContextMenuButton>
+    </div>
-</div>
+  </div>
+</DocumentFragment>
 `;
 
 exports[`ThreadPanel Header expect that My filter for ThreadPanelHeader properly renders Show: My threads 1`] = `
-<div
+<DocumentFragment>
-  className="mx_BaseCard_header_title"
+  <div
->
+    class="mx_BaseCard_header_title"
-  <Heading
-    className="mx_BaseCard_header_title_heading"
-    size="h4"
   >
-    Threads
+    <h4
-  </Heading>
+      class="mx_Heading_h4 mx_BaseCard_header_title_heading"
-  <ContextMenuButton
+    >
-    className="mx_ThreadPanel_dropdown"
+      Threads
-    inputRef={
+    </h4>
-      Object {
+    <div
-        "current": null,
+      aria-expanded="false"
-      }
+      aria-haspopup="true"
-    }
+      class="mx_AccessibleButton mx_ThreadPanel_dropdown"
-    isExpanded={false}
+      role="button"
-    onClick={[Function]}
+      tabindex="0"
-  >
+    >
-    Show: My threads
+      Show: My threads
-  </ContextMenuButton>
+    </div>
-</div>
+  </div>
+</DocumentFragment>
 `;
 
 exports[`ThreadPanel Header expect that ThreadPanelHeader has the correct option selected in the context menu 1`] = `
-<RovingAccessibleButton
+<div
-  aria-checked={true}
+  aria-checked="true"
-  className="mx_ThreadPanel_Header_FilterOptionItem"
+  class="mx_AccessibleButton mx_ThreadPanel_Header_FilterOptionItem focus-visible"
-  onClick={[Function]}
+  data-focus-visible-added=""
   role="menuitemradio"
+  tabindex="0"
 >
-  <AccessibleButton
+  <span>
-    aria-checked={true}
+    All threads
-    className="mx_ThreadPanel_Header_FilterOptionItem"
+  </span>
-    element="div"
+  <span>
-    inputRef={
+    Shows all threads from current room
-      Object {
+  </span>
-        "current": <div
+</div>
-          aria-checked="true"
-          class="mx_AccessibleButton mx_ThreadPanel_Header_FilterOptionItem focus-visible"
-          data-focus-visible-added=""
-          role="menuitemradio"
-          tabindex="0"
-        >
-          <span>
-            All threads
-          </span>
-          <span>
-            Shows all threads from current room
-          </span>
-        </div>,
-      }
-    }
-    onClick={[Function]}
-    onFocus={[Function]}
-    role="menuitemradio"
-    tabIndex={0}
-  >
-    <div
-      aria-checked={true}
-      className="mx_AccessibleButton mx_ThreadPanel_Header_FilterOptionItem"
-      onClick={[Function]}
-      onFocus={[Function]}
-      onKeyDown={[Function]}
-      onKeyUp={[Function]}
-      role="menuitemradio"
-      tabIndex={0}
-    >
-      <span>
-        All threads
-      </span>
-      <span>
-        Shows all threads from current room
-      </span>
-    </div>
-  </AccessibleButton>
-</RovingAccessibleButton>
 `;

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('<BetaCard />', () => {
+    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(<BetaCard featureId={featureId} />);
+            expect(screen.queryByText("Feedback")).toBeTruthy();
+        });
+
+        it("should not show feedback prompt if beta is disabled", () => {
+            mocked(SettingsStore).getValue.mockReturnValue(false);
+            render(<BetaCard featureId={featureId} />);
+            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(<BetaCard featureId={featureId} />);
+            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(<BetaCard featureId={featureId} />);
+            expect(screen.queryByText("Feedback")).toBeFalsy();
+        });
+
+        it("should not show feedback prompt if feedback is disabled", () => {
+            mocked(shouldShowFeedback).mockReturnValue(false);
+            render(<BetaCard featureId={featureId} />);
+            expect(screen.queryByText("Feedback")).toBeFalsy();
+        });
+    });
+});

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(<SpotlightDialog initialText="test23" onFinished={() => 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(<SpotlightDialog initialText="test23" onFinished={() => null} />);
+            await act(async () => {
+                await sleep(200);
+            });
+            wrapper.update();
+
+            const content = wrapper.find(".mx_SpotlightDialog_footer");
+            expect(content.text()).toBeFalsy();
+        });
+    });
 });

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;
 }
 

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();
+    });
+});