From 1f5571062ea8ffc7d89f6adf6b57ecaac7c333be Mon Sep 17 00:00:00 2001
From: David Langley <davidl@element.io>
Date: Fri, 20 Sep 2024 12:24:39 +0100
Subject: [PATCH] Mobile registration optimizations and tests (#62)
* 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
* Use optional prop rather than default prop.
* Redirect to welcome screen if mobile_registration is requested but not enabled in the config.
* autocorrect value should be "off"
* Add unit tests for mobile registration
* Fix test typo
* Fix typo
---
src/components/structures/MatrixChat.tsx | 14 +++---
.../structures/auth/Registration.tsx | 7 ++-
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 ++-
.../components/structures/MatrixChat-test.tsx | 39 +++++++++++++++
.../structures/auth/Registration-test.tsx | 47 +++++++++++++++++--
9 files changed, 142 insertions(+), 19 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<IProps, IState> {
}
private async startRegistration(params: { [key: string]: string }, isMobileRegistration?: boolean): Promise<void> {
- 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<IState> = {
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<IProps, IState> {
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
diff --git a/src/components/structures/auth/Registration.tsx b/src/components/structures/auth/Registration.tsx
index 2dc9125362..0ae5c93346 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<IProps, IState> {
serverConfig={this.props.serverConfig}
canSubmit={!this.state.serverErrorIsFatal}
matrixClient={this.state.matrixClient}
+ mobileRegister={this.props.mobileRegister}
/>
</React.Fragment>
);
@@ -779,7 +780,11 @@ export default class Registration extends React.Component<IProps, IState> {
);
}
if (this.props.mobileRegister) {
- return <div className="mx_MobileRegister_body">{body}</div>;
+ return (
+ <div className="mx_MobileRegister_body" data-testid="mobile-register">
+ {body}
+ </div>
+ );
}
return (
<AuthPage>
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<IInputProps, "onValidate" | "element"> {
id?: string;
@@ -22,6 +23,7 @@ interface IProps extends Omit<IInputProps, "onValidate" | "element"> {
label: TranslationKey;
labelRequired: TranslationKey;
labelInvalid: TranslationKey;
+ tooltipAlignment?: Alignment;
// When present, completely overrides the default validation rules.
validationRules?: (fieldState: IFieldState) => Promise<IValidationResult>;
@@ -77,6 +79,7 @@ class EmailField extends PureComponent<IProps> {
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<IInputProps, "onValidate" | "label" | "element"> {
id?: string;
@@ -22,7 +23,7 @@ interface IProps extends Omit<IInputProps, "onValidate" | "label" | "element"> {
label: TranslationKey;
labelRequired: TranslationKey;
labelInvalid: TranslationKey;
-
+ tooltipAlignment?: Alignment;
onChange(ev: React.FormEvent<HTMLElement>): void;
onValidate?(result: IValidationResult): void;
}
@@ -70,6 +71,7 @@ class PassphraseConfirmField extends PureComponent<IProps> {
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<IInputProps, "onValidate" | "element"> {
autoFocus?: boolean;
@@ -30,6 +31,7 @@ interface IProps extends Omit<IInputProps, "onValidate" | "element"> {
labelEnterPassword: TranslationKey;
labelStrongPassword: TranslationKey;
labelAllowedButUnsafe: TranslationKey;
+ tooltipAlignment?: Alignment;
onChange(ev: React.FormEvent<HTMLElement>): void;
onValidate?(result: IValidationResult): void;
@@ -111,6 +113,7 @@ class PassphraseField extends PureComponent<IProps> {
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..4df3313758 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<IProps, IState
return true;
}
+ private tooltipAlignment(): Alignment | undefined {
+ if (this.props.mobileRegister) {
+ return Alignment.Bottom;
+ }
+ return undefined;
+ }
+
private renderEmail(): ReactNode {
if (!this.showEmail()) {
return null;
@@ -454,6 +463,7 @@ export default class RegistrationForm extends React.PureComponent<IProps, IState
validationRules={this.validateEmailRules.bind(this)}
onChange={this.onEmailChange}
onValidate={this.onEmailValidate}
+ tooltipAlignment={this.tooltipAlignment()}
/>
);
}
@@ -468,6 +478,7 @@ export default class RegistrationForm extends React.PureComponent<IProps, IState
onChange={this.onPasswordChange}
onValidate={this.onPasswordValidate}
userInputs={[this.state.username]}
+ tooltipAlignment={this.tooltipAlignment()}
/>
);
}
@@ -482,6 +493,7 @@ export default class RegistrationForm extends React.PureComponent<IProps, IState
password={this.state.password}
onChange={this.onPasswordConfirmChange}
onValidate={this.onPasswordConfirmValidate}
+ tooltipAlignment={this.tooltipAlignment()}
/>
);
}
@@ -526,6 +538,9 @@ export default class RegistrationForm extends React.PureComponent<IProps, IState
value={this.state.username}
onChange={this.onUsernameChange}
onValidate={this.onUsernameValidate}
+ tooltipAlignment={this.tooltipAlignment()}
+ autoCorrect="off"
+ autoCapitalize="none"
/>
);
}
@@ -557,14 +572,28 @@ export default class RegistrationForm extends React.PureComponent<IProps, IState
}
}
+ let passwordFields: JSX.Element | undefined;
+ if (this.props.mobileRegister) {
+ passwordFields = (
+ <>
+ <div className="mx_AuthBody_fieldRow">{this.renderPassword()}</div>
+ <div className="mx_AuthBody_fieldRow">{this.renderPasswordConfirm()}</div>
+ </>
+ );
+ } else {
+ passwordFields = (
+ <div className="mx_AuthBody_fieldRow">
+ {this.renderPassword()}
+ {this.renderPasswordConfirm()}
+ </div>
+ );
+ }
+
return (
<div>
<form onSubmit={this.onSubmit}>
<div className="mx_AuthBody_fieldRow">{this.renderUsername()}</div>
- <div className="mx_AuthBody_fieldRow">
- {this.renderPassword()}
- {this.renderPasswordConfirm()}
- </div>
+ {passwordFields}
<div className="mx_AuthBody_fieldRow">
{this.renderEmail()}
{this.renderPhoneNumber()}
diff --git a/src/components/views/elements/Field.tsx b/src/components/views/elements/Field.tsx
index 4326f63cd7..6cc5dffc40 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;
@@ -261,6 +263,7 @@ export default class Field extends React.PureComponent<PropShapes, IState> {
validateOnFocus,
usePlaceholderAsHint,
forceTooltipVisible,
+ tooltipAlignment,
...inputProps
} = this.props;
@@ -286,7 +289,7 @@ export default class Field extends React.PureComponent<PropShapes, IState> {
tooltipClassName={classNames("mx_Field_tooltip", "mx_Tooltip_noMargin", tooltipClassName)}
visible={visible}
label={tooltipContent || this.state.feedback}
- alignment={Tooltip.Alignment.Right}
+ alignment={tooltipAlignment || Alignment.Right}
role={role}
/>
);
diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx
index bae633b159..1003d1d167 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("<MatrixChat />", () => {
});
});
});
+
+ describe("mobile registration", () => {
+ const getComponentAndWaitForReady = async (): Promise<RenderResult> => {
+ const renderResult = getComponent();
+ // wait for welcome page chrome render
+ await screen.findByText("powered by Matrix");
+
+ // go to mobile_register 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..9a83f00a9d 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 <Registration {...defaultProps} serverConfig={mkServerConfig(hsUrl, isUrl, authConfig)} />;
+ function getRawComponent(
+ hsUrl = defaultHsUrl,
+ isUrl = defaultIsUrl,
+ authConfig?: OidcClientConfig,
+ mobileRegister?: boolean,
+ ) {
+ return (
+ <Registration
+ {...defaultProps}
+ serverConfig={mkServerConfig(hsUrl, isUrl, authConfig)}
+ mobileRegister={mobileRegister}
+ />
+ );
}
- 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);
+ });
+ });
});
});