From 10810d4baaddf4d2c75ebfd9598b7e298e9e08a4 Mon Sep 17 00:00:00 2001 From: Dennis Kigen Date: Wed, 22 Feb 2023 15:57:23 +0300 Subject: [PATCH] (refactor) Refactor login components --- .../choose-location.component.tsx | 22 ++---- .../choose-location.resource.ts | 38 +++++---- .../location-picker.component.tsx | 78 +++++++++---------- .../src/location-picker/location-picker.scss | 5 +- .../src/login/login.component.tsx | 31 +++++--- .../src/login/login.resource.tsx | 18 ++++- .../apps/esm-login-app/src/login/login.scss | 16 ++++ .../esm-login-app/src/login/login.test.tsx | 4 +- .../apps/esm-login-app/translations/en.json | 1 + .../esm-styleguide/src/_overrides.scss | 12 ++- 10 files changed, 138 insertions(+), 87 deletions(-) diff --git a/packages/apps/esm-login-app/src/choose-location/choose-location.component.tsx b/packages/apps/esm-login-app/src/choose-location/choose-location.component.tsx index 8d5c49e1c..ce1f030ad 100644 --- a/packages/apps/esm-login-app/src/choose-location/choose-location.component.tsx +++ b/packages/apps/esm-login-app/src/choose-location/choose-location.component.tsx @@ -26,7 +26,7 @@ export const ChooseLocation: React.FC = ({ const referrer = state?.referrer; const config = useConfig(); const { user } = useSession(); - const { locationData, isLoading } = useLoginLocations( + const { locations, isLoading } = useLoginLocations( config.chooseLocation.useLoginLocationTag ); @@ -55,35 +55,29 @@ export const ChooseLocation: React.FC = ({ [referrer, config.links.loginSuccess, returnToUrl] ); - // Handle cases where the location picker is disabled, there is only one - // location, or there are no locations. + // Handle cases where the location picker is disabled, there is only one location, or there are no locations. useEffect(() => { if (!isLoading) { - if (!config.chooseLocation.enabled || locationData?.length === 1) { - changeLocation(locationData[0]?.resource.id); + if (!config.chooseLocation.enabled || locations?.length === 1) { + changeLocation(locations[0]?.resource.id); } - if (!isLoading && !locationData?.length) { + if (!isLoading && !locations?.length) { changeLocation(); } } }, [ - locationData, + locations, user, changeLocation, config.chooseLocation.enabled, isLoading, ]); - if (!isLoading && !user) { - navigate({ to: "${openmrsSpaBase}/login" }); - return null; - } - if (!isLoading || !isLoginEnabled) { return ( diff --git a/packages/apps/esm-login-app/src/choose-location/choose-location.resource.ts b/packages/apps/esm-login-app/src/choose-location/choose-location.resource.ts index b3a63d56c..94116872c 100644 --- a/packages/apps/esm-login-app/src/choose-location/choose-location.resource.ts +++ b/packages/apps/esm-login-app/src/choose-location/choose-location.resource.ts @@ -1,15 +1,16 @@ import { useMemo } from "react"; +import { useTranslation } from "react-i18next"; import { openmrsFetch, fhirBaseUrl, FetchResponse, - reportError, + showNotification, } from "@openmrs/esm-framework"; import useSwrInfinite from "swr/infinite"; import { LocationEntry, LocationResponse } from "../types"; interface LoginLocationData { - locationData: Array; + locations: Array; isLoading: boolean; totalResults: number; hasMore: boolean; @@ -19,14 +20,13 @@ interface LoginLocationData { ) => Promise[]>; } -const fhirLocationUrl = `${fhirBaseUrl}/Location?_summary=data`; - export function useLoginLocations( useLoginLocationTag: boolean, count: number = 0, searchQuery: string = "" ): LoginLocationData { - const getUrl = (page, prevPageData: FetchResponse) => { + const { t } = useTranslation(); + function constructUrl(page, prevPageData: FetchResponse) { if ( prevPageData && !prevPageData?.data?.link?.some((link) => link.relation === "next") @@ -34,7 +34,7 @@ export function useLoginLocations( return null; } - let url = fhirLocationUrl; + let url = `${fhirBaseUrl}/Location?_summary=data`; if (count) { url += `&_count=${count}`; @@ -53,24 +53,28 @@ export function useLoginLocations( } return url; - }; + } - const { data, isValidating, setSize, error } = useSwrInfinite< + const { data, isLoading, isValidating, setSize, error } = useSwrInfinite< FetchResponse, Error - >(getUrl, openmrsFetch); + >(constructUrl, openmrsFetch); if (error) { - console.error(error); - reportError(error); + showNotification({ + title: t("errorLoadingLoginLocations", "Error loading login locations"), + kind: "error", + critical: true, + description: error?.message, + }); } - const memoizedLocationData = useMemo(() => { + const memoizedLocations = useMemo(() => { return { - locationData: data - ? [].concat(...data?.map((resp) => resp?.data?.entry ?? [])) + locations: data?.length + ? data?.flatMap((entries) => entries?.data?.entry ?? []) : null, - isLoading: !data && !error, + isLoading, totalResults: data?.[0]?.data?.total ?? null, hasMore: data?.length ? data?.[data.length - 1]?.data?.link.some( @@ -80,7 +84,7 @@ export function useLoginLocations( loadingNewData: isValidating, setPage: setSize, }; - }, [data, error, isValidating, setSize, searchQuery]); + }, [isLoading, data, isValidating, setSize]); - return memoizedLocationData; + return memoizedLocations; } diff --git a/packages/apps/esm-login-app/src/location-picker/location-picker.component.tsx b/packages/apps/esm-login-app/src/location-picker/location-picker.component.tsx index 4a1842737..3023712d3 100644 --- a/packages/apps/esm-login-app/src/location-picker/location-picker.component.tsx +++ b/packages/apps/esm-login-app/src/location-picker/location-picker.component.tsx @@ -31,12 +31,14 @@ const LocationPicker: React.FC = ({ currentLocationUuid, isLoginEnabled, }) => { + const { t } = useTranslation(); const config = useConfig(); + const searchTimeout = 300; + const inputRef = useRef(); const { chooseLocation } = config; - const { t } = useTranslation(); - const [pageSize, setPageSize] = useState(chooseLocation.numberToShow); - const userDefaultLoginLocation: string = "userDefaultLoginLocationKey"; - const getDefaultUserLoginLocation = (): string => { + const userDefaultLoginLocation = "userDefaultLoginLocationKey"; + + const getDefaultUserLoginLocation = () => { const userLocation = window.localStorage.getItem( `${userDefaultLoginLocation}${currentUser}` ); @@ -45,13 +47,28 @@ const LocationPicker: React.FC = ({ ); return isValidLocation ? userLocation : ""; }; - const [activeLocation, setActiveLocation] = useState( - getDefaultUserLoginLocation() ?? "" - ); - const [searchTerm, setSearchTerm] = useState(""); + + const [activeLocation, setActiveLocation] = useState(() => { + if (currentLocationUuid && hideWelcomeMessage) { + return currentLocationUuid; + } + + return getDefaultUserLoginLocation() ?? ""; + }); + + const [isSubmitting, setIsSubmitting] = useState(false); + + const [searchTerm, setSearchTerm] = useState(() => { + if (isSubmitting && inputRef.current) { + let searchInput: HTMLInputElement = inputRef.current; + searchInput.value = ""; + setSearchTerm(null); + } + return ""; + }); const { - locationData, + locations, isLoading, hasMore, totalResults, @@ -63,16 +80,12 @@ const LocationPicker: React.FC = ({ searchTerm ); - const [isSubmitting, setIsSubmitting] = useState(false); - const inputRef = useRef(); - const searchTimeout = 300; - - useEffect(() => { - if (isSubmitting) { - onChangeLocation(activeLocation); - setIsSubmitting(false); + const [pageSize, setPageSize] = useState(() => { + if (!isLoading && totalResults && chooseLocation.numberToShow) { + return Math.min(chooseLocation.numberToShow, totalResults); } - }, [isSubmitting, activeLocation, onChangeLocation]); + return chooseLocation.numberToShow; + }); useEffect(() => { if (activeLocation) { @@ -84,24 +97,11 @@ const LocationPicker: React.FC = ({ }, [activeLocation, currentUser]); useEffect(() => { - if (currentLocationUuid && hideWelcomeMessage) { - setActiveLocation(currentLocationUuid); - } - }, [currentLocationUuid, hideWelcomeMessage]); - - useEffect(() => { - if (isSubmitting && inputRef.current) { - let searchInput: HTMLInputElement = inputRef.current; - searchInput.value = ""; - setSearchTerm(null); - } - }, [isSubmitting]); - - useEffect(() => { - if (!isLoading && totalResults && chooseLocation.numberToShow) { - setPageSize(Math.min(chooseLocation.numberToShow, totalResults)); + if (isSubmitting) { + onChangeLocation(activeLocation); + setIsSubmitting(false); } - }, [isLoading, totalResults, chooseLocation.numberToShow]); + }, [isSubmitting, activeLocation, onChangeLocation]); const search = debounce((location: string) => { setActiveLocation(""); @@ -113,7 +113,7 @@ const LocationPicker: React.FC = ({ setIsSubmitting(true); }; - // Infinte scrolling + // Infinite scroll const observer = useRef(null); const loadingIconRef = useCallback( (node) => { @@ -162,7 +162,7 @@ const LocationPicker: React.FC = ({ {!isLoading ? ( <>
- {locationData?.length > 0 && ( + {locations?.length > 0 && ( = ({ setActiveLocation(ev.toString()); }} > - {locationData.map((entry) => ( + {locations.map((entry) => ( = ({ ))} )} - {locationData?.length === 0 && ( + {locations?.length === 0 && (

{t("noResultsToDisplay", "No results to display")} diff --git a/packages/apps/esm-login-app/src/location-picker/location-picker.scss b/packages/apps/esm-login-app/src/location-picker/location-picker.scss index 2a0a640fa..43230dfe4 100644 --- a/packages/apps/esm-login-app/src/location-picker/location-picker.scss +++ b/packages/apps/esm-login-app/src/location-picker/location-picker.scss @@ -12,7 +12,7 @@ } .paddedContainer { - padding: spacing.$spacing-06; + padding: spacing.$spacing-06 spacing.$spacing-06 spacing.$spacing-05; } .locationCard { @@ -31,7 +31,7 @@ .welcomeMessage { @extend .bodyLong01; - margin-top: spacing.$spacing-01; + margin-top: spacing.$spacing-03; color: $color-gray-70; } @@ -52,7 +52,6 @@ .locationResultsContainer { display: flex; overflow-y: auto; - margin-top: spacing.$spacing-05; padding: 0rem spacing.$spacing-01 0rem; } diff --git a/packages/apps/esm-login-app/src/login/login.component.tsx b/packages/apps/esm-login-app/src/login/login.component.tsx index f16f01554..207516f64 100644 --- a/packages/apps/esm-login-app/src/login/login.component.tsx +++ b/packages/apps/esm-login-app/src/login/login.component.tsx @@ -2,6 +2,7 @@ import React, { useState, useRef, useEffect, useCallback } from "react"; import { useLocation, useNavigate } from "react-router-dom"; import { Button, + InlineLoading, InlineNotification, PasswordInput, TextInput, @@ -10,6 +11,7 @@ import { import { ArrowLeft, ArrowRight } from "@carbon/react/icons"; import { useTranslation } from "react-i18next"; import { + ConfigurableLink, useConfig, interpolateUrl, useSession, @@ -44,6 +46,7 @@ const Login: React.FC = ({ isLoginEnabled }) => { const [username, setUsername] = useState(""); const [password, setPassword] = useState(""); const [errorMessage, setErrorMessage] = useState(""); + const [isLoggingIn, setIsLoggingIn] = useState(false); const passwordInputRef = useRef(null); const usernameInputRef = useRef(null); const formRef = useRef(null); @@ -89,7 +92,7 @@ const Login: React.FC = ({ isLoginEnabled }) => { } else { field.focus(); } - }, [navigate]); + }, [location.state, navigate]); const changeUsername = useCallback( (evt: React.ChangeEvent) => setUsername(evt.target.value), @@ -117,22 +120,25 @@ const Login: React.FC = ({ isLoginEnabled }) => { } try { + setIsLoggingIn(true); const loginRes = await performLogin(username, password); const authData = loginRes.data; const valid = authData && authData.authenticated; - if (!valid) { - throw new Error("invalidCredentials"); - } else { + if (valid) { navigate("/login/location", { state: location.state }); + } else { + throw new Error("invalidCredentials"); } } catch (error) { + setIsLoggingIn(false); setErrorMessage(error.message); resetUserNameAndPassword(); } return false; }, + [ showPassword, continueLogin, @@ -162,8 +168,8 @@ const Login: React.FC = ({ isLoginEnabled }) => {

{errorMessage && ( = ({ isLoginEnabled }) => { className={styles.continueButton} renderIcon={(props) => } iconDescription="Log in" - disabled={!isLoginEnabled} + disabled={!isLoginEnabled || isLoggingIn} > - {t("login", "Log in")} + {isLoggingIn ? ( + + ) : ( + {t("login", "Log in")} + )}
)} @@ -271,9 +284,9 @@ const Login: React.FC = ({ isLoginEnabled }) => {

{t("needHelp", "Need help?")} - +

diff --git a/packages/apps/esm-login-app/src/login/login.resource.tsx b/packages/apps/esm-login-app/src/login/login.resource.tsx index 0ee34dbc4..e42dad14c 100644 --- a/packages/apps/esm-login-app/src/login/login.resource.tsx +++ b/packages/apps/esm-login-app/src/login/login.resource.tsx @@ -1,11 +1,23 @@ -import { openmrsFetch, refetchCurrentUser } from "@openmrs/esm-framework"; +import { + FetchResponse, + openmrsFetch, + refetchCurrentUser, + Session, +} from "@openmrs/esm-framework"; -export function performLogin(username, password) { +export async function performLogin( + username: string, + password: string +): Promise<{ data: Session }> { + const abortController = new AbortController(); const token = window.btoa(`${username}:${password}`); - return openmrsFetch(`/ws/rest/v1/session`, { + const url = `/ws/rest/v1/session`; + + return openmrsFetch(url, { headers: { Authorization: `Basic ${token}`, }, + signal: abortController.signal, }).then((res) => { refetchCurrentUser(); return res; diff --git a/packages/apps/esm-login-app/src/login/login.scss b/packages/apps/esm-login-app/src/login/login.scss index 09174fe4b..ccfc7411f 100644 --- a/packages/apps/esm-login-app/src/login/login.scss +++ b/packages/apps/esm-login-app/src/login/login.scss @@ -41,6 +41,12 @@ display: flex; flex-direction: row; align-items: center; + margin: 1rem 0; +} + +.link { + text-decoration-line: none; + margin-left: 0.5rem; } .powered-by-logo { @@ -55,6 +61,7 @@ width: 23rem; padding: 2.5rem; position: relative; + min-height: 21.5rem; } @media only screen and (min-width: 481px) { @@ -120,3 +127,12 @@ .footer { margin-top: 6.75rem; } + +.loader { + min-height: fit-content; +} + +.errorMessage { + margin-bottom: 1rem; + width: 23rem; +} diff --git a/packages/apps/esm-login-app/src/login/login.test.tsx b/packages/apps/esm-login-app/src/login/login.test.tsx index d19998d60..6f295267f 100644 --- a/packages/apps/esm-login-app/src/login/login.test.tsx +++ b/packages/apps/esm-login-app/src/login/login.test.tsx @@ -100,9 +100,11 @@ describe(``, () => { "yoshi" ); await user.click(screen.getByRole("button", { name: /Continue/i })); + + const loginButton = screen.getByRole("button", { name: /log in/i }); await screen.findByLabelText(/password/i); await user.type(screen.getByLabelText(/password/i), "no-tax-fraud"); - await user.click(screen.getByRole("button", { name: /log in/i })); + await user.click(loginButton); await waitFor(() => expect(performLogin).toHaveBeenCalledWith("yoshi", "no-tax-fraud") ); diff --git a/packages/apps/esm-login-app/translations/en.json b/packages/apps/esm-login-app/translations/en.json index 7600280ca..5cf217881 100644 --- a/packages/apps/esm-login-app/translations/en.json +++ b/packages/apps/esm-login-app/translations/en.json @@ -7,6 +7,7 @@ "error": "Error", "invalidCredentials": "Invalid username or password", "loading": "Loading", + "loggingIn": "Logging in", "login": "Log in", "Logout": "Logout", "needHelp": "Need help?", diff --git a/packages/framework/esm-styleguide/src/_overrides.scss b/packages/framework/esm-styleguide/src/_overrides.scss index aed969b1a..fda773564 100644 --- a/packages/framework/esm-styleguide/src/_overrides.scss +++ b/packages/framework/esm-styleguide/src/_overrides.scss @@ -179,7 +179,7 @@ .cds--btn--tertiary:hover, .cds--btn--tertiary:active, .cds--btn--tertiary:focus { - @include brand-01(background-color); + @include brand-03(background-color); } .cds--btn--primary:hover { @@ -267,3 +267,13 @@ .cds--search-input:focus { outline: 2px solid colors.$orange-40; } + +.cds--search-input:focus ~ .cds--search-close:hover { + outline: none; +} + +/* Radio buttons */ +.cds--radio-button-group--vertical + .cds--radio-button-wrapper:not(:last-of-type) { + margin-bottom: 0; +}