Use semantic headings in user settings - account (#10972)

* account password section

* account email and phone numbers

* update cypress selectors

Author: Kerry

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

@@ -83,10 +83,14 @@ describe("General user settings tab", () => {
                 });
 
             // Wait until spinners disappear
-            cy.get(".mx_GeneralUserSettingsTab_section--account .mx_Spinner").should("not.exist");
-            cy.get(".mx_GeneralUserSettingsTab_section--discovery .mx_Spinner").should("not.exist");
+            cy.findByTestId("accountSection").within(() => {
+                cy.get(".mx_Spinner").should("not.exist");
+            });
+            cy.findByTestId("discoverySection").within(() => {
+                cy.get(".mx_Spinner").should("not.exist");
+            });
 
-            cy.get(".mx_GeneralUserSettingsTab_section--account").within(() => {
+            cy.findByTestId("accountSection").within(() => {
                 // Assert that input areas for changing a password exists
                 cy.get("form.mx_GeneralUserSettingsTab_section--account_changePassword")
                     .scrollIntoView()
@@ -95,29 +99,28 @@ describe("General user settings tab", () => {
                         cy.findByLabelText("New Password").should("be.visible");
                         cy.findByLabelText("Confirm password").should("be.visible");
                     });
+            });
+            // Check email addresses area
+            cy.findByTestId("mx_AccountEmailAddresses")
+                .scrollIntoView()
+                .within(() => {
+                    // Assert that an input area for a new email address is rendered
+                    cy.findByRole("textbox", { name: "Email Address" }).should("be.visible");
 
-                // Check email addresses area
-                cy.get(".mx_EmailAddresses")
-                    .scrollIntoView()
-                    .within(() => {
-                        // Assert that an input area for a new email address is rendered
-                        cy.findByRole("textbox", { name: "Email Address" }).should("be.visible");
-
-                        // Assert the add button is visible
-                        cy.findByRole("button", { name: "Add" }).should("be.visible");
-                    });
+                    // Assert the add button is visible
+                    cy.findByRole("button", { name: "Add" }).should("be.visible");
+                });
 
-                // Check phone numbers area
-                cy.get(".mx_PhoneNumbers")
-                    .scrollIntoView()
-                    .within(() => {
-                        // Assert that an input area for a new phone number is rendered
-                        cy.findByRole("textbox", { name: "Phone Number" }).should("be.visible");
+            // Check phone numbers area
+            cy.findByTestId("mx_AccountPhoneNumbers")
+                .scrollIntoView()
+                .within(() => {
+                    // Assert that an input area for a new phone number is rendered
+                    cy.findByRole("textbox", { name: "Phone Number" }).should("be.visible");
 
-                        // Assert that the add button is rendered
-                        cy.findByRole("button", { name: "Add" }).should("be.visible");
-                    });
-            });
+                    // Assert that the add button is rendered
+                    cy.findByRole("button", { name: "Add" }).should("be.visible");
+                });
 
             // Check language and region setting dropdown
             cy.get(".mx_GeneralUserSettingsTab_section_languageInput")
@@ -188,7 +191,7 @@ describe("General user settings tab", () => {
 
     it("should set a country calling code based on default_country_code", () => {
         // Check phone numbers area
-        cy.get(".mx_PhoneNumbers")
+        cy.findByTestId("mx_AccountPhoneNumbers")
             .scrollIntoView()
             .within(() => {
                 // Assert that an input area for a new phone number is rendered

res/css/views/elements/_TagComposer.pcss

@@ -17,16 +17,12 @@ limitations under the License.
 .mx_TagComposer {
     .mx_TagComposer_input {
         display: flex;
-
-        .mx_Field {
-            flex: 1;
-            margin: 0; /* override from field styles */
-        }
+        flex-direction: row;
 
         .mx_AccessibleButton {
             min-width: 70px;
             padding: 0 8px; /* override from button styles */
-            margin-left: 16px; /* distance from <Field> */
+            align-self: stretch; /* override default settingstab style */
         }
 
         .mx_Field,

res/css/views/settings/tabs/_SettingsTab.pcss

@@ -24,6 +24,24 @@ limitations under the License.
     a {
         color: $links;
     }
+
+    form {
+        display: flex;
+        flex-direction: column;
+        gap: $spacing-8;
+        flex-grow: 1;
+    }
+    // never want full width buttons
+    // event when other content is 100% width
+    .mx_AccessibleButton {
+        align-self: flex-start;
+        justify-self: flex-start;
+    }
+
+    .mx_Field {
+        margin: 0;
+        flex: 1;
+    }
 }
 
 .mx_SettingsTab_warningText {

res/css/views/settings/tabs/user/_GeneralUserSettingsTab.pcss

@@ -14,41 +14,9 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-.mx_GeneralUserSettingsTab_section--account_changePassword {
-    .mx_Field {
-        margin-inline-end: var(--SettingsTab_fullWidthField-margin-inline-end);
-
-        &:first-child {
-            margin-top: 0;
-        }
-    }
-}
-
-/* TODO: Make this selector less painful */
-.mx_GeneralUserSettingsTab_section--account .mx_SettingsTab_subheading:nth-child(n + 1),
-.mx_GeneralUserSettingsTab_section--discovery .mx_SettingsTab_subheading:nth-child(n + 2),
-.mx_SetIdServer .mx_SettingsTab_subheading {
-    margin-top: 24px;
-}
-
-.mx_GeneralUserSettingsTab_section--account,
-.mx_GeneralUserSettingsTab_section--discovery {
-    .mx_Spinner {
-        /* Move the spinner to the left side of the container (default center) */
-        justify-content: flex-start;
-    }
-}
-
-.mx_GeneralUserSettingsTab_section--account .mx_EmailAddresses,
-.mx_GeneralUserSettingsTab_section--account .mx_PhoneNumbers,
-.mx_GeneralUserSettingsTab_section--discovery .mx_GeneralUserSettingsTab_section--discovery_existing {
-    margin-inline-end: var(--SettingsTab_fullWidthField-margin-inline-end);
-}
-
 .mx_GeneralUserSettingsTab_section--discovery_existing {
     display: flex;
     align-items: center;
-    margin-bottom: 5px;
 }
 
 .mx_GeneralUserSettingsTab_section--discovery_existing_address,

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

@@ -276,7 +276,7 @@ export default class EmailAddresses extends React.Component<IProps, IState> {
         }
 
         return (
-            <div className="mx_EmailAddresses">
+            <>
                 {existingEmailElements}
                 <form onSubmit={this.onAddClick} autoComplete="off" noValidate={true}>
                     <Field
@@ -289,7 +289,7 @@ export default class EmailAddresses extends React.Component<IProps, IState> {
                     />
                     {addButton}
                 </form>
-            </div>
+            </>
         );
     }
 }

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

@@ -305,7 +305,7 @@ export default class PhoneNumbers extends React.Component<IProps, IState> {
         );
 
         return (
-            <div className="mx_PhoneNumbers">
+            <>
                 {existingPhoneElements}
                 <form onSubmit={this.onAddClick} autoComplete="off" noValidate={true} className="mx_PhoneNumbers_new">
                     <div className="mx_PhoneNumbers_input">
@@ -321,7 +321,7 @@ export default class PhoneNumbers extends React.Component<IProps, IState> {
                     </div>
                 </form>
                 {addVerifySection}
-            </div>
+            </>
         );
     }
 }

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

@@ -294,7 +294,7 @@ export default class PhoneNumbers extends React.Component<IProps> {
 
         return (
             <SettingsSubsection
-                data-testid="mx_PhoneNumbers"
+                data-testid="mx_DiscoveryPhoneNumbers"
                 heading={_t("Phone numbers")}
                 description={description}
                 stretchContent

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

@@ -60,6 +60,7 @@ 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";
 
 interface IProps {
     closeSettingsFn: () => void;
@@ -196,7 +197,6 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
             );
             logger.warn(e);
         }
-
         this.setState({
             emails: threepids.filter((a) => a.medium === ThreepidMedium.Email),
             msisdns: threepids.filter((a) => a.medium === ThreepidMedium.Phone),
@@ -329,16 +329,6 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
     }
 
     private renderAccountSection(): JSX.Element {
-        let passwordChangeForm: ReactNode = (
-            <ChangePassword
-                className="mx_GeneralUserSettingsTab_section--account_changePassword"
-                rowClassName=""
-                buttonKind="primary"
-                onError={this.onPasswordChangeError}
-                onFinished={this.onPasswordChanged}
-            />
-        );
-
         let threepidSection: ReactNode = null;
 
         // For older homeservers without separate 3PID add and bind methods (MSC2290),
@@ -351,33 +341,52 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
             (this.state.haveIdServer || this.state.serverSupportsSeparateAddAndBind === true)
         ) {
             const emails = this.state.loading3pids ? (
-                <Spinner />
+                <InlineSpinner />
             ) : (
                 <AccountEmailAddresses emails={this.state.emails} onEmailsChange={this.onEmailsChange} />
             );
             const msisdns = this.state.loading3pids ? (
-                <Spinner />
+                <InlineSpinner />
             ) : (
                 <AccountPhoneNumbers msisdns={this.state.msisdns} onMsisdnsChange={this.onMsisdnsChange} />
             );
             threepidSection = (
-                <div>
-                    <span className="mx_SettingsTab_subheading">{_t("Email addresses")}</span>
-                    {emails}
+                <>
+                    <SettingsSubsection
+                        heading={_t("Email addresses")}
+                        stretchContent
+                        data-testid="mx_AccountEmailAddresses"
+                    >
+                        {emails}
+                    </SettingsSubsection>
 
-                    <span className="mx_SettingsTab_subheading">{_t("Phone numbers")}</span>
-                    {msisdns}
-                </div>
+                    <SettingsSubsection
+                        heading={_t("Phone numbers")}
+                        stretchContent
+                        data-testid="mx_AccountPhoneNumbers"
+                    >
+                        {msisdns}
+                    </SettingsSubsection>
+                </>
             );
         } else if (this.state.serverSupportsSeparateAddAndBind === null) {
             threepidSection = <Spinner />;
         }
 
-        let passwordChangeText: ReactNode = _t("Set a new account password…");
-        if (!this.state.canChangePassword) {
-            // Just don't show anything if you can't do anything.
-            passwordChangeText = null;
-            passwordChangeForm = null;
+        let passwordChangeSection: ReactNode = null;
+        if (this.state.canChangePassword) {
+            passwordChangeSection = (
+                <>
+                    <SettingsSubsectionText>{_t("Set a new account password…")}</SettingsSubsectionText>
+                    <ChangePassword
+                        className="mx_GeneralUserSettingsTab_section--account_changePassword"
+                        rowClassName=""
+                        buttonKind="primary"
+                        onError={this.onPasswordChangeError}
+                        onFinished={this.onPasswordChanged}
+                    />
+                </>
+            );
         }
 
         let externalAccountManagement: JSX.Element | undefined;
@@ -386,13 +395,13 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
 
             externalAccountManagement = (
                 <>
-                    <p className="mx_SettingsTab_subsectionText" data-testid="external-account-management-outer">
+                    <SettingsSubsectionText data-testid="external-account-management-outer">
                         {_t(
                             "Your account details are managed separately at <code>%(hostname)s</code>.",
                             { hostname },
                             { code: (sub) => <code>{sub}</code> },
                         )}
-                    </p>
+                    </SettingsSubsectionText>
                     <AccessibleButton
                         onClick={null}
                         element="a"
@@ -408,13 +417,13 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
             );
         }
         return (
-            <div className="mx_SettingsTab_section mx_GeneralUserSettingsTab_section--account">
-                <span className="mx_SettingsTab_subheading">{_t("Account")}</span>
-                {externalAccountManagement}
-                <p className="mx_SettingsTab_subsectionText">{passwordChangeText}</p>
-                {passwordChangeForm}
+            <>
+                <SettingsSubsection heading={_t("Account")} stretchContent data-testid="accountSection">
+                    {externalAccountManagement}
+                    {passwordChangeSection}
+                </SettingsSubsection>
                 {threepidSection}
-            </div>
+            </>
         );
     }
 
@@ -540,7 +549,11 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
                     {_t("Discovery")}
                 </Heading>
             );
-            discoverySection = <SettingsSection heading={heading}>{this.renderDiscoverySection()}</SettingsSection>;
+            discoverySection = (
+                <SettingsSection heading={heading} data-testid="discoverySection">
+                    {this.renderDiscoverySection()}
+                </SettingsSection>
+            );
         }
 
         return (

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

@@ -4,7 +4,7 @@ exports[`<PhoneNumbers /> should handle no numbers 1`] = `
 <div>
   <div
     class="mx_SettingsSubsection"
-    data-testid="mx_PhoneNumbers"
+    data-testid="mx_DiscoveryPhoneNumbers"
   >
     <div
       class="mx_SettingsSubsectionHeading"
@@ -32,7 +32,7 @@ exports[`<PhoneNumbers /> should render a loader while loading 1`] = `
 <div>
   <div
     class="mx_SettingsSubsection"
-    data-testid="mx_PhoneNumbers"
+    data-testid="mx_DiscoveryPhoneNumbers"
   >
     <div
       class="mx_SettingsSubsectionHeading"
@@ -64,7 +64,7 @@ exports[`<PhoneNumbers /> should render phone numbers 1`] = `
 <div>
   <div
     class="mx_SettingsSubsection"
-    data-testid="mx_PhoneNumbers"
+    data-testid="mx_DiscoveryPhoneNumbers"
   >
     <div
       class="mx_SettingsSubsectionHeading"