Use semantic headings in user settings - integrations and account deletion (#10837)

* allow testids in settings sections

* use semantic headings in LabsUserSettingsTab

* put back margin var

* use SettingsTab wrapper

* use semantic headings for deactivate acc section

* use semantic heading in manage integratios

* i18n

* explicit cast to boolean

* Update src/components/views/settings/shared/SettingsSubsection.tsx

Co-authored-by: Richard van der Hoff <[email protected]>

* test manage integration settings

* test deactivate account section display

* remove debug

* fix cypress test

---------

Co-authored-by: Richard van der Hoff <[email protected]>
(cherry picked from commit 8cd84b0)

Author: 2 people

cypress/e2e/settings/general-user-settings-tab.spec.ts

@@ -43,15 +43,15 @@ describe("General user settings tab", () => {
         // Exclude userId from snapshots
         const percyCSS = ".mx_ProfileSettings_profile_controls_userId { visibility: hidden !important; }";
 
-        cy.get(".mx_SettingsTab.mx_GeneralUserSettingsTab").percySnapshotElement("User settings tab - General", {
+        cy.findByTestId("mx_GeneralUserSettingsTab").percySnapshotElement("User settings tab - General", {
             percyCSS,
             // Emulate TabbedView's actual min and max widths
             // 580: '.mx_UserSettingsDialog .mx_TabbedView' min-width
             // 796: 1036 (mx_TabbedView_tabsOnLeft actual width) - 240 (mx_TabbedView_tabPanel margin-right)
             widths: [580, 796],
         });
 
-        cy.get(".mx_SettingsTab.mx_GeneralUserSettingsTab").within(() => {
+        cy.findByTestId("mx_GeneralUserSettingsTab").within(() => {
             // Assert that the top heading is rendered
             cy.findByTestId("general").should("have.text", "General").should("be.visible");
 
@@ -156,16 +156,10 @@ describe("General user settings tab", () => {
                     // Make sure integration manager's toggle switch is enabled
                     cy.get(".mx_ToggleSwitch_enabled").should("be.visible");
 
-                    // Assert space between "Manage integrations" and the integration server address is set to 4px;
-                    cy.get(".mx_SetIntegrationManager_heading_manager").should("have.css", "column-gap", "4px");
-
-                    cy.get(".mx_SetIntegrationManager_heading_manager").within(() => {
-                        cy.get(".mx_SettingsTab_heading").should("have.text", "Manage integrations");
-
-                        // Assert the headings' inline end margin values are set to zero in favor of the column-gap declaration
-                        cy.get(".mx_SettingsTab_heading").should("have.css", "margin-inline-end", "0px");
-                        cy.get(".mx_SettingsTab_subheading").should("have.css", "margin-inline-end", "0px");
-                    });
+                    cy.get(".mx_SetIntegrationManager_heading_manager").should(
+                        "have.text",
+                        "Manage integrations(scalar.vector.im)",
+                    );
                 });
 
             // Assert the account deactivation button is displayed
@@ -178,7 +172,7 @@ describe("General user settings tab", () => {
     });
 
     it("should support adding and removing a profile picture", () => {
-        cy.get(".mx_SettingsTab.mx_GeneralUserSettingsTab .mx_ProfileSettings").within(() => {
+        cy.get(".mx_SettingsTab .mx_ProfileSettings").within(() => {
             // Upload a picture
             cy.get(".mx_ProfileSettings_avatarUpload").selectFile("cypress/fixtures/riot.png", { force: true });
 
@@ -225,7 +219,7 @@ describe("General user settings tab", () => {
     });
 
     it("should support changing a display name", () => {
-        cy.get(".mx_SettingsTab.mx_GeneralUserSettingsTab .mx_ProfileSettings").within(() => {
+        cy.get(".mx_SettingsTab .mx_ProfileSettings").within(() => {
             // Change the diaplay name to USER_NAME_NEW
             cy.findByRole("textbox", { name: "Display Name" }).type(`{selectAll}{del}${USER_NAME_NEW}{enter}`);
         });

res/css/views/settings/_SetIntegrationManager.pcss

@@ -17,20 +17,12 @@ limitations under the License.
 .mx_SetIntegrationManager {
     .mx_SettingsFlag {
         align-items: center;
-        margin-top: var(--SettingsTab_heading_nth_child-margin-top);
 
         .mx_SetIntegrationManager_heading_manager {
             display: flex;
             align-items: center;
             flex-wrap: wrap;
             column-gap: $spacing-4;
-
-            .mx_SettingsTab_heading,
-            .mx_SettingsTab_subheading {
-                margin-top: 0;
-                margin-bottom: 0;
-                margin-inline-end: 0; /* Cancel the default right (inline-end) margin */
-            }
         }
 
         .mx_ToggleSwitch {

src/components/views/settings/SetIntegrationManager.tsx

@@ -23,6 +23,8 @@ import { IntegrationManagerInstance } from "../../../integrations/IntegrationMan
 import SettingsStore from "../../../settings/SettingsStore";
 import { SettingLevel } from "../../../settings/SettingLevel";
 import ToggleSwitch from "../elements/ToggleSwitch";
+import Heading from "../typography/Heading";
+import { SettingsSubsectionText } from "./shared/SettingsSubsection";
 
 interface IProps {}
 
@@ -70,11 +72,15 @@ export default class SetIntegrationManager extends React.Component<IProps, IStat
         }
 
         return (
-            <label className="mx_SetIntegrationManager" htmlFor="toggle_integration">
+            <label
+                className="mx_SetIntegrationManager"
+                data-testid="mx_SetIntegrationManager"
+                htmlFor="toggle_integration"
+            >
                 <div className="mx_SettingsFlag">
                     <div className="mx_SetIntegrationManager_heading_manager">
-                        <span className="mx_SettingsTab_heading">{_t("Manage integrations")}</span>
-                        <span className="mx_SettingsTab_subheading">{managerName}</span>
+                        <Heading size="h2">{_t("Manage integrations")}</Heading>
+                        <Heading size="h3">{managerName}</Heading>
                     </div>
                     <ToggleSwitch
                         id="toggle_integration"
@@ -83,13 +89,13 @@ export default class SetIntegrationManager extends React.Component<IProps, IStat
                         onChange={this.onProvisioningToggled}
                     />
                 </div>
-                <div className="mx_SettingsTab_subsectionText">{bodyText}</div>
-                <div className="mx_SettingsTab_subsectionText">
+                <SettingsSubsectionText>{bodyText}</SettingsSubsectionText>
+                <SettingsSubsectionText>
                     {_t(
                         "Integration managers receive configuration data, and can modify widgets, " +
                             "send room invites, and set power levels on your behalf.",
                     )}
-                </div>
+                </SettingsSubsectionText>
             </label>
         );
     }

src/components/views/settings/tabs/user/GeneralUserSettingsTab.tsx

@@ -54,6 +54,9 @@ import SetIdServer from "../../SetIdServer";
 import SetIntegrationManager from "../../SetIntegrationManager";
 import ToggleSwitch from "../../../elements/ToggleSwitch";
 import { IS_MAC } from "../../../../../Keyboard";
+import SettingsTab from "../SettingsTab";
+import { SettingsSection } from "../../shared/SettingsSection";
+import SettingsSubsection from "../../shared/SettingsSubsection";
 
 interface IProps {
     closeSettingsFn: () => void;
@@ -492,27 +495,24 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
     private renderManagementSection(): JSX.Element {
         // TODO: Improve warning text for account deactivation
         return (
-            <div className="mx_SettingsTab_section" data-testid="account-management-section">
-                <span className="mx_SettingsTab_subheading">{_t("Account management")}</span>
-                <span className="mx_SettingsTab_subsectionText">
-                    {_t("Deactivating your account is a permanent action — be careful!")}
-                </span>
-                <AccessibleButton onClick={this.onDeactivateClicked} kind="danger">
-                    {_t("Deactivate Account")}
-                </AccessibleButton>
-            </div>
+            <SettingsSection heading={_t("Deactivate account")}>
+                <SettingsSubsection
+                    heading={_t("Account management")}
+                    data-testid="account-management-section"
+                    description={_t("Deactivating your account is a permanent action — be careful!")}
+                >
+                    <AccessibleButton onClick={this.onDeactivateClicked} kind="danger">
+                        {_t("Deactivate Account")}
+                    </AccessibleButton>
+                </SettingsSubsection>
+            </SettingsSection>
         );
     }
 
     private renderIntegrationManagerSection(): ReactNode {
         if (!SettingsStore.getValue(UIFeature.Widgets)) return null;
 
-        return (
-            <div className="mx_SettingsTab_section">
-                {/* has its own heading as it includes the current integration manager */}
-                <SetIntegrationManager />
-            </div>
-        );
+        return <SetIntegrationManager />;
     }
 
     public render(): React.ReactNode {
@@ -531,12 +531,7 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
 
         let accountManagementSection: JSX.Element | undefined;
         if (SettingsStore.getValue(UIFeature.Deactivate)) {
-            accountManagementSection = (
-                <>
-                    <div className="mx_SettingsTab_heading">{_t("Deactivate account")}</div>
-                    {this.renderManagementSection()}
-                </>
-            );
+            accountManagementSection = this.renderManagementSection();
         }
 
         let discoverySection;
@@ -552,7 +547,7 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
         }
 
         return (
-            <div className="mx_SettingsTab mx_GeneralUserSettingsTab">
+            <SettingsTab data-testid="mx_GeneralUserSettingsTab">
                 <div className="mx_SettingsTab_heading" data-testid="general">
                     {_t("General")}
                 </div>
@@ -561,9 +556,9 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
                 {this.renderLanguageSection()}
                 {supportsMultiLanguageSpellCheck ? this.renderSpellCheckSection() : null}
                 {discoverySection}
-                {this.renderIntegrationManagerSection() /* Has its own title */}
+                {this.renderIntegrationManagerSection()}
                 {accountManagementSection}
-            </div>
+            </SettingsTab>
         );
     }
 }

src/i18n/strings/en_EN.json

@@ -1568,10 +1568,10 @@
     "Language and region": "Language and region",
     "Spell check": "Spell check",
     "Agree to the identity server (%(serverName)s) Terms of Service to allow yourself to be discoverable by email address or phone number.": "Agree to the identity server (%(serverName)s) Terms of Service to allow yourself to be discoverable by email address or phone number.",
+    "Deactivate account": "Deactivate account",
     "Account management": "Account management",
     "Deactivating your account is a permanent action — be careful!": "Deactivating your account is a permanent action — be careful!",
     "Deactivate Account": "Deactivate Account",
-    "Deactivate account": "Deactivate account",
     "Discovery": "Discovery",
     "%(brand)s version:": "%(brand)s version:",
     "Olm version:": "Olm version:",

test/components/views/settings/tabs/user/GeneralUserSettingsTab-test.tsx

@@ -10,9 +10,11 @@ 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 { render } from "@testing-library/react";
+
+import { fireEvent, render, screen, within } from "@testing-library/react";
 import React from "react";
 import { M_AUTHENTICATION } from "matrix-js-sdk/src/matrix";
+import { logger } from "matrix-js-sdk/src/logger";
 
 import GeneralUserSettingsTab from "../../../../../../src/components/views/settings/tabs/user/GeneralUserSettingsTab";
 import MatrixClientContext from "../../../../../../src/contexts/MatrixClientContext";
@@ -24,6 +26,8 @@ import {
     mockPlatformPeg,
     flushPromises,
 } from "../../../../../test-utils";
+import { UIFeature } from "../../../../../../src/settings/UIFeature";
+import { SettingLevel } from "../../../../../../src/settings/SettingLevel";
 
 describe("<GeneralUserSettingsTab />", () => {
     const defaultProps = {
@@ -49,6 +53,8 @@ describe("<GeneralUserSettingsTab />", () => {
         mockPlatformPeg();
         jest.clearAllMocks();
         clientWellKnownSpy.mockReturnValue({});
+        jest.spyOn(SettingsStore, "getValue").mockRestore();
+        jest.spyOn(logger, "error").mockRestore();
     });
 
     it("does not show account management link when not available", () => {
@@ -74,4 +80,83 @@ describe("<GeneralUserSettingsTab />", () => {
         expect(getByTestId("external-account-management-outer").textContent).toMatch(/.*id\.server\.org/);
         expect(getByTestId("external-account-management-link").getAttribute("href")).toMatch(accountManagementLink);
     });
+
+    describe("Manage integrations", () => {
+        it("should not render manage integrations section when widgets feature is disabled", () => {
+            jest.spyOn(SettingsStore, "getValue").mockImplementation(
+                (settingName) => settingName !== UIFeature.Widgets,
+            );
+            render(getComponent());
+
+            expect(screen.queryByTestId("mx_SetIntegrationManager")).not.toBeInTheDocument();
+            expect(SettingsStore.getValue).toHaveBeenCalledWith(UIFeature.Widgets);
+        });
+        it("should render manage integrations sections", () => {
+            jest.spyOn(SettingsStore, "getValue").mockImplementation(
+                (settingName) => settingName === UIFeature.Widgets,
+            );
+
+            render(getComponent());
+
+            expect(screen.getByTestId("mx_SetIntegrationManager")).toMatchSnapshot();
+        });
+        it("should update integrations provisioning on toggle", () => {
+            jest.spyOn(SettingsStore, "getValue").mockImplementation(
+                (settingName) => settingName === UIFeature.Widgets,
+            );
+            jest.spyOn(SettingsStore, "setValue").mockResolvedValue(undefined);
+
+            render(getComponent());
+
+            const integrationSection = screen.getByTestId("mx_SetIntegrationManager");
+            fireEvent.click(within(integrationSection).getByRole("switch"));
+
+            expect(SettingsStore.setValue).toHaveBeenCalledWith(
+                "integrationProvisioning",
+                null,
+                SettingLevel.ACCOUNT,
+                true,
+            );
+            expect(within(integrationSection).getByRole("switch")).toBeChecked();
+        });
+        it("handles error when updating setting fails", async () => {
+            jest.spyOn(SettingsStore, "getValue").mockImplementation(
+                (settingName) => settingName === UIFeature.Widgets,
+            );
+            jest.spyOn(logger, "error").mockImplementation(() => {});
+
+            jest.spyOn(SettingsStore, "setValue").mockRejectedValue("oups");
+
+            render(getComponent());
+
+            const integrationSection = screen.getByTestId("mx_SetIntegrationManager");
+            fireEvent.click(within(integrationSection).getByRole("switch"));
+
+            await flushPromises();
+
+            expect(logger.error).toHaveBeenCalledWith("Error changing integration manager provisioning");
+            expect(logger.error).toHaveBeenCalledWith("oups");
+            expect(within(integrationSection).getByRole("switch")).not.toBeChecked();
+        });
+    });
+
+    describe("deactive account", () => {
+        it("should not render section when account deactivation feature is disabled", () => {
+            jest.spyOn(SettingsStore, "getValue").mockImplementation(
+                (settingName) => settingName !== UIFeature.Deactivate,
+            );
+            render(getComponent());
+
+            expect(screen.queryByText("Deactivate account")).not.toBeInTheDocument();
+            expect(SettingsStore.getValue).toHaveBeenCalledWith(UIFeature.Deactivate);
+        });
+        it("should render section when account deactivation feature is enabled", () => {
+            jest.spyOn(SettingsStore, "getValue").mockImplementation(
+                (settingName) => settingName === UIFeature.Deactivate,
+            );
+            render(getComponent());
+
+            expect(screen.getByText("Deactivate account").parentElement!).toMatchSnapshot();
+        });
+    });
 });