From d0d9a36d07631f9e91c447ff48e104de6920da97 Mon Sep 17 00:00:00 2001 From: Kerry Date: Fri, 26 May 2023 10:42:01 +1200 Subject: [PATCH] Use semantic headings in user settings - account (#10972) * account password section * account email and phone numbers * update cypress selectors --- .../general-user-settings-tab.spec.ts | 55 +++++++------ res/css/views/elements/_TagComposer.pcss | 8 +- res/css/views/settings/tabs/_SettingsTab.pcss | 18 +++++ .../tabs/user/_GeneralUserSettingsTab.pcss | 32 -------- .../views/settings/account/EmailAddresses.tsx | 4 +- .../views/settings/account/PhoneNumbers.tsx | 4 +- .../views/settings/discovery/PhoneNumbers.tsx | 2 +- .../tabs/user/GeneralUserSettingsTab.tsx | 79 +++++++++++-------- .../__snapshots__/PhoneNumbers-test.tsx.snap | 6 +- 9 files changed, 103 insertions(+), 105 deletions(-) diff --git a/cypress/e2e/settings/general-user-settings-tab.spec.ts b/cypress/e2e/settings/general-user-settings-tab.spec.ts index b78fe390d9..2c12f3cb75 100644 --- a/cypress/e2e/settings/general-user-settings-tab.spec.ts +++ b/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.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"); - }); - - // 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"); - - // Assert that the add button is rendered - cy.findByRole("button", { name: "Add" }).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"); + + // Assert the add button is visible + cy.findByRole("button", { name: "Add" }).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"); + }); // 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 diff --git a/res/css/views/elements/_TagComposer.pcss b/res/css/views/elements/_TagComposer.pcss index 8c9b7b071a..51d9749e2b 100644 --- a/res/css/views/elements/_TagComposer.pcss +++ b/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 */ + align-self: stretch; /* override default settingstab style */ } .mx_Field, diff --git a/res/css/views/settings/tabs/_SettingsTab.pcss b/res/css/views/settings/tabs/_SettingsTab.pcss index b060a02541..6ab44ec5b2 100644 --- a/res/css/views/settings/tabs/_SettingsTab.pcss +++ b/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 { diff --git a/res/css/views/settings/tabs/user/_GeneralUserSettingsTab.pcss b/res/css/views/settings/tabs/user/_GeneralUserSettingsTab.pcss index a0e81ce115..76c5834fa8 100644 --- a/res/css/views/settings/tabs/user/_GeneralUserSettingsTab.pcss +++ b/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, diff --git a/src/components/views/settings/account/EmailAddresses.tsx b/src/components/views/settings/account/EmailAddresses.tsx index e407170116..37d8697d47 100644 --- a/src/components/views/settings/account/EmailAddresses.tsx +++ b/src/components/views/settings/account/EmailAddresses.tsx @@ -276,7 +276,7 @@ export default class EmailAddresses extends React.Component { } return ( -
+ <> {existingEmailElements}
{ /> {addButton} -
+ ); } } diff --git a/src/components/views/settings/account/PhoneNumbers.tsx b/src/components/views/settings/account/PhoneNumbers.tsx index 3d9f070973..33225f1759 100644 --- a/src/components/views/settings/account/PhoneNumbers.tsx +++ b/src/components/views/settings/account/PhoneNumbers.tsx @@ -305,7 +305,7 @@ export default class PhoneNumbers extends React.Component { ); return ( -
+ <> {existingPhoneElements}
@@ -321,7 +321,7 @@ export default class PhoneNumbers extends React.Component {
{addVerifySection} -
+ ); } } diff --git a/src/components/views/settings/discovery/PhoneNumbers.tsx b/src/components/views/settings/discovery/PhoneNumbers.tsx index e9e67490d6..408573790a 100644 --- a/src/components/views/settings/discovery/PhoneNumbers.tsx +++ b/src/components/views/settings/discovery/PhoneNumbers.tsx @@ -294,7 +294,7 @@ export default class PhoneNumbers extends React.Component { return ( void; @@ -196,7 +197,6 @@ export default class GeneralUserSettingsTab extends React.Component a.medium === ThreepidMedium.Email), msisdns: threepids.filter((a) => a.medium === ThreepidMedium.Phone), @@ -329,16 +329,6 @@ export default class GeneralUserSettingsTab extends React.Component - ); - 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 + ) : ( ); const msisdns = this.state.loading3pids ? ( - + ) : ( ); threepidSection = ( -
- {_t("Email addresses")} - {emails} + <> + + {emails} + - {_t("Phone numbers")} - {msisdns} -
+ + {msisdns} + + ); } else if (this.state.serverSupportsSeparateAddAndBind === null) { threepidSection = ; } - 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 = ( + <> + {_t("Set a new account password…")} + + + ); } let externalAccountManagement: JSX.Element | undefined; @@ -386,13 +395,13 @@ export default class GeneralUserSettingsTab extends React.Component -

+ {_t( "Your account details are managed separately at %(hostname)s.", { hostname }, { code: (sub) => {sub} }, )} -

+ - {_t("Account")} - {externalAccountManagement} -

{passwordChangeText}

- {passwordChangeForm} + <> + + {externalAccountManagement} + {passwordChangeSection} + {threepidSection} - + ); } @@ -540,7 +549,11 @@ export default class GeneralUserSettingsTab extends React.Component ); - discoverySection = {this.renderDiscoverySection()}; + discoverySection = ( + + {this.renderDiscoverySection()} + + ); } return ( diff --git a/test/components/views/settings/discovery/__snapshots__/PhoneNumbers-test.tsx.snap b/test/components/views/settings/discovery/__snapshots__/PhoneNumbers-test.tsx.snap index ef9287b2b0..f63ad7ca62 100644 --- a/test/components/views/settings/discovery/__snapshots__/PhoneNumbers-test.tsx.snap +++ b/test/components/views/settings/discovery/__snapshots__/PhoneNumbers-test.tsx.snap @@ -4,7 +4,7 @@ exports[` should handle no numbers 1`] = `
should render a loader while loading 1`] = `
should render phone numbers 1`] = `