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
This commit is contained in:
David Langley 2024-09-20 12:24:39 +01:00 committed by GitHub
parent 4be533813e
commit 1f5571062e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 142 additions and 19 deletions

View file

@ -952,18 +952,20 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
} }
private async startRegistration(params: { [key: string]: string }, isMobileRegistration?: boolean): Promise<void> { 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"); this.showScreen("welcome");
return; return;
} }
const isMobileRegistrationAllowed =
isMobileRegistration && SettingsStore.getValue("Registration.mobileRegistrationHelper");
const newState: Partial<IState> = { const newState: Partial<IState> = {
view: Views.REGISTER, view: Views.REGISTER,
}; };
if (isMobileRegistrationAllowed && params.hs_url) { if (isMobileRegistration && params.hs_url) {
try { try {
const config = await AutoDiscoveryUtils.validateServerConfigWithStaticUrls(params.hs_url); const config = await AutoDiscoveryUtils.validateServerConfigWithStaticUrls(params.hs_url);
newState.serverConfig = config; newState.serverConfig = config;
@ -992,12 +994,12 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
newState.register_id_sid = params.sid; newState.register_id_sid = params.sid;
} }
newState.isMobileRegistration = isMobileRegistrationAllowed; newState.isMobileRegistration = isMobileRegistration;
this.setStateForNewView(newState); this.setStateForNewView(newState);
ThemeController.isLogin = true; ThemeController.isLogin = true;
this.themeWatcher.recheck(); this.themeWatcher.recheck();
this.notifyNewScreen(isMobileRegistrationAllowed ? "mobile_register" : "register"); this.notifyNewScreen(isMobileRegistration ? "mobile_register" : "register");
} }
// switch view to the given room // switch view to the given room

View file

@ -627,6 +627,7 @@ export default class Registration extends React.Component<IProps, IState> {
serverConfig={this.props.serverConfig} serverConfig={this.props.serverConfig}
canSubmit={!this.state.serverErrorIsFatal} canSubmit={!this.state.serverErrorIsFatal}
matrixClient={this.state.matrixClient} matrixClient={this.state.matrixClient}
mobileRegister={this.props.mobileRegister}
/> />
</React.Fragment> </React.Fragment>
); );
@ -779,7 +780,11 @@ export default class Registration extends React.Component<IProps, IState> {
); );
} }
if (this.props.mobileRegister) { 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 ( return (
<AuthPage> <AuthPage>

View file

@ -12,6 +12,7 @@ import Field, { IInputProps } from "../elements/Field";
import { _t, _td, TranslationKey } from "../../../languageHandler"; import { _t, _td, TranslationKey } from "../../../languageHandler";
import withValidation, { IFieldState, IValidationResult } from "../elements/Validation"; import withValidation, { IFieldState, IValidationResult } from "../elements/Validation";
import * as Email from "../../../email"; import * as Email from "../../../email";
import { Alignment } from "../elements/Tooltip";
interface IProps extends Omit<IInputProps, "onValidate" | "element"> { interface IProps extends Omit<IInputProps, "onValidate" | "element"> {
id?: string; id?: string;
@ -22,6 +23,7 @@ interface IProps extends Omit<IInputProps, "onValidate" | "element"> {
label: TranslationKey; label: TranslationKey;
labelRequired: TranslationKey; labelRequired: TranslationKey;
labelInvalid: TranslationKey; labelInvalid: TranslationKey;
tooltipAlignment?: Alignment;
// When present, completely overrides the default validation rules. // When present, completely overrides the default validation rules.
validationRules?: (fieldState: IFieldState) => Promise<IValidationResult>; validationRules?: (fieldState: IFieldState) => Promise<IValidationResult>;
@ -77,6 +79,7 @@ class EmailField extends PureComponent<IProps> {
autoFocus={this.props.autoFocus} autoFocus={this.props.autoFocus}
onChange={this.props.onChange} onChange={this.props.onChange}
onValidate={this.onValidate} onValidate={this.onValidate}
tooltipAlignment={this.props.tooltipAlignment}
/> />
); );
} }

View file

@ -11,6 +11,7 @@ import React, { PureComponent, RefCallback, RefObject } from "react";
import Field, { IInputProps } from "../elements/Field"; import Field, { IInputProps } from "../elements/Field";
import withValidation, { IFieldState, IValidationResult } from "../elements/Validation"; import withValidation, { IFieldState, IValidationResult } from "../elements/Validation";
import { _t, _td, TranslationKey } from "../../../languageHandler"; import { _t, _td, TranslationKey } from "../../../languageHandler";
import { Alignment } from "../elements/Tooltip";
interface IProps extends Omit<IInputProps, "onValidate" | "label" | "element"> { interface IProps extends Omit<IInputProps, "onValidate" | "label" | "element"> {
id?: string; id?: string;
@ -22,7 +23,7 @@ interface IProps extends Omit<IInputProps, "onValidate" | "label" | "element"> {
label: TranslationKey; label: TranslationKey;
labelRequired: TranslationKey; labelRequired: TranslationKey;
labelInvalid: TranslationKey; labelInvalid: TranslationKey;
tooltipAlignment?: Alignment;
onChange(ev: React.FormEvent<HTMLElement>): void; onChange(ev: React.FormEvent<HTMLElement>): void;
onValidate?(result: IValidationResult): void; onValidate?(result: IValidationResult): void;
} }
@ -70,6 +71,7 @@ class PassphraseConfirmField extends PureComponent<IProps> {
onChange={this.props.onChange} onChange={this.props.onChange}
onValidate={this.onValidate} onValidate={this.onValidate}
autoFocus={this.props.autoFocus} autoFocus={this.props.autoFocus}
tooltipAlignment={this.props.tooltipAlignment}
/> />
); );
} }

View file

@ -15,6 +15,7 @@ import withValidation, { IFieldState, IValidationResult } from "../elements/Vali
import { _t, _td, TranslationKey } from "../../../languageHandler"; import { _t, _td, TranslationKey } from "../../../languageHandler";
import Field, { IInputProps } from "../elements/Field"; import Field, { IInputProps } from "../elements/Field";
import { MatrixClientPeg } from "../../../MatrixClientPeg"; import { MatrixClientPeg } from "../../../MatrixClientPeg";
import { Alignment } from "../elements/Tooltip";
interface IProps extends Omit<IInputProps, "onValidate" | "element"> { interface IProps extends Omit<IInputProps, "onValidate" | "element"> {
autoFocus?: boolean; autoFocus?: boolean;
@ -30,6 +31,7 @@ interface IProps extends Omit<IInputProps, "onValidate" | "element"> {
labelEnterPassword: TranslationKey; labelEnterPassword: TranslationKey;
labelStrongPassword: TranslationKey; labelStrongPassword: TranslationKey;
labelAllowedButUnsafe: TranslationKey; labelAllowedButUnsafe: TranslationKey;
tooltipAlignment?: Alignment;
onChange(ev: React.FormEvent<HTMLElement>): void; onChange(ev: React.FormEvent<HTMLElement>): void;
onValidate?(result: IValidationResult): void; onValidate?(result: IValidationResult): void;
@ -111,6 +113,7 @@ class PassphraseField extends PureComponent<IProps> {
value={this.props.value} value={this.props.value}
onChange={this.props.onChange} onChange={this.props.onChange}
onValidate={this.onValidate} onValidate={this.onValidate}
tooltipAlignment={this.props.tooltipAlignment}
/> />
); );
} }

View file

@ -26,6 +26,7 @@ import RegistrationEmailPromptDialog from "../dialogs/RegistrationEmailPromptDia
import CountryDropdown from "./CountryDropdown"; import CountryDropdown from "./CountryDropdown";
import PassphraseConfirmField from "./PassphraseConfirmField"; import PassphraseConfirmField from "./PassphraseConfirmField";
import { PosthogAnalytics } from "../../../PosthogAnalytics"; import { PosthogAnalytics } from "../../../PosthogAnalytics";
import { Alignment } from "../elements/Tooltip";
enum RegistrationField { enum RegistrationField {
Email = "field_email", Email = "field_email",
@ -58,6 +59,7 @@ interface IProps {
serverConfig: ValidatedServerConfig; serverConfig: ValidatedServerConfig;
canSubmit?: boolean; canSubmit?: boolean;
matrixClient: MatrixClient; matrixClient: MatrixClient;
mobileRegister?: boolean;
onRegisterClick(params: { onRegisterClick(params: {
username: string; username: string;
@ -439,6 +441,13 @@ export default class RegistrationForm extends React.PureComponent<IProps, IState
return true; return true;
} }
private tooltipAlignment(): Alignment | undefined {
if (this.props.mobileRegister) {
return Alignment.Bottom;
}
return undefined;
}
private renderEmail(): ReactNode { private renderEmail(): ReactNode {
if (!this.showEmail()) { if (!this.showEmail()) {
return null; return null;
@ -454,6 +463,7 @@ export default class RegistrationForm extends React.PureComponent<IProps, IState
validationRules={this.validateEmailRules.bind(this)} validationRules={this.validateEmailRules.bind(this)}
onChange={this.onEmailChange} onChange={this.onEmailChange}
onValidate={this.onEmailValidate} onValidate={this.onEmailValidate}
tooltipAlignment={this.tooltipAlignment()}
/> />
); );
} }
@ -468,6 +478,7 @@ export default class RegistrationForm extends React.PureComponent<IProps, IState
onChange={this.onPasswordChange} onChange={this.onPasswordChange}
onValidate={this.onPasswordValidate} onValidate={this.onPasswordValidate}
userInputs={[this.state.username]} userInputs={[this.state.username]}
tooltipAlignment={this.tooltipAlignment()}
/> />
); );
} }
@ -482,6 +493,7 @@ export default class RegistrationForm extends React.PureComponent<IProps, IState
password={this.state.password} password={this.state.password}
onChange={this.onPasswordConfirmChange} onChange={this.onPasswordConfirmChange}
onValidate={this.onPasswordConfirmValidate} onValidate={this.onPasswordConfirmValidate}
tooltipAlignment={this.tooltipAlignment()}
/> />
); );
} }
@ -526,6 +538,9 @@ export default class RegistrationForm extends React.PureComponent<IProps, IState
value={this.state.username} value={this.state.username}
onChange={this.onUsernameChange} onChange={this.onUsernameChange}
onValidate={this.onUsernameValidate} 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 ( return (
<div> <div>
<form onSubmit={this.onSubmit}> <form onSubmit={this.onSubmit}>
<div className="mx_AuthBody_fieldRow">{this.renderUsername()}</div> <div className="mx_AuthBody_fieldRow">{this.renderUsername()}</div>
<div className="mx_AuthBody_fieldRow"> {passwordFields}
{this.renderPassword()}
{this.renderPasswordConfirm()}
</div>
<div className="mx_AuthBody_fieldRow"> <div className="mx_AuthBody_fieldRow">
{this.renderEmail()} {this.renderEmail()}
{this.renderPhoneNumber()} {this.renderPhoneNumber()}

View file

@ -17,7 +17,7 @@ import classNames from "classnames";
import { debounce } from "lodash"; import { debounce } from "lodash";
import { IFieldState, IValidationResult } from "./Validation"; import { IFieldState, IValidationResult } from "./Validation";
import Tooltip from "./Tooltip"; import Tooltip, { Alignment } from "./Tooltip";
import { Key } from "../../../Keyboard"; import { Key } from "../../../Keyboard";
// Invoke validation from user input (when typing, etc.) at most once every N ms. // Invoke validation from user input (when typing, etc.) at most once every N ms.
@ -60,6 +60,8 @@ interface IProps {
tooltipContent?: React.ReactNode; tooltipContent?: React.ReactNode;
// If specified the tooltip will be shown regardless of feedback // If specified the tooltip will be shown regardless of feedback
forceTooltipVisible?: boolean; 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 // If specified alongside tooltipContent, the class name to apply to the
// tooltip itself. // tooltip itself.
tooltipClassName?: string; tooltipClassName?: string;
@ -261,6 +263,7 @@ export default class Field extends React.PureComponent<PropShapes, IState> {
validateOnFocus, validateOnFocus,
usePlaceholderAsHint, usePlaceholderAsHint,
forceTooltipVisible, forceTooltipVisible,
tooltipAlignment,
...inputProps ...inputProps
} = this.props; } = this.props;
@ -286,7 +289,7 @@ export default class Field extends React.PureComponent<PropShapes, IState> {
tooltipClassName={classNames("mx_Field_tooltip", "mx_Tooltip_noMargin", tooltipClassName)} tooltipClassName={classNames("mx_Field_tooltip", "mx_Tooltip_noMargin", tooltipClassName)}
visible={visible} visible={visible}
label={tooltipContent || this.state.feedback} label={tooltipContent || this.state.feedback}
alignment={Tooltip.Alignment.Right} alignment={tooltipAlignment || Alignment.Right}
role={role} role={role}
/> />
); );

View file

@ -55,6 +55,7 @@ import { MatrixClientPeg as peg } from "../../../src/MatrixClientPeg";
import DMRoomMap from "../../../src/utils/DMRoomMap"; import DMRoomMap from "../../../src/utils/DMRoomMap";
import { ReleaseAnnouncementStore } from "../../../src/stores/ReleaseAnnouncementStore"; import { ReleaseAnnouncementStore } from "../../../src/stores/ReleaseAnnouncementStore";
import { DRAFT_LAST_CLEANUP_KEY } from "../../../src/DraftCleaner"; import { DRAFT_LAST_CLEANUP_KEY } from "../../../src/DraftCleaner";
import { UIFeature } from "../../../src/settings/UIFeature";
jest.mock("matrix-js-sdk/src/oidc/authorize", () => ({ jest.mock("matrix-js-sdk/src/oidc/authorize", () => ({
completeAuthorizationCodeGrant: jest.fn(), 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();
});
});
}); });

View file

@ -8,7 +8,7 @@ Please see LICENSE files in the repository root for full details.
*/ */
import React from "react"; 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 { createClient, MatrixClient, MatrixError, OidcClientConfig } from "matrix-js-sdk/src/matrix";
import { mocked, MockedObject } from "jest-mock"; import { mocked, MockedObject } from "jest-mock";
import fetchMock from "fetch-mock-jest"; import fetchMock from "fetch-mock-jest";
@ -87,12 +87,23 @@ describe("Registration", function () {
const defaultHsUrl = "https://matrix.org"; const defaultHsUrl = "https://matrix.org";
const defaultIsUrl = "https://vector.im"; const defaultIsUrl = "https://vector.im";
function getRawComponent(hsUrl = defaultHsUrl, isUrl = defaultIsUrl, authConfig?: OidcClientConfig) { function getRawComponent(
return <Registration {...defaultProps} serverConfig={mkServerConfig(hsUrl, isUrl, authConfig)} />; 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) { function getComponent(hsUrl?: string, isUrl?: string, authConfig?: OidcClientConfig, mobileRegister?: boolean) {
return render(getRawComponent(hsUrl, isUrl, authConfig)); return render(getRawComponent(hsUrl, isUrl, authConfig, mobileRegister));
} }
it("should show server picker", async function () { 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);
});
});
}); });
}); });