Split out email & phone number settings to separate components & move discovery to privacy tab (#12670)

* WIP update of threepid settings section

* Remove email / phone number section from original place

and don't show the new one if 3pids are disabled

* Update snapshots

* Pull identity server / 3pid binding settings out to separate component

and put it in the security & privacy section which is its new home

* Update snapshot

* Move relevant part of test & update screenshots / snapshots

* Remove unnecessary dependency

* Add test for discovery settings

* Add spacing in terms agreement

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

@@ -91,11 +91,6 @@ test.describe("General user settings tab", () => {
         // Assert that the default value is rendered again
         await expect(languageInput.getByText("English")).toBeVisible();
 
-        const setIdServer = uut.locator(".mx_SetIdServer");
-        await setIdServer.scrollIntoViewIfNeeded();
-        // Assert that an input area for identity server exists
-        await expect(setIdServer.getByRole("textbox", { name: "Enter a new identity server" })).toBeVisible();
-
         const setIntegrationManager = uut.locator(".mx_SetIntegrationManager");
         await setIntegrationManager.scrollIntoViewIfNeeded();
         await expect(

playwright/e2e/settings/security-user-settings-tab.spec.ts

@@ -47,5 +47,14 @@ test.describe("Security user settings tab", () => {
                 await expect(page.locator(".mx_AnalyticsLearnMoreDialog_wrapper .mx_Dialog")).toMatchScreenshot();
             });
         });
+
+        test("should contain section to set ID server", async ({ app }) => {
+            const tab = await app.settings.openUserSettings("Security");
+
+            const setIdServer = tab.locator(".mx_SetIdServer");
+            await setIdServer.scrollIntoViewIfNeeded();
+            // Assert that an input area for identity server exists
+            await expect(setIdServer.getByRole("textbox", { name: "Enter a new identity server" })).toBeVisible();
+        });
     });
 });

res/css/views/terms/_InlineTermsAgreement.pcss

@@ -15,6 +15,7 @@ limitations under the License.
 */
 
 .mx_InlineTermsAgreement_cbContainer {
+    margin-top: var(--cpd-space-4x);
     margin-bottom: 10px;
     font: var(--cpd-font-body-md-regular);
 

src/components/views/settings/UserPersonalInfoSettings.tsx

@@ -0,0 +1,130 @@
+/*
+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, { useCallback, useEffect, useState } from "react";
+import { ThreepidMedium } from "matrix-js-sdk/src/matrix";
+import { Alert } from "@vector-im/compound-web";
+
+import AccountEmailAddresses from "./account/EmailAddresses";
+import AccountPhoneNumbers from "./account/PhoneNumbers";
+import { _t } from "../../../languageHandler";
+import InlineSpinner from "../elements/InlineSpinner";
+import SettingsSubsection from "./shared/SettingsSubsection";
+import { useMatrixClientContext } from "../../../contexts/MatrixClientContext";
+import { ThirdPartyIdentifier } from "../../../AddThreepid";
+import SettingsStore from "../../../settings/SettingsStore";
+import { UIFeature } from "../../../settings/UIFeature";
+
+type LoadingState = "loading" | "loaded" | "error";
+
+interface ThreepidSectionWrapperProps {
+    error: string;
+    loadingState: LoadingState;
+    children: React.ReactNode;
+}
+
+const ThreepidSectionWrapper: React.FC<ThreepidSectionWrapperProps> = ({ error, loadingState, children }) => {
+    if (loadingState === "loading") {
+        return <InlineSpinner />;
+    } else if (loadingState === "error") {
+        return (
+            <Alert type="critical" title={_t("common|error")}>
+                {error}
+            </Alert>
+        );
+    } else {
+        return <>{children}</>;
+    }
+};
+
+interface UserPersonalInfoSettingsProps {
+    canMake3pidChanges: boolean;
+}
+
+/**
+ * Settings controls allowing the user to set personal information like email addresses.
+ */
+export const UserPersonalInfoSettings: React.FC<UserPersonalInfoSettingsProps> = ({ canMake3pidChanges }) => {
+    const [emails, setEmails] = useState<ThirdPartyIdentifier[] | undefined>();
+    const [phoneNumbers, setPhoneNumbers] = useState<ThirdPartyIdentifier[] | undefined>();
+    const [loadingState, setLoadingState] = useState<"loading" | "loaded" | "error">("loading");
+
+    const client = useMatrixClientContext();
+
+    useEffect(() => {
+        (async () => {
+            try {
+                const threepids = await client.getThreePids();
+                setEmails(threepids.threepids.filter((a) => a.medium === ThreepidMedium.Email));
+                setPhoneNumbers(threepids.threepids.filter((a) => a.medium === ThreepidMedium.Phone));
+                setLoadingState("loaded");
+            } catch (e) {
+                setLoadingState("error");
+            }
+        })();
+    }, [client]);
+
+    const onEmailsChange = useCallback((emails: ThirdPartyIdentifier[]) => {
+        setEmails(emails);
+    }, []);
+
+    const onMsisdnsChange = useCallback((msisdns: ThirdPartyIdentifier[]) => {
+        setPhoneNumbers(msisdns);
+    }, []);
+
+    if (!SettingsStore.getValue(UIFeature.ThirdPartyID)) return null;
+
+    return (
+        <div>
+            <h2>{_t("settings|general|personal_info")}</h2>
+            <SettingsSubsection
+                heading={_t("settings|general|emails_heading")}
+                stretchContent
+                data-testid="mx_AccountEmailAddresses"
+            >
+                <ThreepidSectionWrapper
+                    error={_t("settings|general|unable_to_load_emails")}
+                    loadingState={loadingState}
+                >
+                    <AccountEmailAddresses
+                        emails={emails!}
+                        onEmailsChange={onEmailsChange}
+                        disabled={!canMake3pidChanges}
+                    />
+                </ThreepidSectionWrapper>
+            </SettingsSubsection>
+
+            <SettingsSubsection
+                heading={_t("settings|general|msisdns_heading")}
+                stretchContent
+                data-testid="mx_AccountPhoneNumbers"
+            >
+                <ThreepidSectionWrapper
+                    error={_t("settings|general|unable_to_load_msisdns")}
+                    loadingState={loadingState}
+                >
+                    <AccountPhoneNumbers
+                        msisdns={phoneNumbers!}
+                        onMsisdnsChange={onMsisdnsChange}
+                        disabled={!canMake3pidChanges}
+                    />
+                </ThreepidSectionWrapper>
+            </SettingsSubsection>
+        </div>
+    );
+};
+
+export default UserPersonalInfoSettings;

src/components/views/settings/discovery/DiscoverySettings.tsx

@@ -0,0 +1,190 @@
+/*
+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, { useCallback, useEffect, useState } from "react";
+import { SERVICE_TYPES, ThreepidMedium } from "matrix-js-sdk/src/matrix";
+import { logger } from "matrix-js-sdk/src/logger";
+import { Alert } from "@vector-im/compound-web";
+
+import DiscoveryEmailAddresses from "../discovery/EmailAddresses";
+import DiscoveryPhoneNumbers from "../discovery/PhoneNumbers";
+import { getThreepidsWithBindStatus } from "../../../../boundThreepids";
+import { useMatrixClientContext } from "../../../../contexts/MatrixClientContext";
+import { ThirdPartyIdentifier } from "../../../../AddThreepid";
+import SettingsStore from "../../../../settings/SettingsStore";
+import { UIFeature } from "../../../../settings/UIFeature";
+import { _t } from "../../../../languageHandler";
+import SetIdServer from "../SetIdServer";
+import SettingsSubsection from "../shared/SettingsSubsection";
+import InlineTermsAgreement from "../../terms/InlineTermsAgreement";
+import { Service, ServicePolicyPair, startTermsFlow } from "../../../../Terms";
+import IdentityAuthClient from "../../../../IdentityAuthClient";
+import { abbreviateUrl } from "../../../../utils/UrlUtils";
+import { useDispatcher } from "../../../../hooks/useDispatcher";
+import defaultDispatcher from "../../../../dispatcher/dispatcher";
+import { ActionPayload } from "../../../../dispatcher/payloads";
+
+type RequiredPolicyInfo =
+    | {
+          // This object is passed along to a component for handling
+          policiesAndServices: null; // From the startTermsFlow callback
+          agreedUrls: null; // From the startTermsFlow callback
+          resolve: null; // Promise resolve function for startTermsFlow callback
+      }
+    | {
+          policiesAndServices: ServicePolicyPair[];
+          agreedUrls: string[];
+          resolve: (values: string[]) => void;
+      };
+
+/**
+ * Settings controlling how a user's email addreses and phone numbers can be used to discover them
+ */
+export const DiscoverySettings: React.FC = () => {
+    const client = useMatrixClientContext();
+
+    const [emails, setEmails] = useState<ThirdPartyIdentifier[]>([]);
+    const [phoneNumbers, setPhoneNumbers] = useState<ThirdPartyIdentifier[]>([]);
+    const [loadingState, setLoadingState] = useState<"loading" | "loaded" | "error">("loading");
+    const [idServerName, setIdServerName] = useState<string | undefined>(abbreviateUrl(client.getIdentityServerUrl()));
+    const [canMake3pidChanges, setCanMake3pidChanges] = useState<boolean>(false);
+
+    const [requiredPolicyInfo, setRequiredPolicyInfo] = useState<RequiredPolicyInfo>({
+        // This object is passed along to a component for handling
+        policiesAndServices: null, // From the startTermsFlow callback
+        agreedUrls: null, // From the startTermsFlow callback
+        resolve: null, // Promise resolve function for startTermsFlow callback
+    });
+    const [hasTerms, setHasTerms] = useState<boolean>(false);
+
+    const getThreepidState = useCallback(async () => {
+        const threepids = await getThreepidsWithBindStatus(client);
+        setEmails(threepids.filter((a) => a.medium === ThreepidMedium.Email));
+        setPhoneNumbers(threepids.filter((a) => a.medium === ThreepidMedium.Phone));
+    }, [client]);
+
+    useDispatcher(
+        defaultDispatcher,
+        useCallback(
+            (payload: ActionPayload) => {
+                if (payload.action === "id_server_changed") {
+                    setIdServerName(abbreviateUrl(client.getIdentityServerUrl()));
+
+                    getThreepidState().then();
+                }
+            },
+            [client, getThreepidState],
+        ),
+    );
+
+    useEffect(() => {
+        (async () => {
+            try {
+                await getThreepidState();
+
+                const capabilities = await client.getCapabilities();
+                setCanMake3pidChanges(
+                    !capabilities["m.3pid_changes"] || capabilities["m.3pid_changes"].enabled === true,
+                );
+
+                // By starting the terms flow we get the logic for checking which terms the user has signed
+                // for free. So we might as well use that for our own purposes.
+                const idServerUrl = client.getIdentityServerUrl();
+                if (!idServerUrl) {
+                    return;
+                }
+
+                const authClient = new IdentityAuthClient();
+                try {
+                    const idAccessToken = await authClient.getAccessToken({ check: false });
+                    await startTermsFlow(
+                        client,
+                        [new Service(SERVICE_TYPES.IS, idServerUrl, idAccessToken!)],
+                        (policiesAndServices, agreedUrls, extraClassNames) => {
+                            return new Promise((resolve) => {
+                                setIdServerName(abbreviateUrl(idServerUrl));
+                                setHasTerms(true);
+                                setRequiredPolicyInfo({
+                                    policiesAndServices,
+                                    agreedUrls,
+                                    resolve,
+                                });
+                            });
+                        },
+                    );
+                    // User accepted all terms
+                    setHasTerms(false);
+                } catch (e) {
+                    logger.warn(
+                        `Unable to reach identity server at ${idServerUrl} to check ` + `for terms in Settings`,
+                    );
+                    logger.warn(e);
+                }
+
+                setLoadingState("loaded");
+            } catch (e) {
+                setLoadingState("error");
+            }
+        })();
+    }, [client, getThreepidState]);
+
+    if (!SettingsStore.getValue(UIFeature.ThirdPartyID)) return null;
+
+    if (hasTerms && requiredPolicyInfo.policiesAndServices) {
+        const intro = (
+            <Alert type="info" title={_t("settings|general|discovery_needs_terms_title")}>
+                {_t("settings|general|discovery_needs_terms", { serverName: idServerName })}
+            </Alert>
+        );
+        return (
+            <>
+                <InlineTermsAgreement
+                    policiesAndServicePairs={requiredPolicyInfo.policiesAndServices}
+                    agreedUrls={requiredPolicyInfo.agreedUrls}
+                    onFinished={requiredPolicyInfo.resolve}
+                    introElement={intro}
+                />
+                {/* has its own heading as it includes the current identity server */}
+                <SetIdServer missingTerms={true} />
+            </>
+        );
+    }
+
+    const threepidSection = idServerName ? (
+        <>
+            <DiscoveryEmailAddresses
+                emails={emails}
+                isLoading={loadingState === "loading"}
+                disabled={!canMake3pidChanges}
+            />
+            <DiscoveryPhoneNumbers
+                msisdns={phoneNumbers}
+                isLoading={loadingState === "loading"}
+                disabled={!canMake3pidChanges}
+            />
+        </>
+    ) : null;
+
+    return (
+        <SettingsSubsection heading={_t("settings|discovery|title")} data-testid="discoverySection">
+            {threepidSection}
+            {/* has its own heading as it includes the current identity server */}
+            <SetIdServer missingTerms={false} />
+        </SettingsSubsection>
+    );
+};
+
+export default DiscoverySettings;

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

@@ -17,10 +17,9 @@ limitations under the License.
 */
 
 import React, { ReactNode } from "react";
-import { SERVICE_TYPES, HTTPError, IThreepid, ThreepidMedium } from "matrix-js-sdk/src/matrix";
+import { HTTPError } from "matrix-js-sdk/src/matrix";
 import { logger } from "matrix-js-sdk/src/logger";
 
-import { Icon as WarningIcon } from "../../../../../../res/img/feather-customised/warning-triangle.svg";
 import { UserFriendlyError, _t } from "../../../../../languageHandler";
 import UserProfileSettings from "../../UserProfileSettings";
 import * as languageHandler from "../../../../../languageHandler";
@@ -31,22 +30,10 @@ import AccessibleButton from "../../../elements/AccessibleButton";
 import DeactivateAccountDialog from "../../../dialogs/DeactivateAccountDialog";
 import PlatformPeg from "../../../../../PlatformPeg";
 import Modal from "../../../../../Modal";
-import dis from "../../../../../dispatcher/dispatcher";
-import { Service, ServicePolicyPair, startTermsFlow } from "../../../../../Terms";
-import IdentityAuthClient from "../../../../../IdentityAuthClient";
-import { abbreviateUrl } from "../../../../../utils/UrlUtils";
-import { getThreepidsWithBindStatus } from "../../../../../boundThreepids";
 import { SettingLevel } from "../../../../../settings/SettingLevel";
 import { UIFeature } from "../../../../../settings/UIFeature";
-import { ActionPayload } from "../../../../../dispatcher/payloads";
 import ErrorDialog, { extractErrorMessageFromError } from "../../../dialogs/ErrorDialog";
-import AccountPhoneNumbers from "../../account/PhoneNumbers";
-import AccountEmailAddresses from "../../account/EmailAddresses";
-import DiscoveryEmailAddresses from "../../discovery/EmailAddresses";
-import DiscoveryPhoneNumbers from "../../discovery/PhoneNumbers";
 import ChangePassword from "../../ChangePassword";
-import InlineTermsAgreement from "../../../terms/InlineTermsAgreement";
-import SetIdServer from "../../SetIdServer";
 import SetIntegrationManager from "../../SetIntegrationManager";
 import ToggleSwitch from "../../../elements/ToggleSwitch";
 import { IS_MAC } from "../../../../../Keyboard";
@@ -54,10 +41,8 @@ import SettingsTab from "../SettingsTab";
 import { SettingsSection } from "../../shared/SettingsSection";
 import SettingsSubsection, { SettingsSubsectionText } from "../../shared/SettingsSubsection";
 import { SettingsSubsectionHeading } from "../../shared/SettingsSubsectionHeading";
-import Heading from "../../../typography/Heading";
-import InlineSpinner from "../../../elements/InlineSpinner";
-import { ThirdPartyIdentifier } from "../../../../../AddThreepid";
 import { SDKContext } from "../../../../../contexts/SDKContext";
+import UserPersonalInfoSettings from "../../UserPersonalInfoSettings";
 
 interface IProps {
     closeSettingsFn: () => void;
@@ -67,25 +52,6 @@ interface IState {
     language: string;
     spellCheckEnabled?: boolean;
     spellCheckLanguages: string[];
-    haveIdServer: boolean;
-    idServerHasUnsignedTerms: boolean;
-    requiredPolicyInfo:
-        | {
-              // This object is passed along to a component for handling
-              hasTerms: false;
-              policiesAndServices: null; // From the startTermsFlow callback
-              agreedUrls: null; // From the startTermsFlow callback
-              resolve: null; // Promise resolve function for startTermsFlow callback
-          }
-        | {
-              hasTerms: boolean;
-              policiesAndServices: ServicePolicyPair[];
-              agreedUrls: string[];
-              resolve: (values: string[]) => void;
-          };
-    emails: ThirdPartyIdentifier[];
-    msisdns: ThirdPartyIdentifier[];
-    loading3pids: boolean; // whether or not the emails and msisdns have been loaded
     canChangePassword: boolean;
     idServerName?: string;
     externalAccountManagementUrl?: string;
@@ -96,38 +62,19 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
     public static contextType = SDKContext;
     public context!: React.ContextType<typeof SDKContext>;
 
-    private readonly dispatcherRef: string;
-
     public constructor(props: IProps, context: React.ContextType<typeof SDKContext>) {
         super(props);
         this.context = context;
 
-        const cli = this.context.client!;
-
         this.state = {
             language: languageHandler.getCurrentLanguage(),
             spellCheckEnabled: false,
             spellCheckLanguages: [],
-            haveIdServer: Boolean(cli.getIdentityServerUrl()),
-            idServerHasUnsignedTerms: false,
-            requiredPolicyInfo: {
-                // This object is passed along to a component for handling
-                hasTerms: false,
-                policiesAndServices: null, // From the startTermsFlow callback
-                agreedUrls: null, // From the startTermsFlow callback
-                resolve: null, // Promise resolve function for startTermsFlow callback
-            },
-            emails: [],
-            msisdns: [],
-            loading3pids: true, // whether or not the emails and msisdns have been loaded
             canChangePassword: false,
             canMake3pidChanges: false,
         };
 
-        this.dispatcherRef = dis.register(this.onAction);
-
         this.getCapabilities();
-        this.getThreepidState();
     }
 
     public async componentDidMount(): Promise<void> {
@@ -145,25 +92,6 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
         }
     }
 
-    public componentWillUnmount(): void {
-        dis.unregister(this.dispatcherRef);
-    }
-
-    private onAction = (payload: ActionPayload): void => {
-        if (payload.action === "id_server_changed") {
-            this.setState({ haveIdServer: Boolean(this.context.client!.getIdentityServerUrl()) });
-            this.getThreepidState();
-        }
-    };
-
-    private onEmailsChange = (emails: ThirdPartyIdentifier[]): void => {
-        this.setState({ emails });
-    };
-
-    private onMsisdnsChange = (msisdns: ThirdPartyIdentifier[]): void => {
-        this.setState({ msisdns });
-    };
-
     private async getCapabilities(): Promise<void> {
         const cli = this.context.client!;
 
@@ -185,73 +113,6 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
         this.setState({ canChangePassword, externalAccountManagementUrl, canMake3pidChanges });
     }
 
-    private async getThreepidState(): Promise<void> {
-        const cli = this.context.client!;
-
-        // Check to see if terms need accepting
-        this.checkTerms();
-
-        // Need to get 3PIDs generally for Account section and possibly also for
-        // Discovery (assuming we have an IS and terms are agreed).
-        let threepids: IThreepid[] = [];
-        try {
-            threepids = await getThreepidsWithBindStatus(cli);
-        } catch (e) {
-            const idServerUrl = cli.getIdentityServerUrl();
-            logger.warn(
-                `Unable to reach identity server at ${idServerUrl} to check ` + `for 3PIDs bindings in Settings`,
-            );
-            logger.warn(e);
-        }
-        this.setState({
-            emails: threepids.filter((a) => a.medium === ThreepidMedium.Email),
-            msisdns: threepids.filter((a) => a.medium === ThreepidMedium.Phone),
-            loading3pids: false,
-        });
-    }
-
-    private async checkTerms(): Promise<void> {
-        // By starting the terms flow we get the logic for checking which terms the user has signed
-        // for free. So we might as well use that for our own purposes.
-        const idServerUrl = this.context.client!.getIdentityServerUrl();
-        if (!this.state.haveIdServer || !idServerUrl) {
-            this.setState({ idServerHasUnsignedTerms: false });
-            return;
-        }
-
-        const authClient = new IdentityAuthClient();
-        try {
-            const idAccessToken = await authClient.getAccessToken({ check: false });
-            await startTermsFlow(
-                this.context.client!,
-                [new Service(SERVICE_TYPES.IS, idServerUrl, idAccessToken!)],
-                (policiesAndServices, agreedUrls, extraClassNames) => {
-                    return new Promise((resolve, reject) => {
-                        this.setState({
-                            idServerName: abbreviateUrl(idServerUrl),
-                            requiredPolicyInfo: {
-                                hasTerms: true,
-                                policiesAndServices,
-                                agreedUrls,
-                                resolve,
-                            },
-                        });
-                    });
-                },
-            );
-            // User accepted all terms
-            this.setState({
-                requiredPolicyInfo: {
-                    ...this.state.requiredPolicyInfo, // set first so we can override
-                    hasTerms: false,
-                },
-            });
-        } catch (e) {
-            logger.warn(`Unable to reach identity server at ${idServerUrl} to check ` + `for terms in Settings`);
-            logger.warn(e);
-        }
-    }
-
     private onLanguageChange = (newLanguage: string): void => {
         if (this.state.language === newLanguage) return;
 
@@ -324,48 +185,6 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
     };
 
     private renderAccountSection(): JSX.Element {
-        let threepidSection: ReactNode = null;
-
-        if (SettingsStore.getValue(UIFeature.ThirdPartyID)) {
-            const emails = this.state.loading3pids ? (
-                <InlineSpinner />
-            ) : (
-                <AccountEmailAddresses
-                    emails={this.state.emails}
-                    onEmailsChange={this.onEmailsChange}
-                    disabled={!this.state.canMake3pidChanges}
-                />
-            );
-            const msisdns = this.state.loading3pids ? (
-                <InlineSpinner />
-            ) : (
-                <AccountPhoneNumbers
-                    msisdns={this.state.msisdns}
-                    onMsisdnsChange={this.onMsisdnsChange}
-                    disabled={!this.state.canMake3pidChanges}
-                />
-            );
-            threepidSection = (
-                <>
-                    <SettingsSubsection
-                        heading={_t("settings|general|emails_heading")}
-                        stretchContent
-                        data-testid="mx_AccountEmailAddresses"
-                    >
-                        {emails}
-                    </SettingsSubsection>
-
-                    <SettingsSubsection
-                        heading={_t("settings|general|msisdns_heading")}
-                        stretchContent
-                        data-testid="mx_AccountPhoneNumbers"
-                    >
-                        {msisdns}
-                    </SettingsSubsection>
-                </>
-            );
-        }
-
         let passwordChangeSection: ReactNode = null;
         if (this.state.canChangePassword) {
             passwordChangeSection = (
@@ -419,7 +238,6 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
                     {externalAccountManagement}
                     {passwordChangeSection}
                 </SettingsSubsection>
-                {threepidSection}
             </>
         );
     }
@@ -455,51 +273,6 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
         );
     }
 
-    private renderDiscoverySection(): JSX.Element {
-        if (this.state.requiredPolicyInfo.hasTerms) {
-            const intro = (
-                <SettingsSubsectionText>
-                    {_t("settings|general|discovery_needs_terms", { serverName: this.state.idServerName })}
-                </SettingsSubsectionText>
-            );
-            return (
-                <>
-                    <InlineTermsAgreement
-                        policiesAndServicePairs={this.state.requiredPolicyInfo.policiesAndServices}
-                        agreedUrls={this.state.requiredPolicyInfo.agreedUrls}
-                        onFinished={this.state.requiredPolicyInfo.resolve}
-                        introElement={intro}
-                    />
-                    {/* has its own heading as it includes the current identity server */}
-                    <SetIdServer missingTerms={true} />
-                </>
-            );
-        }
-
-        const threepidSection = this.state.haveIdServer ? (
-            <>
-                <DiscoveryEmailAddresses
-                    emails={this.state.emails}
-                    isLoading={this.state.loading3pids}
-                    disabled={!this.state.canMake3pidChanges}
-                />
-                <DiscoveryPhoneNumbers
-                    msisdns={this.state.msisdns}
-                    isLoading={this.state.loading3pids}
-                    disabled={!this.state.canMake3pidChanges}
-                />
-            </>
-        ) : null;
-
-        return (
-            <>
-                {threepidSection}
-                {/* has its own heading as it includes the current identity server */}
-                <SetIdServer missingTerms={false} />
-            </>
-        );
-    }
-
     private renderManagementSection(): JSX.Element {
         // TODO: Improve warning text for account deactivation
         return (
@@ -533,40 +306,15 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
             accountManagementSection = this.renderManagementSection();
         }
 
-        let discoverySection;
-        if (SettingsStore.getValue(UIFeature.IdentityServer)) {
-            const discoWarning = this.state.requiredPolicyInfo.hasTerms ? (
-                <WarningIcon
-                    className="mx_GeneralUserSettingsTab_warningIcon"
-                    width="18"
-                    height="18"
-                    // override icon default values
-                    aria-hidden={false}
-                    aria-label={_t("common|warning")}
-                />
-            ) : null;
-            const heading = (
-                <Heading size="2">
-                    {discoWarning}
-                    {_t("settings|general|discovery_section")}
-                </Heading>
-            );
-            discoverySection = (
-                <SettingsSection heading={heading} data-testid="discoverySection">
-                    {this.renderDiscoverySection()}
-                </SettingsSection>
-            );
-        }
-
         return (
             <SettingsTab data-testid="mx_GeneralUserSettingsTab">
                 <SettingsSection>
                     <UserProfileSettings />
+                    <UserPersonalInfoSettings canMake3pidChanges={this.state.canMake3pidChanges} />
                     {this.renderAccountSection()}
                     {this.renderLanguageSection()}
                     {supportsMultiLanguageSpellCheck ? this.renderSpellCheckSection() : null}
                 </SettingsSection>
-                {discoverySection}
                 {this.renderIntegrationManagerSection()}
                 {accountManagementSection}
             </SettingsTab>

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

@@ -43,6 +43,7 @@ import SettingsTab from "../SettingsTab";
 import { SettingsSection } from "../../shared/SettingsSection";
 import SettingsSubsection, { SettingsSubsectionText } from "../../shared/SettingsSubsection";
 import { useOwnDevices } from "../../devices/useOwnDevices";
+import DiscoverySettings from "../../discovery/DiscoverySettings";
 
 interface IIgnoredUserProps {
     userId: string;
@@ -336,6 +337,7 @@ export default class SecurityUserSettingsTab extends React.Component<IProps, ISt
             };
             privacySection = (
                 <SettingsSection heading={_t("common|privacy")}>
+                    <DiscoverySettings />
                     <SettingsSubsection
                         heading={_t("common|analytics")}
                         description={_t("settings|security|analytics_description")}

src/i18n/strings/en_EN.json

@@ -2443,6 +2443,9 @@
         "code_block_expand_default": "Expand code blocks by default",
         "code_block_line_numbers": "Show line numbers in code blocks",
         "disable_historical_profile": "Show current profile picture and name for users in message history",
+        "discovery": {
+            "title": "How to find you"
+        },
         "emoji_autocomplete": "Enable Emoji suggestions while typing",
         "enable_markdown": "Enable Markdown",
         "enable_markdown_description": "Start messages with <code>/plain</code> to send without markdown.",
@@ -2479,11 +2482,11 @@
             "deactivate_section": "Deactivate Account",
             "deactivate_warning": "Deactivating your account is a permanent action — be careful!",
             "dialog_title": "<strong>Settings:</strong> General",
-            "discovery_email_empty": "Discovery options will appear once you have added an email above.",
+            "discovery_email_empty": "Discovery options will appear once you have added an email.",
             "discovery_email_verification_instructions": "Verify the link in your inbox",
-            "discovery_msisdn_empty": "Discovery options will appear once you have added a phone number above.",
+            "discovery_msisdn_empty": "Discovery options will appear once you have added a phone number.",
             "discovery_needs_terms": "Agree to the identity server (%(serverName)s) Terms of Service to allow yourself to be discoverable by email address or phone number.",
-            "discovery_section": "Discovery",
+            "discovery_needs_terms_title": "Let people find you",
             "display_name": "Display Name",
             "display_name_error": "Unable to set display name",
             "email_address_in_use": "This email address is already in use",
@@ -2522,11 +2525,14 @@
             "oidc_manage_button": "Manage account",
             "password_change_section": "Set a new account password…",
             "password_change_success": "Your password was successfully changed.",
+            "personal_info": "Personal info",
             "profile_subtitle": "This is how you appear to others on the app.",
             "remove_email_prompt": "Remove %(email)s?",
             "remove_msisdn_prompt": "Remove %(phone)s?",
             "spell_check_locale_placeholder": "Choose a locale",
             "spell_check_section": "Spell check",
+            "unable_to_load_emails": "Unable to load email addresses",
+            "unable_to_load_msisdns": "Unable to load phone numbers",
             "username": "Username"
         },
         "image_thumbnails": "Show previews/thumbnails for images",

test/components/views/settings/discovery/DiscoverySettings-test.tsx

@@ -0,0 +1,104 @@
+/*
+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 { act, render, screen } from "@testing-library/react";
+import { MatrixClient } from "matrix-js-sdk/src/matrix";
+import { mocked } from "jest-mock";
+import userEvent from "@testing-library/user-event";
+
+import DiscoverySettings from "../../../../../src/components/views/settings/discovery/DiscoverySettings";
+import { stubClient } from "../../../../test-utils";
+import MatrixClientContext from "../../../../../src/contexts/MatrixClientContext";
+import { UIFeature } from "../../../../../src/settings/UIFeature";
+import SettingsStore from "../../../../../src/settings/SettingsStore";
+import defaultDispatcher from "../../../../../src/dispatcher/dispatcher";
+
+const mockGetAccessToken = jest.fn().mockResolvedValue("$$getAccessToken");
+jest.mock("../../../../../src/IdentityAuthClient", () =>
+    jest.fn().mockImplementation(() => ({
+        getAccessToken: mockGetAccessToken,
+    })),
+);
+
+describe("DiscoverySettings", () => {
+    let client: MatrixClient;
+
+    beforeEach(() => {
+        client = stubClient();
+    });
+
+    afterEach(() => {
+        jest.restoreAllMocks();
+    });
+
+    const DiscoveryWrapper = (props = {}) => <MatrixClientContext.Provider value={client} {...props} />;
+
+    it("is empty if 3pid features are disabled", async () => {
+        jest.spyOn(SettingsStore, "getValue").mockImplementation((key) => {
+            if (key === UIFeature.ThirdPartyID) return false;
+        });
+
+        const { container } = render(<DiscoverySettings />, { wrapper: DiscoveryWrapper });
+
+        expect(container).toBeEmptyDOMElement();
+    });
+
+    it("displays alert if an identity server needs terms accepting", async () => {
+        mocked(client).getIdentityServerUrl.mockReturnValue("https://example.com");
+        mocked(client).getTerms.mockResolvedValue({
+            ["policies"]: { en: "No ball games" },
+        });
+
+        render(<DiscoverySettings />, { wrapper: DiscoveryWrapper });
+
+        await expect(await screen.findByText("Let people find you")).toBeInTheDocument();
+    });
+
+    it("button to accept terms is disabled if checkbox not checked", async () => {
+        mocked(client).getIdentityServerUrl.mockReturnValue("https://example.com");
+        mocked(client).getTerms.mockResolvedValue({
+            ["policies"]: { en: "No ball games" },
+        });
+
+        render(<DiscoverySettings />, { wrapper: DiscoveryWrapper });
+
+        const acceptCheckbox = await screen.findByRole("checkbox", { name: "Accept" });
+        const continueButton = screen.getByRole("button", { name: "Continue" });
+        expect(acceptCheckbox).toBeInTheDocument();
+        expect(continueButton).toHaveAttribute("aria-disabled", "true");
+
+        await userEvent.click(acceptCheckbox);
+        expect(continueButton).not.toHaveAttribute("aria-disabled", "true");
+    });
+
+    it("updates if ID server is changed", async () => {
+        render(<DiscoverySettings />, { wrapper: DiscoveryWrapper });
+
+        mocked(client).getThreePids.mockClear();
+
+        act(() => {
+            defaultDispatcher.dispatch(
+                {
+                    action: "id_server_changed",
+                },
+                true,
+            );
+        });
+
+        expect(client.getThreePids).toHaveBeenCalled();
+    });
+});

test/components/views/settings/discovery/__snapshots__/EmailAddresses-test.tsx.snap

@@ -20,7 +20,7 @@ exports[`<EmailAddresses /> should handle no email addresses 1`] = `
       <div
         class="mx_SettingsSubsection_text"
       >
-        Discovery options will appear once you have added an email above.
+        Discovery options will appear once you have added an email.
       </div>
     </div>
   </div>

test/components/views/settings/discovery/__snapshots__/PhoneNumbers-test.tsx.snap

@@ -83,7 +83,7 @@ exports[`<PhoneNumbers /> should handle no numbers 1`] = `
       <div
         class="mx_SettingsSubsection_text"
       >
-        Discovery options will appear once you have added a phone number above.
+        Discovery options will appear once you have added a phone number.
       </div>
     </div>
   </div>

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

@@ -42,14 +42,14 @@ exports[`<GeneralUserSettingsTab /> 3pids should display 3pid email addresses an
       >
         <input
           autocomplete="email"
-          id="mx_Field_50"
+          id="mx_Field_41"
           label="Email Address"
           placeholder="Email Address"
           type="text"
           value=""
         />
         <label
-          for="mx_Field_50"
+          for="mx_Field_41"
         >
           Email Address
         </label>
@@ -150,14 +150,14 @@ exports[`<GeneralUserSettingsTab /> 3pids should display 3pid email addresses an
           </span>
           <input
             autocomplete="tel-national"
-            id="mx_Field_51"
+            id="mx_Field_42"
             label="Phone Number"
             placeholder="Phone Number"
             type="text"
             value=""
           />
           <label
-            for="mx_Field_51"
+            for="mx_Field_42"
           >
             Phone Number
           </label>