Skip to content

Commit

Permalink
Mobile registration optimizations and tests (#62)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
langleyd authored Sep 20, 2024
1 parent 4be5338 commit 1f55710
Show file tree
Hide file tree
Showing 9 changed files with 142 additions and 19 deletions.
14 changes: 8 additions & 6 deletions src/components/structures/MatrixChat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion src/components/structures/auth/Registration.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
);
Expand Down Expand Up @@ -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>
Expand Down
3 changes: 3 additions & 0 deletions src/components/views/auth/EmailField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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>;
Expand Down Expand Up @@ -77,6 +79,7 @@ class EmailField extends PureComponent<IProps> {
autoFocus={this.props.autoFocus}
onChange={this.props.onChange}
onValidate={this.onValidate}
tooltipAlignment={this.props.tooltipAlignment}
/>
);
}
Expand Down
4 changes: 3 additions & 1 deletion src/components/views/auth/PassphraseConfirmField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down Expand Up @@ -70,6 +71,7 @@ class PassphraseConfirmField extends PureComponent<IProps> {
onChange={this.props.onChange}
onValidate={this.onValidate}
autoFocus={this.props.autoFocus}
tooltipAlignment={this.props.tooltipAlignment}
/>
);
}
Expand Down
3 changes: 3 additions & 0 deletions src/components/views/auth/PassphraseField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -111,6 +113,7 @@ class PassphraseField extends PureComponent<IProps> {
value={this.props.value}
onChange={this.props.onChange}
onValidate={this.onValidate}
tooltipAlignment={this.props.tooltipAlignment}
/>
);
}
Expand Down
37 changes: 33 additions & 4 deletions src/components/views/auth/RegistrationForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -58,6 +59,7 @@ interface IProps {
serverConfig: ValidatedServerConfig;
canSubmit?: boolean;
matrixClient: MatrixClient;
mobileRegister?: boolean;

onRegisterClick(params: {
username: string;
Expand Down Expand Up @@ -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;
Expand All @@ -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()}
/>
);
}
Expand All @@ -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()}
/>
);
}
Expand All @@ -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()}
/>
);
}
Expand Down Expand Up @@ -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"
/>
);
}
Expand Down Expand Up @@ -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()}
Expand Down
7 changes: 5 additions & 2 deletions src/components/views/elements/Field.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -261,6 +263,7 @@ export default class Field extends React.PureComponent<PropShapes, IState> {
validateOnFocus,
usePlaceholderAsHint,
forceTooltipVisible,
tooltipAlignment,
...inputProps
} = this.props;

Expand All @@ -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}
/>
);
Expand Down
39 changes: 39 additions & 0 deletions test/components/structures/MatrixChat-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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();
});
});
});
47 changes: 42 additions & 5 deletions test/components/structures/auth/Registration-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 () {
Expand Down Expand Up @@ -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);
});
});
});
});

0 comments on commit 1f55710

Please sign in to comment.