code review

src/components/common/BackButton.tsx

@@ -9,27 +9,20 @@ export const BackButton = () => {
 
   return (
     <button
-      className="bg-white dark:bg-dark-blue rounded-md shadow-md py-2 px-8"
+      className="bg-white dark:bg-dark-blue rounded-md shadow-md py-2 px-8 w-fit flex items-center"
       onClick={goBack}
     >
       <svg
         xmlns="http://www.w3.org/2000/svg"
         viewBox="0 0 448 512"
         height="24"
         width="16"
-        className="float-left mr-4"
+        className="mr-4"
       >
-        {theme === 'dark' ? (
-          <path
-            fill="#ffffff"
-            d="M9.4 233.4c-12.5 12.5-12.5 32.8 0 45.3l160 160c12.5 12.5 32.8 12.5 45.3 0s12.5-32.8 0-45.3L109.2 288 416 288c17.7 0 32-14.3 32-32s-14.3-32-32-32l-306.7 0L214.6 118.6c12.5-12.5 12.5-32.8 0-45.3s-32.8-12.5-45.3 0l-160 160z"
-          />
-        ) : (
-          <path
-            fill="#000000"
-            d="M9.4 233.4c-12.5 12.5-12.5 32.8 0 45.3l160 160c12.5 12.5 32.8 12.5 45.3 0s12.5-32.8 0-45.3L109.2 288 416 288c17.7 0 32-14.3 32-32s-14.3-32-32-32l-306.7 0L214.6 118.6c12.5-12.5 12.5-32.8 0-45.3s-32.8-12.5-45.3 0l-160 160z"
-          />
-        )}
+        <path
+          fill={theme === 'dark' ? '#ffffff' : '#000000'}
+          d="M9.4 233.4c-12.5 12.5-12.5 32.8 0 45.3l160 160c12.5 12.5 32.8 12.5 45.3 0s12.5-32.8 0-45.3L109.2 288 416 288c17.7 0 32-14.3 32-32s-14.3-32-32-32l-306.7 0L214.6 118.6c12.5-12.5 12.5-32.8 0-45.3s-32.8-12.5-45.3 0l-160 160z"
+        />
       </svg>
       <p className="inline-block text-sm md:text-base">Back</p>
     </button>

src/components/common/DisplayValue.tsx

@@ -0,0 +1,13 @@
+type DisplayValueProps = {
+  label: string;
+  value: string | number | string[] | null;
+};
+
+export const DisplayValue = ({ label, value }: DisplayValueProps) => {
+  const displayValue = Array.isArray(value) ? value.join(', ') : value;
+  return (
+    <p>
+      <b>{label}:</b> {displayValue}
+    </p>
+  );
+};

src/components/countries/CountriesRegionSelect.tsx

@@ -1,8 +1,9 @@
+import { ChangeEvent } from 'react';
+
 type Props = {
-  regions: string[];
-  selectedRegion: string;
-  // TODO: Function naming?
-  onChange: (region: string) => void;
+  regions: string[] | null;
+  selectedRegion: string | null;
+  onChange: (e: ChangeEvent<HTMLSelectElement>) => void;
 };
 
 export const CountriesRegionSelect = ({
@@ -12,16 +13,14 @@ export const CountriesRegionSelect = ({
 }: Props) => {
   return (
     <select
-      className="bg-white dark:bg-dark-blue md:self-end w-1/2 md:w-1/6 h-16 pl-6 text-xl rounded-md shadow-md hover:cursor-pointer"
-      onChange={(event) => onChange(event.target.value)}
-      defaultValue={selectedRegion}
+      className="bg-white dark:bg-dark-blue md:self-end w-fit h-16 px-6 text-xl rounded-md shadow-md hover:cursor-pointer"
+      onChange={(e) => onChange(e)}
+      defaultValue={selectedRegion || ''} // if no selectedRegion is null we default to "All regions"
     >
-      {/* TODO: Shoudl this be disabled and hidden? */}
-      <option className="pl-6" value="" disabled hidden>
-        Filter by region
-      </option>
-      {regions.map((region) => (
-        <option className="pl-12" key={region} value={region}>
+      {/* Might have diviated from the design here, but I think it's a better user experience to have the first option be "All regions" so we can go back to the original view. */}
+      <option value="">All regions</option>
+      {regions?.map((region) => (
+        <option key={region} value={region}>
           {region}
         </option>
       ))}

src/components/countries/CountriesSearch.tsx

@@ -1,11 +1,10 @@
 import { useDarkMode } from '../../hooks/useDarkMode';
 
 type Props = {
-  value: string;
-  onChange: (event: string) => void;
+  onSearchChange: (e: React.ChangeEvent<HTMLInputElement>) => void;
 };
 
-const CountriesSearch = ({ value, onChange }: Props) => {
+export const CountriesSearch = ({ onSearchChange }: Props) => {
   const [theme] = useDarkMode();
   const fillColor = theme === 'light' ? '#808080' : '#FFFFFF';
 
@@ -27,13 +26,10 @@ const CountriesSearch = ({ value, onChange }: Props) => {
       {/* TODO: Should this be its own shared component? */}
       <input
         className="bg-white text-xl dark:bg-dark-blue dark:text-white dark:placeholder:text-white w-full indent-2 outline-none"
-        value={value}
         type="text"
         placeholder="Search for a country..."
-        onChange={(event) => onChange(event.target.value)}
+        onChange={onSearchChange}
       />
     </div>
   );
 };
-
-export { CountriesSearch };

src/components/countries/CountryCard.tsx

@@ -1,38 +1,34 @@
 import { Link } from 'react-router-dom';
 import { Country } from '../../models/Country';
 import { Card } from '../common/Card';
+import { DisplayValue } from '../common/DisplayValue';
+
 type Props = {
   country: Country;
 };
 
-// TODO: Check the ordering of tags
-// TODO: Should the a encompass all tags instead of div/card?
-const CountryCard = ({ country }: Props) => {
+export const CountryCard = ({ country }: Props) => {
   return (
-    <Card className="bg-white dark:bg-dark-blue rounded-md shadow-md overflow-hidden md:flex-[0_0_20%] md:hover:flex-[0_0_22%]">
-      <Link to={`/country/${country.name.common}`}>
-        {/* TODO: How to fit image */}
+    <Link to={`/country/${country.name.common.toLowerCase()}`}>
+      <Card className="bg-white dark:bg-dark-blue rounded-md shadow-md h-full">
+        {/* TODO: How to fit image? answer: use object-cover (css: object-fit: cover) */}
         <img
-          className="object-contain w-full h-1/2 shadow-sm"
+          className="object-cover w-full h-1/2 shadow-sm"
           src={country.flags.svg}
+          alt={`Flag of ${country.name.common}`}
         />
         <section className="text-left p-6">
           <p className="text-xl font-bold mb-3">{country.name.common}</p>
           <div>
-            <p>
-              <b>Population:</b> {country.population.toLocaleString('EN-US')}
-            </p>
-            <p>
-              <b>Region:</b> {country.region}
-            </p>
-            <p>
-              <b>Capital:</b> {country.capital}
-            </p>
+            <DisplayValue
+              label="Population"
+              value={country.population.toLocaleString(navigator.language)}
+            />
+            <DisplayValue label="Region" value={country.region} />
+            <DisplayValue label="Capital" value={country.capital} />
           </div>
         </section>
-      </Link>
-    </Card>
+      </Card>
+    </Link>
   );
 };
-
-export { CountryCard };

src/components/ui/layout/Header.tsx

@@ -1,14 +1,10 @@
 import { DarkModeSwitch } from '../../DarkModeSwitch';
 
-const Header = () => (
-  <nav className="bg-white dark:bg-dark-blue h-20 shadow-sm flex justify-center">
-    <div className="h-full flex items-center justify-between w-11/12">
-      <p className="text-left text-md md:text-xl font-bold">
-        Where in the world?
-      </p>
-      <DarkModeSwitch />
-    </div>
+export const Header = () => (
+  <nav className="bg-white dark:bg-dark-blue h-20 shadow-sm flex justify-between items-center w-full px-10">
+    <p className="text-left text-md md:text-xl font-bold">
+      Where in the world?
+    </p>
+    <DarkModeSwitch />
   </nav>
 );
-
-export { Header };

src/components/ui/layout/Layout.tsx

@@ -1,7 +1,7 @@
 import { Outlet } from 'react-router-dom';
 import { Header } from './Header';
 
-const Layout = () => {
+export const Layout = () => {
   return (
     <>
       <Header />
@@ -11,5 +11,3 @@ const Layout = () => {
     </>
   );
 };
-
-export { Layout };

src/hooks/useCountries.ts

@@ -0,0 +1,84 @@
+import { useState, useEffect } from 'react';
+import { Country } from '../models/Country';
+import { getAllCountries, getAllRegions } from '../services/CountriesService';
+
+type UseCountries = {
+  countries: Country[] | null;
+  filteredCountries: Country[] | null;
+  regions: string[] | null;
+  loading: boolean;
+  error: Error | null;
+};
+
+export const useCountries = (
+  searchValue: string,
+  selectedRegion: string | null,
+): UseCountries => {
+  const [regions, setRegions] = useState<string[] | null>(null);
+  const [countries, setCountries] = useState<Country[] | null>(null);
+  const [filteredCountries, setFilteredCountries] = useState<Country[] | null>(
+    null,
+  );
+  const [loading, setLoading] = useState<boolean>(true);
+  const [error, setError] = useState<Error | null>(null);
+
+  useEffect(() => {
+    const fetchData = async () => {
+      try {
+        setLoading(true);
+        const [countriesData, regionsData] = await Promise.all([
+          getAllCountries(),
+          getAllRegions(),
+        ]);
+        setCountries(countriesData);
+        setRegions(regionsData);
+      } catch (err) {
+        setError(
+          err instanceof Error ? err : new Error('Failed to fetch data'),
+        );
+      } finally {
+        setLoading(false);
+      }
+    };
+
+    fetchData();
+  }, []);
+
+  useEffect(() => {
+    if (searchValue.trim() === '' && !selectedRegion) {
+      setFilteredCountries(null);
+      return;
+    }
+
+    if (!countries) return;
+
+    let filtered = [...countries];
+
+    // Filter by region if selected
+    if (selectedRegion) {
+      filtered = filtered.filter(
+        (country) => country.region === selectedRegion,
+      );
+    }
+
+    // Filter by search term if present
+    if (searchValue.trim() !== '') {
+      const searchTerm = searchValue.toLowerCase();
+      filtered = filtered.filter(
+        (country) =>
+          country.name.common.toLowerCase().includes(searchTerm) ||
+          country.name.official.toLowerCase().includes(searchTerm),
+      );
+    }
+
+    setFilteredCountries(filtered);
+  }, [searchValue, selectedRegion, countries]);
+
+  return {
+    countries,
+    filteredCountries,
+    regions,
+    loading,
+    error,
+  };
+};

src/loaders/CountryDetailsLoader.ts

@@ -15,10 +15,11 @@ export const countryDetailsLoader = async ({
 }: LoaderFunctionArgs): Promise<CountryDetailsData> => {
   if (!params.country) {
     throw new Error('Expected params.country');
+    // maybe route to 404 page or redirect to home page with a toast saying "Country not found" or something for better UX
   }
   const { country } = params;
   const countryData = await getCountry(country);
-  let borderCountries = [] as Country[];
+  let borderCountries: Country[] = [];
   if (countryData.borders) {
     borderCountries = await getCountriesByCountryCodes(countryData.borders);
   }

src/services/CountriesService.ts

@@ -24,7 +24,9 @@ const getFilteredResponse = async (fields: string[]) => {
 
 const getAllRegions = async () => {
   const responseData = await getFilteredResponse(['region']);
-  return responseData.map((entry) => entry.region);
+  const regions = responseData.map((entry) => entry.region);
+  const uniqueRegions = Array.from(new Set(regions)).sort();
+  return uniqueRegions;
 };
 
 const getCountriesByRegion = async (region: string) => {
@@ -47,11 +49,6 @@ const getCountriesByCountryCodes = async (codes: string[]) => {
   return response.data;
 };
 
-const getCountriesByRegionAndSearch = async (region: string) => {
-  const response = await axios.get<Country[]>(`${BASE_URL}/region/${region}`);
-  return response.data;
-};
-
 export {
   getAllCountries,
   getCountriesBySearch,