From 44c0db2664a74fa78ddb386bd3c8430f32e730a0 Mon Sep 17 00:00:00 2001 From: David Langley Date: Thu, 19 Sep 2024 15:22:17 +0100 Subject: [PATCH 1/7] Mobile registration optimizations - don't autocaptialize or autocorrect on username field - show each password field in their own row - improve position of tooltip on mobile so that it's visible --- .../structures/auth/Registration.tsx | 1 + src/components/views/auth/EmailField.tsx | 3 ++ .../views/auth/PassphraseConfirmField.tsx | 4 +- src/components/views/auth/PassphraseField.tsx | 3 ++ .../views/auth/RegistrationForm.tsx | 37 +++++++++++++++++-- src/components/views/elements/Field.tsx | 7 +++- 6 files changed, 48 insertions(+), 7 deletions(-) diff --git a/src/components/structures/auth/Registration.tsx b/src/components/structures/auth/Registration.tsx index 2dc9125362..f69bbc939b 100644 --- a/src/components/structures/auth/Registration.tsx +++ b/src/components/structures/auth/Registration.tsx @@ -627,6 +627,7 @@ export default class Registration extends React.Component { serverConfig={this.props.serverConfig} canSubmit={!this.state.serverErrorIsFatal} matrixClient={this.state.matrixClient} + mobileRegister={this.props.mobileRegister} /> ); diff --git a/src/components/views/auth/EmailField.tsx b/src/components/views/auth/EmailField.tsx index e14e3920fd..acd5597259 100644 --- a/src/components/views/auth/EmailField.tsx +++ b/src/components/views/auth/EmailField.tsx @@ -12,6 +12,7 @@ import Field, { IInputProps } from "../elements/Field"; import { _t, _td, TranslationKey } from "../../../languageHandler"; import withValidation, { IFieldState, IValidationResult } from "../elements/Validation"; import * as Email from "../../../email"; +import { Alignment } from "../elements/Tooltip"; interface IProps extends Omit { id?: string; @@ -22,6 +23,7 @@ interface IProps extends Omit { label: TranslationKey; labelRequired: TranslationKey; labelInvalid: TranslationKey; + tooltipAlignment?: Alignment; // When present, completely overrides the default validation rules. validationRules?: (fieldState: IFieldState) => Promise; @@ -77,6 +79,7 @@ class EmailField extends PureComponent { autoFocus={this.props.autoFocus} onChange={this.props.onChange} onValidate={this.onValidate} + tooltipAlignment={this.props.tooltipAlignment} /> ); } diff --git a/src/components/views/auth/PassphraseConfirmField.tsx b/src/components/views/auth/PassphraseConfirmField.tsx index b72f61310d..ec26099ded 100644 --- a/src/components/views/auth/PassphraseConfirmField.tsx +++ b/src/components/views/auth/PassphraseConfirmField.tsx @@ -11,6 +11,7 @@ import React, { PureComponent, RefCallback, RefObject } from "react"; import Field, { IInputProps } from "../elements/Field"; import withValidation, { IFieldState, IValidationResult } from "../elements/Validation"; import { _t, _td, TranslationKey } from "../../../languageHandler"; +import { Alignment } from "../elements/Tooltip"; interface IProps extends Omit { id?: string; @@ -22,7 +23,7 @@ interface IProps extends Omit { label: TranslationKey; labelRequired: TranslationKey; labelInvalid: TranslationKey; - + tooltipAlignment?: Alignment; onChange(ev: React.FormEvent): void; onValidate?(result: IValidationResult): void; } @@ -70,6 +71,7 @@ class PassphraseConfirmField extends PureComponent { onChange={this.props.onChange} onValidate={this.onValidate} autoFocus={this.props.autoFocus} + tooltipAlignment={this.props.tooltipAlignment} /> ); } diff --git a/src/components/views/auth/PassphraseField.tsx b/src/components/views/auth/PassphraseField.tsx index 985cf7724d..6770b141a5 100644 --- a/src/components/views/auth/PassphraseField.tsx +++ b/src/components/views/auth/PassphraseField.tsx @@ -15,6 +15,7 @@ import withValidation, { IFieldState, IValidationResult } from "../elements/Vali import { _t, _td, TranslationKey } from "../../../languageHandler"; import Field, { IInputProps } from "../elements/Field"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; +import { Alignment } from "../elements/Tooltip"; interface IProps extends Omit { autoFocus?: boolean; @@ -30,6 +31,7 @@ interface IProps extends Omit { labelEnterPassword: TranslationKey; labelStrongPassword: TranslationKey; labelAllowedButUnsafe: TranslationKey; + tooltipAlignment?: Alignment; onChange(ev: React.FormEvent): void; onValidate?(result: IValidationResult): void; @@ -111,6 +113,7 @@ class PassphraseField extends PureComponent { value={this.props.value} onChange={this.props.onChange} onValidate={this.onValidate} + tooltipAlignment={this.props.tooltipAlignment} /> ); } diff --git a/src/components/views/auth/RegistrationForm.tsx b/src/components/views/auth/RegistrationForm.tsx index c8f7fd3d0f..fb8aacb964 100644 --- a/src/components/views/auth/RegistrationForm.tsx +++ b/src/components/views/auth/RegistrationForm.tsx @@ -26,6 +26,7 @@ import RegistrationEmailPromptDialog from "../dialogs/RegistrationEmailPromptDia import CountryDropdown from "./CountryDropdown"; import PassphraseConfirmField from "./PassphraseConfirmField"; import { PosthogAnalytics } from "../../../PosthogAnalytics"; +import { Alignment } from "../elements/Tooltip"; enum RegistrationField { Email = "field_email", @@ -58,6 +59,7 @@ interface IProps { serverConfig: ValidatedServerConfig; canSubmit?: boolean; matrixClient: MatrixClient; + mobileRegister?: boolean; onRegisterClick(params: { username: string; @@ -439,6 +441,13 @@ export default class RegistrationForm extends React.PureComponent ); } @@ -468,6 +478,7 @@ export default class RegistrationForm extends React.PureComponent ); } @@ -482,6 +493,7 @@ export default class RegistrationForm extends React.PureComponent ); } @@ -526,6 +538,9 @@ export default class RegistrationForm extends React.PureComponent ); } @@ -557,14 +572,28 @@ export default class RegistrationForm extends React.PureComponent +
{this.renderPassword()}
+
{this.renderPasswordConfirm()}
+ + ); + } else { + passwordFields = ( +
+ {this.renderPassword()} + {this.renderPasswordConfirm()} +
+ ); + } + return (
{this.renderUsername()}
-
- {this.renderPassword()} - {this.renderPasswordConfirm()} -
+ {passwordFields}
{this.renderEmail()} {this.renderPhoneNumber()} diff --git a/src/components/views/elements/Field.tsx b/src/components/views/elements/Field.tsx index 4326f63cd7..32b1d5d093 100644 --- a/src/components/views/elements/Field.tsx +++ b/src/components/views/elements/Field.tsx @@ -17,7 +17,7 @@ import classNames from "classnames"; import { debounce } from "lodash"; import { IFieldState, IValidationResult } from "./Validation"; -import Tooltip from "./Tooltip"; +import Tooltip, { Alignment } from "./Tooltip"; import { Key } from "../../../Keyboard"; // Invoke validation from user input (when typing, etc.) at most once every N ms. @@ -60,6 +60,8 @@ interface IProps { tooltipContent?: React.ReactNode; // If specified the tooltip will be shown regardless of feedback forceTooltipVisible?: boolean; + // If specified, the tooltip with be aligned accorindly with the field, defaults to RIGHT. + tooltipAlignment?: Alignment; // If specified alongside tooltipContent, the class name to apply to the // tooltip itself. tooltipClassName?: string; @@ -125,6 +127,7 @@ export default class Field extends React.PureComponent { validateOnFocus: true, validateOnBlur: true, validateOnChange: true, + tooltipAlignment: Tooltip.Alignment.Right, }; /* @@ -286,7 +289,7 @@ export default class Field extends React.PureComponent { tooltipClassName={classNames("mx_Field_tooltip", "mx_Tooltip_noMargin", tooltipClassName)} visible={visible} label={tooltipContent || this.state.feedback} - alignment={Tooltip.Alignment.Right} + alignment={this.props.tooltipAlignment} role={role} /> ); From 13777581467ffb2a42d32a025e789aaf89550b59 Mon Sep 17 00:00:00 2001 From: David Langley Date: Thu, 19 Sep 2024 16:00:23 +0100 Subject: [PATCH 2/7] Use optional prop rather than default prop. --- src/components/views/elements/Field.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/views/elements/Field.tsx b/src/components/views/elements/Field.tsx index 32b1d5d093..6cc5dffc40 100644 --- a/src/components/views/elements/Field.tsx +++ b/src/components/views/elements/Field.tsx @@ -60,7 +60,7 @@ interface IProps { tooltipContent?: React.ReactNode; // If specified the tooltip will be shown regardless of feedback forceTooltipVisible?: boolean; - // If specified, the tooltip with be aligned accorindly with the field, defaults to RIGHT. + // If specified, the tooltip with be aligned accorindly with the field, defaults to Right. tooltipAlignment?: Alignment; // If specified alongside tooltipContent, the class name to apply to the // tooltip itself. @@ -127,7 +127,6 @@ export default class Field extends React.PureComponent { validateOnFocus: true, validateOnBlur: true, validateOnChange: true, - tooltipAlignment: Tooltip.Alignment.Right, }; /* @@ -264,6 +263,7 @@ export default class Field extends React.PureComponent { validateOnFocus, usePlaceholderAsHint, forceTooltipVisible, + tooltipAlignment, ...inputProps } = this.props; @@ -289,7 +289,7 @@ export default class Field extends React.PureComponent { tooltipClassName={classNames("mx_Field_tooltip", "mx_Tooltip_noMargin", tooltipClassName)} visible={visible} label={tooltipContent || this.state.feedback} - alignment={this.props.tooltipAlignment} + alignment={tooltipAlignment || Alignment.Right} role={role} /> ); From a04e1a5a452ba0315191c75afca512c0b1a3c759 Mon Sep 17 00:00:00 2001 From: David Langley Date: Thu, 19 Sep 2024 16:01:13 +0100 Subject: [PATCH 3/7] Redirect to welcome screen if mobile_registration is requested but not enabled in the config. --- src/components/structures/MatrixChat.tsx | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index 8e0eaabe4f..1726c8462d 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -952,18 +952,20 @@ export default class MatrixChat extends React.PureComponent { } private async startRegistration(params: { [key: string]: string }, isMobileRegistration?: boolean): Promise { - if (!SettingsStore.getValue(UIFeature.Registration)) { + // If registration is disabled or mobile registration is requested but not enabled in settings redirect to the welcome screen + if ( + !SettingsStore.getValue(UIFeature.Registration) || + (isMobileRegistration && !SettingsStore.getValue("Registration.mobileRegistrationHelper")) + ) { this.showScreen("welcome"); return; } - const isMobileRegistrationAllowed = - isMobileRegistration && SettingsStore.getValue("Registration.mobileRegistrationHelper"); const newState: Partial = { view: Views.REGISTER, }; - if (isMobileRegistrationAllowed && params.hs_url) { + if (isMobileRegistration && params.hs_url) { try { const config = await AutoDiscoveryUtils.validateServerConfigWithStaticUrls(params.hs_url); newState.serverConfig = config; @@ -992,12 +994,12 @@ export default class MatrixChat extends React.PureComponent { newState.register_id_sid = params.sid; } - newState.isMobileRegistration = isMobileRegistrationAllowed; + newState.isMobileRegistration = isMobileRegistration; this.setStateForNewView(newState); ThemeController.isLogin = true; this.themeWatcher.recheck(); - this.notifyNewScreen(isMobileRegistrationAllowed ? "mobile_register" : "register"); + this.notifyNewScreen(isMobileRegistration ? "mobile_register" : "register"); } // switch view to the given room From 4d8af8d18957f03616722ef322e6c83177f9d9f8 Mon Sep 17 00:00:00 2001 From: David Langley Date: Fri, 20 Sep 2024 08:49:33 +0100 Subject: [PATCH 4/7] autocorrect value should be "off" --- src/components/views/auth/RegistrationForm.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/auth/RegistrationForm.tsx b/src/components/views/auth/RegistrationForm.tsx index fb8aacb964..4df3313758 100644 --- a/src/components/views/auth/RegistrationForm.tsx +++ b/src/components/views/auth/RegistrationForm.tsx @@ -539,7 +539,7 @@ export default class RegistrationForm extends React.PureComponent ); From efcd61083f5b04176c7e6d9f5fc3be8fcdc91777 Mon Sep 17 00:00:00 2001 From: David Langley Date: Fri, 20 Sep 2024 11:16:29 +0100 Subject: [PATCH 5/7] Add unit tests for mobile registration --- .../structures/auth/Registration.tsx | 6 ++- .../components/structures/MatrixChat-test.tsx | 39 +++++++++++++++ .../structures/auth/Registration-test.tsx | 47 +++++++++++++++++-- 3 files changed, 86 insertions(+), 6 deletions(-) diff --git a/src/components/structures/auth/Registration.tsx b/src/components/structures/auth/Registration.tsx index f69bbc939b..0ae5c93346 100644 --- a/src/components/structures/auth/Registration.tsx +++ b/src/components/structures/auth/Registration.tsx @@ -780,7 +780,11 @@ export default class Registration extends React.Component { ); } if (this.props.mobileRegister) { - return
{body}
; + return ( +
+ {body} +
+ ); } return ( diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index bae633b159..16809bcac0 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -55,6 +55,7 @@ import { MatrixClientPeg as peg } from "../../../src/MatrixClientPeg"; import DMRoomMap from "../../../src/utils/DMRoomMap"; import { ReleaseAnnouncementStore } from "../../../src/stores/ReleaseAnnouncementStore"; import { DRAFT_LAST_CLEANUP_KEY } from "../../../src/DraftCleaner"; +import { UIFeature } from "../../../src/settings/UIFeature"; jest.mock("matrix-js-sdk/src/oidc/authorize", () => ({ completeAuthorizationCodeGrant: jest.fn(), @@ -1462,4 +1463,42 @@ describe("", () => { }); }); }); + + describe("mobile registration", () => { + const getComponentAndWaitForReady = async (): Promise => { + const renderResult = getComponent(); + // wait for welcome page chrome render + await screen.findByText("powered by Matrix"); + + // go to login page + defaultDispatcher.dispatch({ + action: "start_mobile_registration", + }); + + await flushPromises(); + + return renderResult; + }; + + const enabledMobileRegistration = (): void => { + jest.spyOn(SettingsStore, "getValue").mockImplementation((settingName: string) => { + if (settingName === "Registration.mobileRegistrationHelper") return true; + if (settingName === UIFeature.Registration) return true; + }); + }; + + it("should render welcome screen if mobile registration is not enabled in settings", async () => { + await getComponentAndWaitForReady(); + + await screen.findByText("powered by Matrix"); + }); + + it("should render mobile registration", async () => { + enabledMobileRegistration(); + + await getComponentAndWaitForReady(); + + expect(screen.getByTestId("mobile-register")).toBeInTheDocument(); + }); + }); }); diff --git a/test/components/structures/auth/Registration-test.tsx b/test/components/structures/auth/Registration-test.tsx index c31eff9b7c..e30f0ad6ee 100644 --- a/test/components/structures/auth/Registration-test.tsx +++ b/test/components/structures/auth/Registration-test.tsx @@ -8,7 +8,7 @@ Please see LICENSE files in the repository root for full details. */ import React from "react"; -import { fireEvent, render, screen, waitForElementToBeRemoved } from "@testing-library/react"; +import { fireEvent, render, screen, waitFor, waitForElementToBeRemoved } from "@testing-library/react"; import { createClient, MatrixClient, MatrixError, OidcClientConfig } from "matrix-js-sdk/src/matrix"; import { mocked, MockedObject } from "jest-mock"; import fetchMock from "fetch-mock-jest"; @@ -87,12 +87,23 @@ describe("Registration", function () { const defaultHsUrl = "https://matrix.org"; const defaultIsUrl = "https://vector.im"; - function getRawComponent(hsUrl = defaultHsUrl, isUrl = defaultIsUrl, authConfig?: OidcClientConfig) { - return ; + function getRawComponent( + hsUrl = defaultHsUrl, + isUrl = defaultIsUrl, + authConfig?: OidcClientConfig, + mobileRegister?: boolean, + ) { + return ( + + ); } - function getComponent(hsUrl?: string, isUrl?: string, authConfig?: OidcClientConfig) { - return render(getRawComponent(hsUrl, isUrl, authConfig)); + function getComponent(hsUrl?: string, isUrl?: string, authConfig?: OidcClientConfig, mobileRegister?: boolean) { + return render(getRawComponent(hsUrl, isUrl, authConfig, mobileRegister)); } it("should show server picker", async function () { @@ -208,5 +219,31 @@ describe("Registration", function () { ); }); }); + + describe("when is mobile registeration", () => { + it("should not show server picker", async function () { + const { container } = getComponent(defaultHsUrl, defaultIsUrl, undefined, true); + expect(container.querySelector(".mx_ServerPicker")).toBeFalsy(); + }); + + it("should show username field with autocaps disabled", async function () { + const { container } = getComponent(defaultHsUrl, defaultIsUrl, undefined, true); + + await waitFor(() => + expect(container.querySelector("#mx_RegistrationForm_username")).toHaveAttribute( + "autocapitalize", + "none", + ), + ); + }); + + it("should show password and confirm password fields in separate rows", async function () { + const { container } = getComponent(defaultHsUrl, defaultIsUrl, undefined, true); + + await waitFor(() => expect(container.querySelector("#mx_RegistrationForm_username")).toBeTruthy); + // when password and confirm password fields are in separate rows there should be 4 rather than 3 + expect(container.querySelectorAll(".mx_AuthBody_fieldRow")).toHaveLength(4); + }); + }); }); }); From 87a8adb5555f63877aa1da8c02fa40984e17885f Mon Sep 17 00:00:00 2001 From: David Langley Date: Fri, 20 Sep 2024 11:34:53 +0100 Subject: [PATCH 6/7] Fix test typo --- test/components/structures/auth/Registration-test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/components/structures/auth/Registration-test.tsx b/test/components/structures/auth/Registration-test.tsx index e30f0ad6ee..9a83f00a9d 100644 --- a/test/components/structures/auth/Registration-test.tsx +++ b/test/components/structures/auth/Registration-test.tsx @@ -240,7 +240,7 @@ describe("Registration", function () { it("should show password and confirm password fields in separate rows", async function () { const { container } = getComponent(defaultHsUrl, defaultIsUrl, undefined, true); - await waitFor(() => expect(container.querySelector("#mx_RegistrationForm_username")).toBeTruthy); + await waitFor(() => expect(container.querySelector("#mx_RegistrationForm_username")).toBeTruthy()); // when password and confirm password fields are in separate rows there should be 4 rather than 3 expect(container.querySelectorAll(".mx_AuthBody_fieldRow")).toHaveLength(4); }); From 003f7b1b3b74c85f02a0f14d958d7ccbcd2f597c Mon Sep 17 00:00:00 2001 From: David Langley Date: Fri, 20 Sep 2024 12:08:31 +0100 Subject: [PATCH 7/7] Fix typo --- test/components/structures/MatrixChat-test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index 16809bcac0..1003d1d167 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -1470,7 +1470,7 @@ describe("", () => { // wait for welcome page chrome render await screen.findByText("powered by Matrix"); - // go to login page + // go to mobile_register page defaultDispatcher.dispatch({ action: "start_mobile_registration", });