Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Use semantic headings in user settings - account #10972

Merged
merged 7 commits into from
May 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 27 additions & 24 deletions cypress/e2e/settings/general-user-settings-tab.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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")
Expand Down Expand Up @@ -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
Expand Down
8 changes: 2 additions & 6 deletions res/css/views/elements/_TagComposer.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
18 changes: 18 additions & 0 deletions res/css/views/settings/tabs/_SettingsTab.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
32 changes: 0 additions & 32 deletions res/css/views/settings/tabs/user/_GeneralUserSettingsTab.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions src/components/views/settings/account/EmailAddresses.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -289,7 +289,7 @@ export default class EmailAddresses extends React.Component<IProps, IState> {
/>
{addButton}
</form>
</div>
</>
);
}
}
4 changes: 2 additions & 2 deletions src/components/views/settings/account/PhoneNumbers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand All @@ -321,7 +321,7 @@ export default class PhoneNumbers extends React.Component<IProps, IState> {
</div>
</form>
{addVerifySection}
</div>
</>
);
}
}
2 changes: 1 addition & 1 deletion src/components/views/settings/discovery/PhoneNumbers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand All @@ -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;
Expand All @@ -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"
Expand All @@ -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>
</>
);
}

Expand Down Expand Up @@ -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 (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down