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

refactor(web): UI code pruning and clean up round #3 #1540

Merged
merged 19 commits into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 2 additions & 2 deletions web/src/components/core/ListSearch.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) [2023] SUSE LLC
* Copyright (c) [2023-2024] SUSE LLC
*
* All Rights Reserved.
*
Expand Down Expand Up @@ -42,7 +42,7 @@ const search = (elements, term) => {
* @param {object} props
* @param {string} [props.placeholder]
* @param {object[]} [props.elements] - List of elements in which to search.
* @param {(elements: object[]) => void} - Callback to be called with the filtered list of elements.
* @param {(elements: object[]) => void} [props.onChange] - Callback to be called with the filtered list of elements.
*/
export default function ListSearch({
placeholder = _("Search"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ import { plainRender } from "~/test-utils";
import { LoginPage } from "~/components/core";
import { AuthErrors } from "~/context/auth";

let mockIsAuthenticated;
const mockLoginFn = jest.fn();
let consoleErrorSpy: jest.SpyInstance;
let mockIsAuthenticated: boolean;
let mockLoginError;
const mockLoginFn = jest.fn();

jest.mock("~/context/auth", () => ({
...jest.requireActual("~/context/auth"),
Expand All @@ -40,16 +41,16 @@ jest.mock("~/context/auth", () => ({
},
}));

describe.skip("LoginPage", () => {
describe("LoginPage", () => {
beforeAll(() => {
mockIsAuthenticated = false;
mockLoginError = null;
mockLoginFn.mockResolvedValue({ status: 200 });
jest.spyOn(console, "error").mockImplementation();
consoleErrorSpy = jest.spyOn(console, "error").mockImplementation();
});

afterAll(() => {
console.error.mockRestore();
consoleErrorSpy.mockRestore();
});

describe("when user is not authenticated", () => {
Expand Down Expand Up @@ -114,7 +115,7 @@ describe.skip("LoginPage", () => {

it("renders a button to know more about the project", async () => {
const { user } = plainRender(<LoginPage />);
const button = screen.getByRole("button", { name: "What is this?" });
const button = screen.getByRole("button", { name: "More about this" });

await user.click(button);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
* find current contact information at www.suse.com.
*/

// @ts-check

import React, { useState } from "react";
import { Navigate } from "react-router-dom";
import {
Expand All @@ -33,16 +31,13 @@ import {
FormGroup,
Grid,
GridItem,
Stack,
} from "@patternfly/react-core";
import { About, EmptyState, FormValidationError, Page, PasswordInput } from "~/components/core";
import { Center } from "~/components/layout";
import { AuthErrors, useAuth } from "~/context/auth";
import { _ } from "~/i18n";
import { sprintf } from "sprintf-js";

// @ts-check

/**
* Renders the UI that lets the user log into the system.
* @component
Expand All @@ -52,7 +47,7 @@ export default function LoginPage() {
const [error, setError] = useState(false);
const { isLoggedIn, login: loginFn, error: loginError } = useAuth();

const login = async (e) => {
const login = async (e: React.FormEvent) => {
e.preventDefault();
const result = await loginFn(password);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) [2022] SUSE LLC
* Copyright (c) [2022-2024] SUSE LLC
*
* All Rights Reserved.
*
Expand Down Expand Up @@ -38,8 +38,8 @@ describe("when the passwords do not match", () => {
it("uses the given password value for confirmation too", async () => {
plainRender(<PasswordAndConfirmationInput value="12345" />);

const passwordInput = screen.getByLabelText("Password");
const confirmationInput = screen.getByLabelText("Password confirmation");
const passwordInput = screen.getByLabelText("Password") as HTMLInputElement;
const confirmationInput = screen.getByLabelText("Password confirmation") as HTMLInputElement;

expect(passwordInput.value).toEqual("12345");
expect(passwordInput.value).toEqual(confirmationInput.value);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) [2022] SUSE LLC
* Copyright (c) [2022-2024] SUSE LLC
*
* All Rights Reserved.
*
Expand All @@ -19,34 +19,43 @@
* find current contact information at www.suse.com.
*/

// @ts-check

import React, { useEffect, useState } from "react";
import { FormGroup } from "@patternfly/react-core";
import { FormValidationError, PasswordInput } from "~/components/core";
import { _ } from "~/i18n";

// TODO: improve the component to allow working only in uncontrlled mode if
// needed.
// TODO: improve the showErrors thingy
// TODO:
// * add documentation,
// * allow working only in uncontrlled mode if needed, and
dgdavid marked this conversation as resolved.
Show resolved Hide resolved
// * improve the showErrors thingy

type PasswordAndConfirmationInputProps = {
inputRef?: React.RefObject<HTMLInputElement>;
value?: string;
showErrors?: boolean;
isDisabled?: boolean;
onChange?: (e: React.SyntheticEvent, v: string) => void;
onValidation?: (r: boolean) => void;
};

const PasswordAndConfirmationInput = ({
inputRef,
showErrors = true,
value,
onChange,
onValidation,
isDisabled = false,
}) => {
}: PasswordAndConfirmationInputProps) => {
const passwordInput = inputRef?.current;
const [password, setPassword] = useState(value || "");
const [confirmation, setConfirmation] = useState(value || "");
const [error, setError] = useState("");
const [password, setPassword] = useState<string>(value || "");
const [confirmation, setConfirmation] = useState<string>(value || "");
const [error, setError] = useState<string>("");

useEffect(() => {
if (isDisabled) setError("");
}, [isDisabled]);

const validate = (password, passwordConfirmation) => {
const validate = (password: string, passwordConfirmation: string) => {
let newError = "";
showErrors && setError(newError);
passwordInput?.setCustomValidity(newError);
Expand All @@ -63,13 +72,13 @@ const PasswordAndConfirmationInput = ({
}
};

const onValueChange = (event, value) => {
const onValueChange = (event: React.SyntheticEvent, value: string) => {
setPassword(value);
validate(value, confirmation);
if (typeof onChange === "function") onChange(event, value);
};

const onConfirmationChange = (_, confirmationValue) => {
const onConfirmationChange = (_: React.SyntheticEvent, confirmationValue: string) => {
setConfirmation(confirmationValue);
validate(password, confirmationValue);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { _ } from "~/i18n";
import { useConfigMutation, useL10n } from "~/queries/l10n";
import textStyles from "@patternfly/react-styles/css/utilities/Text/text";

// TODO: Add documentation and typechecking
// TODO: Add documentation
// TODO: Evaluate if worth it extracting the selector
export default function KeyboardSelection() {
const navigate = useNavigate();
Expand All @@ -40,7 +40,7 @@ export default function KeyboardSelection() {

const searchHelp = _("Filter by description or keymap code");

const onSubmit = async (e) => {
const onSubmit = async (e: React.SyntheticEvent) => {
e.preventDefault();
setConfig.mutate({ keymap: selected });
navigate(-1);
Expand Down Expand Up @@ -68,7 +68,7 @@ export default function KeyboardSelection() {
});

if (keymapsList.length === 0) {
keymapsList = <b>{_("None of the keymaps match the filter.")}</b>;
keymapsList = [<b>{_("None of the keymaps match the filter.")}</b>];
}

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { _ } from "~/i18n";
import { useConfigMutation, useL10n } from "~/queries/l10n";
import textStyles from "@patternfly/react-styles/css/utilities/Text/text";

// TODO: Add documentation and typechecking
// TODO: Add documentation
// TODO: Evaluate if worth it extracting the selector
export default function LocaleSelection() {
const navigate = useNavigate();
Expand All @@ -38,7 +38,7 @@ export default function LocaleSelection() {

const searchHelp = _("Filter by language, territory or locale code");

const onSubmit = async (e) => {
const onSubmit = async (e: React.SyntheticEvent) => {
e.preventDefault();
setConfig.mutate({ locales: [selected] });
navigate(-1);
Expand Down Expand Up @@ -69,7 +69,7 @@ export default function LocaleSelection() {
});

if (localesList.length === 0) {
localesList = <b>{_("None of the locales match the filter.")}</b>;
localesList = [<b>{_("None of the locales match the filter.")}</b>];
}

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,17 @@ import React, { useState } from "react";
import { Divider, Flex, Form, FormGroup, Radio, Text } from "@patternfly/react-core";
import { useNavigate } from "react-router-dom";
import { ListSearch, Page } from "~/components/core";
import { _ } from "~/i18n";
import { timezoneTime } from "~/utils";
import { useConfigMutation, useL10n } from "~/queries/l10n";
import { Timezone } from "~/types/l10n";
import textStyles from "@patternfly/react-styles/css/utilities/Text/text";
import { _ } from "~/i18n";

type TimezoneWithDetails = Timezone & { details: string };

let date;
let date: Date;

const timezoneWithDetails = (timezone) => {
const timezoneWithDetails = (timezone: Timezone): TimezoneWithDetails => {
const offset = timezone.utcOffset;

if (offset === undefined) return { ...timezone, details: timezone.id };
Expand All @@ -42,14 +45,14 @@ const timezoneWithDetails = (timezone) => {
return { ...timezone, details: `${timezone.id} ${utc}` };
};

const sortedTimezones = (timezones) => {
const sortedTimezones = (timezones: Timezone[]) => {
return timezones.sort((timezone1, timezone2) => {
const timezoneText = (t) => t.parts.join("").toLowerCase();
const timezoneText = (t: Timezone) => t.parts.join("").toLowerCase();
return timezoneText(timezone1) > timezoneText(timezone2) ? 1 : -1;
});
};

// TODO: Add documentation and typechecking
// TODO: Add documentation
// TODO: Evaluate if worth it extracting the selector
// TODO: Refactor timezones/extendedTimezones thingy
export default function TimezoneSelection() {
Expand All @@ -63,42 +66,44 @@ export default function TimezoneSelection() {

const searchHelp = _("Filter by territory, time zone code or UTC offset");

const onSubmit = async (e) => {
const onSubmit = async (e: React.SyntheticEvent) => {
e.preventDefault();
setConfig.mutate({ timezone: selected });
navigate(-1);
};

let timezonesList = filteredTimezones.map(({ id, country, details, parts }) => {
return (
<Radio
id={id}
key={id}
name="timezone"
onChange={() => setSelected(id)}
label={
<>
<span className={`${textStyles.fontSizeLg}`}>
<b>{parts.join("-")}</b>
</span>{" "}
<Text component="small">{country}</Text>
</>
}
description={
<Flex columnGap={{ default: "columnGapXs" }}>
<Text component="small">{timezoneTime(id, { date }) || ""}</Text>
<Divider orientation={{ default: "vertical" }} />
<div>{details}</div>
</Flex>
}
value={id}
isChecked={id === selected}
/>
);
});
let timezonesList = filteredTimezones.map(
({ id, country, details, parts }: TimezoneWithDetails) => {
return (
<Radio
id={id}
key={id}
name="timezone"
onChange={() => setSelected(id)}
label={
<>
<span className={`${textStyles.fontSizeLg}`}>
<b>{parts.join("-")}</b>
</span>{" "}
<Text component="small">{country}</Text>
</>
}
description={
<Flex columnGap={{ default: "columnGapXs" }}>
<Text component="small">{timezoneTime(id, { date }) || ""}</Text>
<Divider orientation={{ default: "vertical" }} />
<div>{details}</div>
</Flex>
}
value={id}
isChecked={id === selected}
/>
);
},
);

if (timezonesList.length === 0) {
timezonesList = <b>{_("None of the time zones match the filter.")}</b>;
timezonesList = [<b>{_("None of the time zones match the filter.")}</b>];
}

return (
Expand Down
Loading
Loading