From 9f011b955ba34d24cfc69aab576478ab1d4ef862 Mon Sep 17 00:00:00 2001 From: Kerry Date: Wed, 24 May 2023 14:37:10 +1200 Subject: [PATCH] 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 --- res/css/structures/_RoomStatusBar.pcss | 2 +- res/css/views/settings/_SetIdServer.pcss | 14 ++- res/css/views/settings/_SettingsFieldset.pcss | 3 - .../tabs/user/_GeneralUserSettingsTab.pcss | 4 +- .../feather-customised/warning-triangle.svg | 2 +- src/components/structures/RoomStatusBar.tsx | 8 +- src/components/views/settings/SetIdServer.tsx | 58 +++++----- .../views/settings/SettingsFieldset.tsx | 8 +- .../settings/discovery/EmailAddresses.tsx | 27 +++-- .../views/settings/discovery/PhoneNumbers.tsx | 27 +++-- .../settings/shared/SettingsSubsection.tsx | 18 ++-- .../tabs/user/GeneralUserSettingsTab.tsx | 66 ++++++------ .../SettingsFieldset-test.tsx.snap | 24 +++-- .../discovery/EmailAddresses-test.tsx | 22 +++- .../settings/discovery/PhoneNumbers-test.tsx | 37 +++++-- .../EmailAddresses-test.tsx.snap | 97 +++++++++++++++++ .../__snapshots__/PhoneNumbers-test.tsx.snap | 101 ++++++++++++++++++ .../SecurityRoomSettingsTab-test.tsx.snap | 6 +- .../SpaceSettingsVisibilityTab-test.tsx.snap | 6 +- 19 files changed, 407 insertions(+), 123 deletions(-) create mode 100644 test/components/views/settings/discovery/__snapshots__/EmailAddresses-test.tsx.snap create mode 100644 test/components/views/settings/discovery/__snapshots__/PhoneNumbers-test.tsx.snap diff --git a/res/css/structures/_RoomStatusBar.pcss b/res/css/structures/_RoomStatusBar.pcss index d3e08adfd6..60191c3452 100644 --- a/res/css/structures/_RoomStatusBar.pcss +++ b/res/css/structures/_RoomStatusBar.pcss @@ -160,7 +160,7 @@ limitations under the License. } } -.mx_RoomStatusBar_connectionLostBar img { +.mx_RoomStatusBar_connectionLostBar svg { padding-left: 10px; padding-right: 10px; vertical-align: middle; diff --git a/res/css/views/settings/_SetIdServer.pcss b/res/css/views/settings/_SetIdServer.pcss index 8dc634400c..7813a35422 100644 --- a/res/css/views/settings/_SetIdServer.pcss +++ b/res/css/views/settings/_SetIdServer.pcss @@ -1,5 +1,5 @@ /* -Copyright 2019 The Matrix.org Foundation C.I.C. +Copyright 2019, 2023 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. @@ -14,8 +14,16 @@ See the License for the specific language governing permissions and limitations under the License. */ -.mx_SetIdServer .mx_Field_input { - margin-inline-end: var(--SettingsTab_fullWidthField-margin-inline-end); +.mx_SetIdServer { + display: flex; + flex-direction: column; + align-items: flex-start; + gap: $spacing-8; + + .mx_Field { + width: 100%; + margin: 0; + } } .mx_SetIdServer_tooltip { diff --git a/res/css/views/settings/_SettingsFieldset.pcss b/res/css/views/settings/_SettingsFieldset.pcss index 556fcdf8eb..5acf3754a7 100644 --- a/res/css/views/settings/_SettingsFieldset.pcss +++ b/res/css/views/settings/_SettingsFieldset.pcss @@ -15,7 +15,6 @@ limitations under the License. */ .mx_SettingsFieldset { - margin: 10px 80px 10px 0; box-sizing: content-box; } @@ -31,8 +30,6 @@ limitations under the License. } .mx_SettingsFieldset_description { - color: $settings-subsection-fg-color; - font-size: $font-14px; display: block; margin-top: 0; margin-bottom: 10px; diff --git a/res/css/views/settings/tabs/user/_GeneralUserSettingsTab.pcss b/res/css/views/settings/tabs/user/_GeneralUserSettingsTab.pcss index 7b8ff43f62..a0e81ce115 100644 --- a/res/css/views/settings/tabs/user/_GeneralUserSettingsTab.pcss +++ b/res/css/views/settings/tabs/user/_GeneralUserSettingsTab.pcss @@ -61,6 +61,8 @@ limitations under the License. margin-left: 5px; } -.mx_GeneralUserSettingsTab_heading_warningIcon { +.mx_GeneralUserSettingsTab_warningIcon { vertical-align: middle; + margin-right: $spacing-8; + margin-bottom: 2px; } diff --git a/res/img/feather-customised/warning-triangle.svg b/res/img/feather-customised/warning-triangle.svg index 3d18e30fb2..29a9f85b5a 100644 --- a/res/img/feather-customised/warning-triangle.svg +++ b/res/img/feather-customised/warning-triangle.svg @@ -3,7 +3,7 @@ {
- +
{_t("Connectivity to the server has been lost.")} diff --git a/src/components/views/settings/SetIdServer.tsx b/src/components/views/settings/SetIdServer.tsx index 9b3ad376cd..df2f992601 100644 --- a/src/components/views/settings/SetIdServer.tsx +++ b/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 { discoButtonContent = ; } discoSection = ( -
- {discoBodyText} + <> + {discoBodyText} {discoButtonContent} -
+ ); } return ( -
- {sectionTitle} - {bodyText} - - - {_t("Change")} - - {discoSection} - + +
+ + + {_t("Change")} + + {discoSection} + +
); } } diff --git a/src/components/views/settings/SettingsFieldset.tsx b/src/components/views/settings/SettingsFieldset.tsx index 42d184d29b..617564aed3 100644 --- a/src/components/views/settings/SettingsFieldset.tsx +++ b/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 { // section title legend: string | ReactNode; @@ -24,7 +26,11 @@ interface Props extends HTMLAttributes { const SettingsFieldset: React.FC = ({ legend, className, children, description, ...rest }) => (
{legend} - {description &&
{description}
} + {description && ( +
+ {description} +
+ )} {children}
); diff --git a/src/components/views/settings/discovery/EmailAddresses.tsx b/src/components/views/settings/discovery/EmailAddresses.tsx index 3a34156083..c848b1722c 100644 --- a/src/components/views/settings/discovery/EmailAddresses.tsx +++ b/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 { public render(): React.ReactNode { let content; - if (this.props.emails.length > 0) { + if (this.props.isLoading) { + content = ; + } else if (this.props.emails.length > 0) { content = this.props.emails.map((e) => { return ; }); - } else { - content = ( - - {_t("Discovery options will appear once you have added an email above.")} - - ); } - return
{content}
; + const hasEmails = !!this.props.emails.length; + + return ( + + {content} + + ); } } diff --git a/src/components/views/settings/discovery/PhoneNumbers.tsx b/src/components/views/settings/discovery/PhoneNumbers.tsx index 29e5184301..e9e67490d6 100644 --- a/src/components/views/settings/discovery/PhoneNumbers.tsx +++ b/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 { public render(): React.ReactNode { let content; - if (this.props.msisdns.length > 0) { + if (this.props.isLoading) { + content = ; + } else if (this.props.msisdns.length > 0) { content = this.props.msisdns.map((e) => { return ; }); - } else { - content = ( - - {_t("Discovery options will appear once you have added a phone number above.")} - - ); } - return
{content}
; + const description = + (!content && _t("Discovery options will appear once you have added a phone number above.")) || undefined; + + return ( + + {content} + + ); } } diff --git a/src/components/views/settings/shared/SettingsSubsection.tsx b/src/components/views/settings/shared/SettingsSubsection.tsx index ab36e9bfea..035306f5f3 100644 --- a/src/components/views/settings/shared/SettingsSubsection.tsx +++ b/src/components/views/settings/shared/SettingsSubsection.tsx @@ -47,14 +47,16 @@ export const SettingsSubsection: React.FC = ({ {description}
)} -
- {children} -
+ {!!children && ( +
+ {children} +
+ )}
); diff --git a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.tsx b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.tsx index 7d7e855688..dc896d6546 100644 --- a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.tsx +++ b/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 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 + {_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 }, )} - + ); return ( -
+ <> {/* has its own heading as it includes the current identity server */} -
+ ); } - const emails = this.state.loading3pids ? : ; - const msisdns = this.state.loading3pids ? : ; - const threepidSection = this.state.haveIdServer ? ( <> - {_t("Email addresses")} - {emails} - - {_t("Phone numbers")} - {msisdns} + + ) : null; return ( -
+ <> {threepidSection} {/* has its own heading as it includes the current identity server */} -
+ ); } @@ -520,16 +517,6 @@ export default class GeneralUserSettingsTab extends React.Component - ) : 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 -
- {discoWarning} {_t("Discovery")} -
- {this.renderDiscoverySection()} - + const discoWarning = this.state.requiredPolicyInfo.hasTerms ? ( + + ) : null; + const heading = ( + + {discoWarning} + {_t("Discovery")} + ); + discoverySection = {this.renderDiscoverySection()}; } return ( diff --git a/test/components/views/settings/__snapshots__/SettingsFieldset-test.tsx.snap b/test/components/views/settings/__snapshots__/SettingsFieldset-test.tsx.snap index 77d37ccaa0..968308763a 100644 --- a/test/components/views/settings/__snapshots__/SettingsFieldset-test.tsx.snap +++ b/test/components/views/settings/__snapshots__/SettingsFieldset-test.tsx.snap @@ -14,7 +14,11 @@ exports[` renders fieldset with plain text description 1`] =
- Changes to who can read history. +
+ Changes to who can read history. +
test @@ -37,14 +41,18 @@ exports[` renders fieldset with react description 1`] = `
-

- Test -

- - a link - +

+ Test +

+ + a link + +
test diff --git a/test/components/views/settings/discovery/EmailAddresses-test.tsx b/test/components/views/settings/discovery/EmailAddresses-test.tsx index 27af0df95c..8ff76cd3c0 100644 --- a/test/components/views/settings/discovery/EmailAddresses-test.tsx +++ b/test/components/views/settings/discovery/EmailAddresses-test.tsx @@ -21,7 +21,7 @@ import { IRequestTokenResponse } from "matrix-js-sdk/src/client"; import { MatrixError } from "matrix-js-sdk/src/http-api"; import { UserFriendlyError } from "../../../../../src/languageHandler"; -import { EmailAddress } from "../../../../../src/components/views/settings/discovery/EmailAddresses"; +import EmailAddresses, { EmailAddress } from "../../../../../src/components/views/settings/discovery/EmailAddresses"; import { clearAllModals, getMockClientWithEventEmitter } from "../../../../test-utils"; const mockGetAccessToken = jest.fn().mockResolvedValue("getAccessToken"); @@ -147,3 +147,23 @@ describe("", () => { }); }); }); + +describe("", () => { + it("should render a loader while loading", async () => { + const { container } = render(); + + expect(container).toMatchSnapshot(); + }); + + it("should render email addresses", async () => { + const { container } = render(); + + expect(container).toMatchSnapshot(); + }); + + it("should handle no email addresses", async () => { + const { container } = render(); + + expect(container).toMatchSnapshot(); + }); +}); diff --git a/test/components/views/settings/discovery/PhoneNumbers-test.tsx b/test/components/views/settings/discovery/PhoneNumbers-test.tsx index cac8ed7c42..e8b1e27fc0 100644 --- a/test/components/views/settings/discovery/PhoneNumbers-test.tsx +++ b/test/components/views/settings/discovery/PhoneNumbers-test.tsx @@ -18,18 +18,17 @@ import React from "react"; import { render, screen } from "@testing-library/react"; import { IThreepid, ThreepidMedium } from "matrix-js-sdk/src/@types/threepids"; -import { PhoneNumber } from "../../../../../src/components/views/settings/discovery/PhoneNumbers"; +import PhoneNumbers, { PhoneNumber } from "../../../../../src/components/views/settings/discovery/PhoneNumbers"; +const msisdn: IThreepid = { + medium: ThreepidMedium.Phone, + address: "+441111111111", + validated_at: 12345, + added_at: 12342, + bound: false, +}; describe("", () => { it("should track props.msisdn.bound changes", async () => { - const msisdn: IThreepid = { - medium: ThreepidMedium.Phone, - address: "+441111111111", - validated_at: 12345, - added_at: 12342, - bound: false, - }; - const { rerender } = render(); await screen.findByText("Share"); @@ -38,3 +37,23 @@ describe("", () => { await screen.findByText("Revoke"); }); }); + +describe("", () => { + it("should render a loader while loading", async () => { + const { container } = render(); + + expect(container).toMatchSnapshot(); + }); + + it("should render phone numbers", async () => { + const { container } = render(); + + expect(container).toMatchSnapshot(); + }); + + it("should handle no numbers", async () => { + const { container } = render(); + + expect(container).toMatchSnapshot(); + }); +}); diff --git a/test/components/views/settings/discovery/__snapshots__/EmailAddresses-test.tsx.snap b/test/components/views/settings/discovery/__snapshots__/EmailAddresses-test.tsx.snap new file mode 100644 index 0000000000..43a1f033a3 --- /dev/null +++ b/test/components/views/settings/discovery/__snapshots__/EmailAddresses-test.tsx.snap @@ -0,0 +1,97 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` should handle no email addresses 1`] = ` +
+
+
+

+ Email addresses +

+
+
+
+ Discovery options will appear once you have added an email above. +
+
+
+
+`; + +exports[` should render a loader while loading 1`] = ` +
+
+
+

+ Email addresses +

+
+
+
+
+
+
+
+
+`; + +exports[` should render email addresses 1`] = ` +
+
+
+

+ Email addresses +

+
+
+
+ + foo@bar.com + +
+ Share +
+
+
+
+
+`; diff --git a/test/components/views/settings/discovery/__snapshots__/PhoneNumbers-test.tsx.snap b/test/components/views/settings/discovery/__snapshots__/PhoneNumbers-test.tsx.snap new file mode 100644 index 0000000000..ef9287b2b0 --- /dev/null +++ b/test/components/views/settings/discovery/__snapshots__/PhoneNumbers-test.tsx.snap @@ -0,0 +1,101 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` should handle no numbers 1`] = ` +
+
+
+

+ Phone numbers +

+
+
+
+ Discovery options will appear once you have added a phone number above. +
+
+
+
+`; + +exports[` should render a loader while loading 1`] = ` +
+
+
+

+ Phone numbers +

+
+
+
+
+
+
+
+
+`; + +exports[` should render phone numbers 1`] = ` +
+
+
+

+ Phone numbers +

+
+
+
+ + + + +441111111111 + +
+ Revoke +
+
+
+
+
+`; diff --git a/test/components/views/settings/tabs/room/__snapshots__/SecurityRoomSettingsTab-test.tsx.snap b/test/components/views/settings/tabs/room/__snapshots__/SecurityRoomSettingsTab-test.tsx.snap index 98a6d5a113..28e4382176 100644 --- a/test/components/views/settings/tabs/room/__snapshots__/SecurityRoomSettingsTab-test.tsx.snap +++ b/test/components/views/settings/tabs/room/__snapshots__/SecurityRoomSettingsTab-test.tsx.snap @@ -12,7 +12,11 @@ exports[` history visibility uses shared as default h
- Changes to who can read history will only apply to future messages in this room. The visibility of existing history will be unchanged. +
+ Changes to who can read history will only apply to future messages in this room. The visibility of existing history will be unchanged. +