Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(feat) O3-2583: Configure Password Field Placement on Login Screen #820

Merged
merged 5 commits into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/apps/esm-login-app/__mocks__/config.mock.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
export const mockConfig = {
import { ConfigSchema } from "../src/config-schema";

export const mockConfig: ConfigSchema = {
provider: {
type: "basic",
loginUrl: "",
Expand All @@ -8,6 +10,7 @@ export const mockConfig = {
enabled: true,
numberToShow: 3,
useLoginLocationTag: true,
locationsPerRequest: 50,
},
logo: {
src: null,
Expand All @@ -16,4 +19,5 @@ export const mockConfig = {
links: {
loginSuccess: "${openmrsSpaBase}/home",
},
passwordOnSeparateScreen: true,
};
9 changes: 8 additions & 1 deletion packages/apps/esm-login-app/src/config-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ export const configSchema = {
_description: "Alt text, shown on hover",
},
},
passwordOnSeparateScreen: {
denniskigen marked this conversation as resolved.
Show resolved Hide resolved
_type: Type.Boolean,
_default: true,
_description:
"Whether to show the password field on a separate screen. If false, the password field will be shown on the same screen.",
},
};

export interface ConfigSchema {
Expand All @@ -81,7 +87,7 @@ export interface ConfigSchema {
};
chooseLocation: {
enabled: boolean;
numberToShow: boolean;
numberToShow: number;
denniskigen marked this conversation as resolved.
Show resolved Hide resolved
locationsPerRequest: number;
useLoginLocationTag: boolean;
};
Expand All @@ -92,4 +98,5 @@ export interface ConfigSchema {
src: string;
alt: string;
};
passwordOnSeparateScreen: boolean;
}
74 changes: 42 additions & 32 deletions packages/apps/esm-login-app/src/login/login.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export interface LoginProps extends LoginReferrer {}

const Login: React.FC<LoginProps> = () => {
const config = useConfig();
const { passwordOnSeparateScreen } = config;
const isLoginEnabled = useConnectivity();
const { t } = useTranslation();
const { user } = useSession();
Expand All @@ -52,7 +53,10 @@ const Login: React.FC<LoginProps> = () => {
const passwordInputRef = useRef<HTMLInputElement>(null);
const usernameInputRef = useRef<HTMLInputElement>(null);
const formRef = useRef<HTMLFormElement>(null);
const showPassword = location.pathname === "/login/confirm";

const showUsername = location.pathname === "/login";
const showPassword =
!passwordOnSeparateScreen || location.pathname === "/login/confirm";

const handleLogin = useCallback(
(session: Session) => {
Expand Down Expand Up @@ -195,7 +199,7 @@ const Login: React.FC<LoginProps> = () => {
/>
)}
<Tile className={styles["login-card"]}>
{showPassword ? (
{passwordOnSeparateScreen && showPassword ? (
ibacher marked this conversation as resolved.
Show resolved Hide resolved
<div className={styles["back-button-div"]}>
<Button
className={styles["back-button"]}
Expand All @@ -216,7 +220,7 @@ const Login: React.FC<LoginProps> = () => {
) : null}
<div className={styles["center"]}>{logo}</div>
<form onSubmit={handleSubmit} ref={formRef}>
{!showPassword && (
{showUsername && (
<div className={styles["input-group"]}>
<TextInput
id="username"
Expand All @@ -229,38 +233,33 @@ const Login: React.FC<LoginProps> = () => {
autoFocus
required
/>
<input
id="password"
style={hidden}
type="password"
name="password"
value={password}
onChange={changePassword}
/>
jayasanka-sack marked this conversation as resolved.
Show resolved Hide resolved
<Button
className={styles.continueButton}
renderIcon={(props) => <ArrowRight size={24} {...props} />}
type="submit"
iconDescription="Continue to login"
onClick={continueLogin}
disabled={!isLoginEnabled}
>
{t("continue", "Continue")}
</Button>
{/* For password managers */}
{passwordOnSeparateScreen && (
<input
id="password"
style={hidden}
type="password"
name="password"
value={password}
onChange={changePassword}
/>
)}
{passwordOnSeparateScreen && (
<Button
className={styles.continueButton}
renderIcon={(props) => <ArrowRight size={24} {...props} />}
type="submit"
iconDescription="Continue to login"
onClick={continueLogin}
disabled={!isLoginEnabled}
>
{t("continue", "Continue")}
</Button>
)}
</div>
)}
{showPassword && (
<div className={styles["input-group"]}>
<input
id="username"
type="text"
name="username"
style={hidden}
value={username}
onChange={changeUsername}
required
/>

<PasswordInput
id="password"
invalidText={t(
Expand All @@ -275,7 +274,18 @@ const Login: React.FC<LoginProps> = () => {
required
showPasswordLabel="Show password"
/>

{/* For password managers */}
{passwordOnSeparateScreen && (
<input
id="username"
type="text"
name="username"
style={hidden}
value={username}
onChange={changeUsername}
required
/>
)}
<Button
type="submit"
className={styles.continueButton}
Expand Down
4 changes: 4 additions & 0 deletions packages/apps/esm-login-app/src/login/login.scss
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@
:global(.cds--text-input__field-outer-wrapper) {
width: 18rem;
}

&:not(:last-child) {
margin-bottom: 1rem;
}
}

.continueButton {
Expand Down
98 changes: 89 additions & 9 deletions packages/apps/esm-login-app/src/login/login.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,13 @@ mockedUseConfig.mockReturnValue(mockConfig);

describe("Login", () => {
it("renders the login form", () => {
renderWithRouter(Login);
renderWithRouter(
Login,
{},
{
route: "/login",
}
);

screen.getByRole("img", { name: /OpenMRS logo/i });
expect(screen.queryByAltText(/logo/i)).not.toBeInTheDocument();
Expand Down Expand Up @@ -79,7 +85,6 @@ describe("Login", () => {
{},
{
route: "/login",
routes: ["/login", "/login/confirm"],
}
);
const user = userEvent.setup();
Expand Down Expand Up @@ -109,12 +114,11 @@ describe("Login", () => {
{},
{
route: "/login",
routes: ["/login", "/login/confirm"],
}
);
const user = userEvent.setup();

expect(performLogin).not.toHaveBeenCalled();
ibacher marked this conversation as resolved.
Show resolved Hide resolved
mockedLogin.mockClear();
await user.type(
screen.getByRole("textbox", { name: /Username/i }),
"yoshi"
Expand All @@ -130,6 +134,7 @@ describe("Login", () => {
);
});

// TODO: Complete the test
it("sends the user to the location select page on login if there is more than one location", async () => {
let refreshUser = (user: any) => {};
mockedLogin.mockImplementation(() => {
Expand All @@ -148,11 +153,7 @@ describe("Login", () => {
Login,
{},
{
routeParams: {
path: "/login(/confirm)?",
exact: true,
},
routes: ["/login", "/login/confirm"],
ibacher marked this conversation as resolved.
Show resolved Hide resolved
route: "/login",
}
);

Expand All @@ -167,4 +168,83 @@ describe("Login", () => {
await user.type(screen.getByLabelText(/password/i), "no-tax-fraud");
await user.click(screen.getByRole("button", { name: /log in/i }));
});

it("should render the both the username and password fields when the passwordOnSeparateScreen config is false", async () => {
mockedUseConfig.mockReturnValue({
...mockConfig,
passwordOnSeparateScreen: false,
});

renderWithRouter(
Login,
{},
{
route: "/login",
}
);

const usernameInput = screen.queryByRole("textbox", { name: /username/i });
const continueButton = screen.queryByRole("button", { name: /Continue/i });
const passwordInput = screen.queryByLabelText(/password/i);
const loginButton = screen.queryByRole("button", { name: /log in/i });

expect(usernameInput).toBeInTheDocument();
expect(continueButton).not.toBeInTheDocument();
expect(passwordInput).toBeInTheDocument();
expect(loginButton).toBeInTheDocument();
});

it("should not render the password field when the passwordOnSeparateScreen config is true (default)", async () => {
mockedUseConfig.mockReturnValue({
...mockConfig,
});

renderWithRouter(
Login,
{},
{
route: "/login",
}
);

const usernameInput = screen.queryByRole("textbox", { name: /username/i });
const continueButton = screen.queryByRole("button", { name: /Continue/i });
const passwordInput = screen.queryByLabelText(/password/i);
const loginButton = screen.queryByRole("button", { name: /log in/i });

expect(usernameInput).toBeInTheDocument();
expect(continueButton).toBeInTheDocument();
expect(passwordInput).not.toBeInTheDocument();
expect(loginButton).not.toBeInTheDocument();
});

it("should be able to login when the passwordOnSeparateScreen config is false", async () => {
mockedLogin.mockReturnValue(Promise.resolve({ some: "data" }));
mockedUseConfig.mockReturnValue({
...mockConfig,
passwordOnSeparateScreen: false,
});
const user = userEvent.setup();
mockedLogin.mockClear();

renderWithRouter(
Login,
{},
{
route: "/login",
}
);

const usernameInput = screen.getByRole("textbox", { name: /username/i });
const passwordInput = screen.getByLabelText(/password/i);
const loginButton = screen.getByRole("button", { name: /log in/i });

await user.type(usernameInput, "yoshi");
await user.type(passwordInput, "no-tax-fraud");
await user.click(loginButton);

await waitFor(() =>
expect(performLogin).toHaveBeenCalledWith("yoshi", "no-tax-fraud")
);
});
});
Loading