Use semantic headings in user settings - discovery (#10838)

* 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

* use currentColor in warning-triangle svg, update use in RoomStatusBar

* use semantic headings for discovery section

* test manage integration settings

* test deactivate account section display

* remove SettingsFieldset margins

* threepids styles

* remove debug

* test discovery email and phone

src/components/structures/RoomStatusBar.tsx

@@ -20,6 +20,7 @@ import { SyncState, ISyncStateData } from "matrix-js-sdk/src/sync";
 import { Room } from "matrix-js-sdk/src/models/room";
 import { MatrixError } from "matrix-js-sdk/src/matrix";
 
+import { Icon as WarningIcon } from "../../../res/img/feather-customised/warning-triangle.svg";
 import { _t, _td } from "../../languageHandler";
 import Resend from "../../Resend";
 import dis from "../../dispatcher/dispatcher";
@@ -279,12 +280,7 @@ export default class RoomStatusBar extends React.PureComponent<IProps, IState> {
                 <div className="mx_RoomStatusBar">
                     <div role="alert">
                         <div className="mx_RoomStatusBar_connectionLostBar">
-                            <img
-                                src={require("../../../res/img/feather-customised/warning-triangle.svg").default}
-                                width="24"
-                                height="24"
-                                alt=""
-                            />
+                            <WarningIcon width="24" height="24" />
                             <div>
                                 <div className="mx_RoomStatusBar_connectionLostBar_title">
                                     {_t("Connectivity to the server has been lost.")}

src/components/views/settings/SetIdServer.tsx

@@ -32,6 +32,8 @@ import InlineSpinner from "../elements/InlineSpinner";
 import AccessibleButton from "../elements/AccessibleButton";
 import Field from "../elements/Field";
 import QuestionDialog from "../dialogs/QuestionDialog";
+import SettingsFieldset from "./SettingsFieldset";
+import { SettingsSubsectionText } from "./shared/SettingsSubsection";
 
 // We'll wait up to this long when checking for 3PID bindings on the IS.
 const REACHABILITY_TIMEOUT = 10000; // ms
@@ -428,41 +430,41 @@ export default class SetIdServer extends React.Component<IProps, IState> {
                 discoButtonContent = <InlineSpinner />;
             }
             discoSection = (
-                <div>
-                    <span className="mx_SettingsTab_subsectionText">{discoBodyText}</span>
+                <>
+                    <SettingsSubsectionText>{discoBodyText}</SettingsSubsectionText>
                     <AccessibleButton onClick={this.onDisconnectClicked} kind="danger_sm">
                         {discoButtonContent}
                     </AccessibleButton>
-                </div>
+                </>
             );
         }
 
         return (
-            <form className="mx_SetIdServer" onSubmit={this.checkIdServer}>
-                <span className="mx_SettingsTab_subheading">{sectionTitle}</span>
-                <span className="mx_SettingsTab_subsectionText">{bodyText}</span>
-                <Field
-                    label={_t("Enter a new identity server")}
-                    type="text"
-                    autoComplete="off"
-                    placeholder={this.state.defaultIdServer}
-                    value={this.state.idServer}
-                    onChange={this.onIdentityServerChanged}
-                    tooltipContent={this.getTooltip()}
-                    tooltipClassName="mx_SetIdServer_tooltip"
-                    disabled={this.state.busy}
-                    forceValidity={this.state.error ? false : undefined}
-                />
-                <AccessibleButton
-                    type="submit"
-                    kind="primary_sm"
-                    onClick={this.checkIdServer}
-                    disabled={!this.idServerChangeEnabled()}
-                >
-                    {_t("Change")}
-                </AccessibleButton>
-                {discoSection}
-            </form>
+            <SettingsFieldset legend={sectionTitle} description={bodyText}>
+                <form className="mx_SetIdServer" onSubmit={this.checkIdServer}>
+                    <Field
+                        label={_t("Enter a new identity server")}
+                        type="text"
+                        autoComplete="off"
+                        placeholder={this.state.defaultIdServer}
+                        value={this.state.idServer}
+                        onChange={this.onIdentityServerChanged}
+                        tooltipContent={this.getTooltip()}
+                        tooltipClassName="mx_SetIdServer_tooltip"
+                        disabled={this.state.busy}
+                        forceValidity={this.state.error ? false : undefined}
+                    />
+                    <AccessibleButton
+                        type="submit"
+                        kind="primary_sm"
+                        onClick={this.checkIdServer}
+                        disabled={!this.idServerChangeEnabled()}
+                    >
+                        {_t("Change")}
+                    </AccessibleButton>
+                    {discoSection}
+                </form>
+            </SettingsFieldset>
         );
     }
 }

src/components/views/settings/SettingsFieldset.tsx

@@ -15,6 +15,8 @@ limitations under the License.
 import React, { ReactNode, HTMLAttributes } from "react";
 import classNames from "classnames";
 
+import { SettingsSubsectionText } from "./shared/SettingsSubsection";
+
 interface Props extends HTMLAttributes<HTMLFieldSetElement> {
     // section title
     legend: string | ReactNode;
@@ -24,7 +26,11 @@ interface Props extends HTMLAttributes<HTMLFieldSetElement> {
 const SettingsFieldset: React.FC<Props> = ({ legend, className, children, description, ...rest }) => (
     <fieldset {...rest} className={classNames("mx_SettingsFieldset", className)}>
         <legend className="mx_SettingsFieldset_legend">{legend}</legend>
-        {description && <div className="mx_SettingsFieldset_description">{description}</div>}
+        {description && (
+            <div className="mx_SettingsFieldset_description">
+                <SettingsSubsectionText>{description}</SettingsSubsectionText>
+            </div>
+        )}
         {children}
     </fieldset>
 );

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

@@ -25,6 +25,8 @@ import { MatrixClientPeg } from "../../../../MatrixClientPeg";
 import Modal from "../../../../Modal";
 import AddThreepid, { Binding } from "../../../../AddThreepid";
 import ErrorDialog, { extractErrorMessageFromError } from "../../dialogs/ErrorDialog";
+import SettingsSubsection from "../shared/SettingsSubsection";
+import InlineSpinner from "../../elements/InlineSpinner";
 import AccessibleButton, { ButtonEvent } from "../../elements/AccessibleButton";
 
 /*
@@ -258,23 +260,32 @@ export class EmailAddress extends React.Component<IEmailAddressProps, IEmailAddr
 }
 interface IProps {
     emails: IThreepid[];
+    isLoading?: boolean;
 }
 
 export default class EmailAddresses extends React.Component<IProps> {
     public render(): React.ReactNode {
         let content;
-        if (this.props.emails.length > 0) {
+        if (this.props.isLoading) {
+            content = <InlineSpinner />;
+        } else if (this.props.emails.length > 0) {
             content = this.props.emails.map((e) => {
                 return <EmailAddress email={e} key={e.address} />;
             });
-        } else {
-            content = (
-                <span className="mx_SettingsTab_subsectionText">
-                    {_t("Discovery options will appear once you have added an email above.")}
-                </span>
-            );
         }
 
-        return <div className="mx_EmailAddresses">{content}</div>;
+        const hasEmails = !!this.props.emails.length;
+
+        return (
+            <SettingsSubsection
+                heading={_t("Email addresses")}
+                description={
+                    (!hasEmails && _t("Discovery options will appear once you have added an email above.")) || undefined
+                }
+                stretchContent
+            >
+                {content}
+            </SettingsSubsection>
+        );
     }
 }

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

@@ -26,6 +26,8 @@ import Modal from "../../../../Modal";
 import AddThreepid, { Binding } from "../../../../AddThreepid";
 import ErrorDialog, { extractErrorMessageFromError } from "../../dialogs/ErrorDialog";
 import Field from "../../elements/Field";
+import SettingsSubsection from "../shared/SettingsSubsection";
+import InlineSpinner from "../../elements/InlineSpinner";
 import AccessibleButton, { ButtonEvent } from "../../elements/AccessibleButton";
 
 /*
@@ -273,23 +275,32 @@ export class PhoneNumber extends React.Component<IPhoneNumberProps, IPhoneNumber
 
 interface IProps {
     msisdns: IThreepid[];
+    isLoading?: boolean;
 }
 
 export default class PhoneNumbers extends React.Component<IProps> {
     public render(): React.ReactNode {
         let content;
-        if (this.props.msisdns.length > 0) {
+        if (this.props.isLoading) {
+            content = <InlineSpinner />;
+        } else if (this.props.msisdns.length > 0) {
             content = this.props.msisdns.map((e) => {
                 return <PhoneNumber msisdn={e} key={e.address} />;
             });
-        } else {
-            content = (
-                <span className="mx_SettingsTab_subsectionText">
-                    {_t("Discovery options will appear once you have added a phone number above.")}
-                </span>
-            );
         }
 
-        return <div className="mx_PhoneNumbers">{content}</div>;
+        const description =
+            (!content && _t("Discovery options will appear once you have added a phone number above.")) || undefined;
+
+        return (
+            <SettingsSubsection
+                data-testid="mx_PhoneNumbers"
+                heading={_t("Phone numbers")}
+                description={description}
+                stretchContent
+            >
+                {content}
+            </SettingsSubsection>
+        );
     }
 }

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

@@ -47,14 +47,16 @@ export const SettingsSubsection: React.FC<SettingsSubsectionProps> = ({
                 <SettingsSubsectionText>{description}</SettingsSubsectionText>
             </div>
         )}
-        <div
-            className={classNames("mx_SettingsSubsection_content", {
-                mx_SettingsSubsection_contentStretch: !!stretchContent,
-                mx_SettingsSubsection_noHeading: !heading && !description,
-            })}
-        >
-            {children}
-        </div>
+        {!!children && (
+            <div
+                className={classNames("mx_SettingsSubsection_content", {
+                    mx_SettingsSubsection_contentStretch: !!stretchContent,
+                    mx_SettingsSubsection_noHeading: !heading && !description,
+                })}
+            >
+                {children}
+            </div>
+        )}
     </div>
 );
 

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

@@ -18,11 +18,12 @@ limitations under the License.
 
 import React, { ReactNode } from "react";
 import { SERVICE_TYPES } from "matrix-js-sdk/src/service-types";
-import { IThreepid } from "matrix-js-sdk/src/@types/threepids";
+import { IThreepid, ThreepidMedium } from "matrix-js-sdk/src/@types/threepids";
 import { logger } from "matrix-js-sdk/src/logger";
 import { IDelegatedAuthConfig, M_AUTHENTICATION } from "matrix-js-sdk/src/matrix";
 import { HTTPError } from "matrix-js-sdk/src/matrix";
 
+import { Icon as WarningIcon } from "../../../../../../res/img/feather-customised/warning-triangle.svg";
 import { UserFriendlyError, _t } from "../../../../../languageHandler";
 import ProfileSettings from "../../ProfileSettings";
 import * as languageHandler from "../../../../../languageHandler";
@@ -56,8 +57,9 @@ import ToggleSwitch from "../../../elements/ToggleSwitch";
 import { IS_MAC } from "../../../../../Keyboard";
 import SettingsTab from "../SettingsTab";
 import { SettingsSection } from "../../shared/SettingsSection";
-import SettingsSubsection from "../../shared/SettingsSubsection";
+import SettingsSubsection, { SettingsSubsectionText } from "../../shared/SettingsSubsection";
 import { SettingsSubsectionHeading } from "../../shared/SettingsSubsectionHeading";
+import Heading from "../../../typography/Heading";
 
 interface IProps {
     closeSettingsFn: () => void;
@@ -194,9 +196,10 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
             );
             logger.warn(e);
         }
+
         this.setState({
-            emails: threepids.filter((a) => a.medium === "email"),
-            msisdns: threepids.filter((a) => a.medium === "msisdn"),
+            emails: threepids.filter((a) => a.medium === ThreepidMedium.Email),
+            msisdns: threepids.filter((a) => a.medium === ThreepidMedium.Phone),
             loading3pids: false,
         });
     }
@@ -449,16 +452,16 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
     private renderDiscoverySection(): JSX.Element {
         if (this.state.requiredPolicyInfo.hasTerms) {
             const intro = (
-                <span className="mx_SettingsTab_subsectionText">
+                <SettingsSubsectionText>
                     {_t(
                         "Agree to the identity server (%(serverName)s) Terms of Service to " +
                             "allow yourself to be discoverable by email address or phone number.",
                         { serverName: this.state.idServerName },
                     )}
-                </span>
+                </SettingsSubsectionText>
             );
             return (
-                <div>
+                <>
                     <InlineTermsAgreement
                         policiesAndServicePairs={this.state.requiredPolicyInfo.policiesAndServices}
                         agreedUrls={this.state.requiredPolicyInfo.agreedUrls}
@@ -467,29 +470,23 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
                     />
                     {/* has its own heading as it includes the current identity server */}
                     <SetIdServer missingTerms={true} />
-                </div>
+                </>
             );
         }
 
-        const emails = this.state.loading3pids ? <Spinner /> : <DiscoveryEmailAddresses emails={this.state.emails} />;
-        const msisdns = this.state.loading3pids ? <Spinner /> : <DiscoveryPhoneNumbers msisdns={this.state.msisdns} />;
-
         const threepidSection = this.state.haveIdServer ? (
             <>
-                <span className="mx_SettingsTab_subheading">{_t("Email addresses")}</span>
-                {emails}
-
-                <span className="mx_SettingsTab_subheading">{_t("Phone numbers")}</span>
-                {msisdns}
+                <DiscoveryEmailAddresses emails={this.state.emails} isLoading={this.state.loading3pids} />
+                <DiscoveryPhoneNumbers msisdns={this.state.msisdns} isLoading={this.state.loading3pids} />
             </>
         ) : null;
 
         return (
-            <div className="mx_SettingsTab_section mx_GeneralUserSettingsTab_section--discovery">
+            <>
                 {threepidSection}
                 {/* has its own heading as it includes the current identity server */}
                 <SetIdServer missingTerms={false} />
-            </div>
+            </>
         );
     }
 
@@ -520,16 +517,6 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
         const plaf = PlatformPeg.get();
         const supportsMultiLanguageSpellCheck = plaf?.supportsSpellCheckSettings();
 
-        const discoWarning = this.state.requiredPolicyInfo.hasTerms ? (
-            <img
-                className="mx_GeneralUserSettingsTab_heading_warningIcon"
-                src={require("../../../../../../res/img/feather-customised/warning-triangle.svg").default}
-                width="18"
-                height="18"
-                alt={_t("Warning")}
-            />
-        ) : null;
-
         let accountManagementSection: JSX.Element | undefined;
         if (SettingsStore.getValue(UIFeature.Deactivate)) {
             accountManagementSection = this.renderManagementSection();
@@ -537,14 +524,23 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
 
         let discoverySection;
         if (SettingsStore.getValue(UIFeature.IdentityServer)) {
-            discoverySection = (
-                <>
-                    <div className="mx_SettingsTab_heading">
-                        {discoWarning} {_t("Discovery")}
-                    </div>
-                    {this.renderDiscoverySection()}
-                </>
+            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("Warning")}
+                />
+            ) : null;
+            const heading = (
+                <Heading size="h2">
+                    {discoWarning}
+                    {_t("Discovery")}
+                </Heading>
             );
+            discoverySection = <SettingsSection heading={heading}>{this.renderDiscoverySection()}</SettingsSection>;
         }
 
         return (