fix(settings): Remove divider in country selector

Because: * Divider is picked up as an option on Android * HR nested in select component is not currently supported on Android * Countries should be sorted alphabetically This commit: * Remove HR in select component * Sort country list alphabetically based on localized name * Select USA by default for now Closes #FXA-11210
mozilla · Mar 3, 2025 · e29eeda · e29eeda
1 parent 384431d
commit e29eeda
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 55 deletions.
diff --git a/packages/fxa-settings/src/components/InputPhoneNumber/index.test.tsx b/packages/fxa-settings/src/components/InputPhoneNumber/index.test.tsx
@@ -3,10 +3,9 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 import React from 'react';
-import { act, screen } from '@testing-library/react';
+import { screen } from '@testing-library/react';
 import { renderWithLocalizationProvider } from 'fxa-react/lib/test-utils/localizationProvider';
 import { extendedCountryOptions, Subject } from './mocks';
-import userEvent from '@testing-library/user-event';
 
 describe('InputPhoneNumber', () => {
   it('renders as expected with default countries', () => {
@@ -20,46 +19,39 @@ describe('InputPhoneNumber', () => {
     const options = screen.getAllByRole('option');
     expect(options.length).toBe(2);
 
-    // The first option should be United States, and it should be selected by default
-    expect(options[0]).toHaveTextContent('United States (+1)');
-    expect((options[0] as HTMLOptionElement).selected).toBe(true);
+    // expect list to be sorted alphabetically
+    expect(options[0]).toHaveTextContent('Canada (+1)');
+    expect(options[1]).toHaveTextContent('United States (+1)');
+    // US is currently the default
+    expect((options[1] as HTMLOptionElement).selected).toBe(true);
 
     expect(
       screen.getByRole('textbox', { name: /Enter phone number/i })
     ).toBeInTheDocument();
   });
 
-  it('renders a select option for every country, and the country with ID=1 is always the first option', async () => {
-    const user = userEvent.setup();
+  it('renders a select option for every country, and the country with ID=1 is selected by default', async () => {
     renderWithLocalizationProvider(
       <Subject countries={extendedCountryOptions} />
     );
     let options = screen.getAllByRole('option');
     expect(options.length).toBe(extendedCountryOptions.length);
 
     // There should be an option for each country listed
-    options.forEach((option, index) => {
-      const { name, code } = extendedCountryOptions[index];
-      const expectedText = `${name} (${code})`;
-      expect(option).toHaveTextContent(expectedText);
+    expect(extendedCountryOptions.length).toEqual(options.length);
+    extendedCountryOptions.forEach((countryOption) => {
+      expect(
+        options.some((option) =>
+          option.textContent?.includes(countryOption.name)
+        )
+      ).toBe(true);
     });
 
-    // The first option should correspond to the country with ID=1
+    // The default country (country with ID=1) should be selected by default
     const defaultCountry = extendedCountryOptions.find(
       (country) => country.id === 1
     );
-    expect(options[0]).toHaveTextContent(
-      `${defaultCountry?.name} (${defaultCountry?.code})`
-    );
     const selectElement = options[0].closest('select') as HTMLSelectElement;
-    expect(selectElement).not.toBeNull();
-
-    // We must use 'act' because we need to get all options after selection has finished
-    await act(async () => {
-      // Select "Murica" (id=100)
-      await user.selectOptions(selectElement, ['100']);
-    });
-    const updatedOptions = screen.getAllByRole('option');
-    expect(updatedOptions[0]).toHaveTextContent('United States (+1)');
+    expect(selectElement).toHaveValue(defaultCountry?.id.toString());
   });
 });
diff --git a/packages/fxa-settings/src/components/InputPhoneNumber/index.tsx b/packages/fxa-settings/src/components/InputPhoneNumber/index.tsx
@@ -61,13 +61,23 @@ const InputPhoneNumber = ({
   errorBannerId,
 }: InputPhoneNumberProps) => {
   const ftlMsgResolver = useFtlMsgResolver();
-  const localizedCountries = countries.map((country) => ({
-    ...country,
-    localizedName: ftlMsgResolver.getMsg(country.ftlId, country.name),
-  }));
+  const sortedLocalizedCountries = countries
+    .map((country) => ({
+      ...country,
+      localizedName:
+        ftlMsgResolver.getMsg(country.ftlId, country.name) || country.name,
+    }))
+    .sort((a, b) => {
+      return new Intl.Collator(navigator.language).compare(
+        a.localizedName,
+        b.localizedName
+      );
+    });
+  // currently USA is the default country, but we should aim to select the default country based on user location
+  // FXA-11212
   const defaultCountry =
-    localizedCountries.find((country) => country.id === 1) ||
-    localizedCountries[0];
+    sortedLocalizedCountries.find((country) => country.id === 1) ||
+    sortedLocalizedCountries[0];
   const [selectedCountry, setSelectedCountry] = useState(defaultCountry);
 
   const localizedLabel = ftlMsgResolver.getMsg(
@@ -76,7 +86,7 @@ const InputPhoneNumber = ({
   );
 
   const handleCountryChange = (event: React.ChangeEvent<HTMLSelectElement>) => {
-    const selectedCountry = localizedCountries.find(
+    const selectedCountry = sortedLocalizedCountries.find(
       (country) => country.id === parseInt(event.target.value, 10)
     );
     if (selectedCountry) {
@@ -121,30 +131,15 @@ const InputPhoneNumber = ({
         value={selectedCountry.id}
         className={`bg-transparent border border-grey-200 rounded-md py-2 ps-10 w-[60px] me-2 focus:border-blue-400 focus:outline-none focus:shadow-input-blue-focus ${selectedCountry.classNameFlag} bg-no-repeat bg-[length:1.5rem_1rem] bg-[40%_50%] text-transparent`}
       >
-        {/* Default country is always first */}
-        <option
-          className={`${defaultCountry.classNameFlag} bg-contain`}
-          value={defaultCountry.id}
-        >
-          {defaultCountry.localizedName} ({defaultCountry.code})
-        </option>
-
-        {/* Note, at the time of writing we're using react-dom 18.3 which complains about
-        <hr> inside <select> being invalid, but it is valid. This should be fixed in later
-         versions: https://github.com/facebook/react/issues/27572 */}
-        <hr className="my-1" />
-
-        {localizedCountries
-          .filter((country) => country.id !== defaultCountry.id)
-          .map((country) => (
-            <option
-              key={country.id}
-              value={country.id}
-              className={`${country.classNameFlag} bg-contain`}
-            >
-              {country.localizedName} ({country.code})
-            </option>
-          ))}
+        {sortedLocalizedCountries.map((country) => (
+          <option
+            key={country.id}
+            value={country.id}
+            className={`${country.classNameFlag} bg-contain`}
+          >
+            {country.localizedName} ({country.code})
+          </option>
+        ))}
       </select>
 
       {/* Because the country code may not be unique, the above `select`'s `value` must