(refactor) Improve footer component and a11y (#1240)

This PR is a follow-up to #1192 that makes the following changes: - Extracts footer styles to a dedicated stylesheet - Replaces the `Button` Carbon component with a `Link` component for the "Learn more" button - Adds error handing for the logo image loading - Improves a11y with aria labels - Adds a type annotation for the `Logo`` interface
openmrs · Dec 16, 2024 · 7269bbf · 7269bbf
1 parent 78eae7c
commit 7269bbf
Show file tree

Hide file tree

Showing 6 changed files with 105 additions and 83 deletions.
diff --git a/packages/apps/esm-login-app/src/footer.component.tsx b/packages/apps/esm-login-app/src/footer.component.tsx
@@ -1,44 +1,54 @@
-import React from 'react';
-import { useConfig, ArrowRightIcon } from '@openmrs/esm-framework';
-import { Tile, Button } from '@carbon/react';
+import React, { useCallback } from 'react';
+import { Link, Tile } from '@carbon/react';
 import { useTranslation } from 'react-i18next';
+import { useConfig, ArrowRightIcon } from '@openmrs/esm-framework';
 import { type ConfigSchema } from './config-schema';
-import styles from './login/login.scss';
+import styles from './footer.scss';
+
+interface Logo {
+  src: string;
+  alt?: string;
+}
 
 const Footer: React.FC = () => {
-  const {t} = useTranslation();
+  const { t } = useTranslation();
   const config = useConfig<ConfigSchema>();
-  const logos = config.footer.additionalLogos || [];
+  const logos: Logo[] = config.footer.additionalLogos || [];
+
+  const handleImageLoadError = useCallback((error: React.SyntheticEvent<HTMLImageElement, Event>) => {
+    console.error('Failed to load image', error);
+  }, []);
 
   return (
     <div className={styles.footer}>
       <Tile className={styles.poweredByTile}>
         <div className={styles.poweredByContainer}>
           <span className={styles.poweredByText}>{t('builtWith', 'Built with')}</span>
-            <svg role="img" className={styles.poweredByLogo}>
-              <use href="#omrs-logo-full-color"></use>
-            </svg>
+          <svg aria-label={t('openmrsLogo', 'OpenMRS Logo')} className={styles.poweredByLogo} role="img">
+            <use href="#omrs-logo-full-color"></use>
+          </svg>
           <span className={styles.poweredByText}>
             {t('poweredBySubtext', 'An open-source medical record system and global community')}
           </span>
-          <Button
-            className={styles.learnMore}
-            iconDescription={t('learnMore', 'Learn More')}
-            kind="ghost"
-            onClick={() => window.open('https://openmrs.org', '_blank')}
-            renderIcon={(props) => <ArrowRightIcon {...props} size={20} className={styles.arrowRightIcon}/>}
+          <Link
+            className={styles.learnMoreButton}
+            href="https://openmrs.org"
+            rel="noopener noreferrer"
+            renderIcon={() => <ArrowRightIcon size={16} aria-label="Arrow right icon" />}
+            target="_blank"
           >
-            <span>{t('learnMore', 'Learn More')}</span>
-          </Button>
+            {t('learnMore', 'Learn more')}
+          </Link>
         </div>
       </Tile>
 
       <div className={styles.logosContainer}>
-        {logos.map((logo, index) => (
+        {logos.map((logo) => (
           <img
-            key={index}
             alt={logo.alt ? t(logo.alt) : t('footerlogo', 'Footer Logo')}
             className={styles.poweredByLogo}
+            key={logo.src}
+            onError={handleImageLoadError}
             src={logo.src}
           />
         ))}
@@ -47,4 +57,4 @@ const Footer: React.FC = () => {
   );
 };
 
-export default Footer;
+export default Footer;
diff --git a/packages/apps/esm-login-app/src/footer.scss b/packages/apps/esm-login-app/src/footer.scss
@@ -0,0 +1,58 @@
+@use '@carbon/colors';
+@use '@carbon/layout';
+
+.footer {
+  display: flex;
+  justify-content: space-between;
+  align-items: center;
+  padding: layout.$spacing-05;
+  position: absolute;
+  bottom: 0;
+  flex-wrap: wrap;
+  gap: layout.$spacing-05;
+  width: 100%;
+}
+
+.poweredByTile {
+  display: flex;
+  text-align: left;
+  max-width: fit-content;
+  min-height: fit-content;
+  font-size: smaller;
+  background-color: colors.$white;
+  padding: layout.$spacing-03 layout.$spacing-05;
+  border: 1px solid colors.$gray-20;
+  border-radius: layout.$spacing-04;
+  flex-wrap: wrap;
+}
+
+.poweredByContainer {
+  display: flex;
+  height: layout.$spacing-06;
+  align-items: center;
+  gap: layout.$spacing-03;
+}
+
+.poweredByLogo {
+  height: layout.$spacing-07;
+  width: auto;
+  max-width: layout.$spacing-12;
+  border-collapse: collapse;
+  padding: 0;
+  object-fit: contain;
+  display: block;
+  flex-shrink: 0;
+}
+
+.poweredByLogo + .poweredByText {
+  margin-left: layout.$spacing-02;
+}
+
+.learnMoreButton {
+  display: flex;
+  align-items: center;
+
+  svg {
+    fill: colors.$blue-60;
+  }
+}
diff --git a/packages/apps/esm-login-app/src/login/login.component.tsx b/packages/apps/esm-login-app/src/login/login.component.tsx
@@ -51,9 +51,9 @@ const Login: React.FC = () => {
   useEffect(() => {
     if (showPasswordOnSeparateScreen) {
       if (showPasswordField) {
-        passwordInputRef.current?.focus(); 
+        passwordInputRef.current?.focus();
       } else {
-        usernameInputRef.current?.focus(); 
+        usernameInputRef.current?.focus();
       }
     }
   }, [showPasswordField, showPasswordOnSeparateScreen]);
@@ -135,7 +135,7 @@ const Login: React.FC = () => {
     [username, password, navigate, showPasswordOnSeparateScreen],
   );
 
-  if (!loginProvider || loginProvider.type === 'basic'){
+  if (!loginProvider || loginProvider.type === 'basic') {
     return (
       <div className={styles.container}>
         <Tile className={styles.loginCard}>
@@ -182,7 +182,7 @@ const Login: React.FC = () => {
                       type="submit"
                       className={styles.continueButton}
                       renderIcon={(props) => <ArrowRightIcon size={24} {...props} />}
-                      iconDescription="Log in"
+                      iconDescription={t('loginButtonIconDescription', 'Log in button')}
                       disabled={!isLoginEnabled || isLoggingIn}
                     >
                       {isLoggingIn ? (
@@ -238,7 +238,7 @@ const Login: React.FC = () => {
       </div>
     );
   }
-    return null;
+  return null;
 };
 
 export default Login;
diff --git a/packages/apps/esm-login-app/src/login/login.scss b/packages/apps/esm-login-app/src/login/login.scss
@@ -50,31 +50,6 @@
   margin-left: 0.5rem;
 }
 
-.footer {
-  display: flex;
-  justify-content: space-between; 
-  align-items: center;
-  padding: 1rem;
-  position: absolute; 
-  bottom: 0;
-  flex-wrap: wrap;
-  gap: 1rem; 
-  width: 100%;
-}
-
-.poweredByTile {
-  display: flex;
-  text-align: left;
-  max-width: fit-content;
-  min-height: fit-content;
-  font-size: smaller;
-  background-color: #ffffff;
-  padding: 0.5rem 1rem;
-  border: 1px solid #e0e0e0;
-  border-radius: 1rem;
-  flex-wrap: wrap;
-}
-
 .logosContainer {
   display: flex;
   max-height: 2rem;
@@ -85,28 +60,6 @@
   opacity: 80%;
 }
 
-.poweredByContainer{
-  display: flex;
-  height: 1.5rem;
-  align-items: center;
-  gap: 0.5rem;
-}
-
-.poweredByLogo {
-  height: 2rem; 
-  width: auto;
-  max-width: 6rem;
-  border-collapse: collapse;
-  padding: 0;
-  object-fit: contain;
-  display: block; 
-  flex-shrink: 0;
-}
-
-.poweredByLogo + .poweredByText {
-  margin-left: 0.25rem;
-}
-
 .loginCard {
   border-radius: 0;
   border: 1px solid $ui-03;
@@ -127,7 +80,7 @@
 @media only screen and (max-width: 1024px) {
   .footer {
     flex-direction: row;
-    justify-content: center; 
+    justify-content: center;
     padding: 1rem;
   }
 
@@ -143,7 +96,7 @@
     gap: 0.75rem;
   }
 
-  .container{
+  .container {
     height: 100vh;
   }
 }
@@ -160,9 +113,9 @@
 
   .footer {
     flex-direction: column;
-    align-items: center; 
+    align-items: center;
     justify-content: center;
-    gap: 0.5rem; 
+    gap: 0.5rem;
     padding: 1rem;
   }
 
@@ -171,8 +124,8 @@
     align-items: center;
     justify-content: center;
     padding: 1.2rem 1rem;
-    font-size: 0.7rem; 
-    height: auto; 
+    font-size: 0.7rem;
+    height: auto;
     max-width: 100%;
     border-radius: 0.75rem;
   }

diff --git a/packages/apps/esm-login-app/src/login/login.test.tsx b/packages/apps/esm-login-app/src/login/login.test.tsx
@@ -35,12 +35,12 @@ describe('Login', () => {
     renderWithRouter(
       Login,
       {},
-      { 
+      {
         route: '/login',
       },
     );
 
-    screen.getByRole('img', { name: /OpenMRS logo/i });
+    expect(screen.getAllByRole('img', { name: /OpenMRS logo/i })).toHaveLength(2);
     expect(screen.queryByAltText(/^logo$/i)).not.toBeInTheDocument();
     screen.getByRole('textbox', { name: /Username/i });
     screen.getByRole('button', { name: /Continue/i });

diff --git a/packages/apps/esm-login-app/translations/en.json b/packages/apps/esm-login-app/translations/en.json
@@ -1,6 +1,5 @@
 {
-  "back": "Back",
-  "backToUserNameIconLabel": "Back to username",
+  "builtWith": "Built with",
   "cancel": "Cancel",
   "change": "Change",
   "changePassword": "Change password",
@@ -10,6 +9,7 @@
   "errorChangingPassword": "Error changing password",
   "footerlogo": "Footer Logo",
   "invalidCredentials": "Invalid username or password",
+  "learnMore": "Learn more",
   "locationPreferenceRemoved": "Login location preference removed",
   "locationPreferenceRemovedMessage": "You will need to select a location on each login",
   "locationSaved": "Location saved",
@@ -18,6 +18,7 @@
   "locationUpdateMessage": "Your preferred login location has been updated",
   "loggingIn": "Logging in",
   "login": "Log in",
+  "loginButtonIconDescription": "Log in button",
   "Logout": "Logout",
   "newPassword": "New password",
   "newPasswordRequired": "New password is required",
@@ -28,7 +29,7 @@
   "passwordChangedSuccessfully": "Password changed successfully",
   "passwordConfirmationRequired": "Password confirmation is required",
   "passwordsDoNotMatch": "Passwords do not match",
-  "poweredBy": "Powered by",
+  "poweredBySubtext": "An open-source medical record system and global community",
   "rememberLocationForFutureLogins": "Remember my location for future logins",
   "selectYourLocation": "Select your location from the list below. Use the search bar to find your location.",
   "showPassword": "Show password",