Improve tests for Dropdown

client/src/components/util/GameCreationForm/Dropdown.jsx

@@ -23,6 +23,25 @@ export const Item = ({ value, display, onSelect, children }) => {
   );
 };
 
+export const findSelectedDisplay = (selected, children) => {
+  if (selected === undefined) {
+    return null;
+  }
+
+  let found = null;
+  React.Children.toArray(children).forEach(element => {
+    if (
+      React.isValidElement(element) &&
+      found === null &&
+      element.props.value === selected
+    ) {
+      const { value, display } = element.props;
+      found = display ?? value;
+    }
+  });
+  return found;
+}
+
 export const Dropdown = ({ selected, open, onSelect, onClick, children }) => {
   const transitionRef = useRef(null);
   const [displayed, setDisplayed] = useState(null);
@@ -33,25 +52,8 @@ export const Dropdown = ({ selected, open, onSelect, onClick, children }) => {
     },
     [onSelect]
   );
+  useEffect(() => setDisplayed(d => findSelectedDisplay(selected, children) ?? d), [children, selected]);
 
-  useEffect(() => {
-    if (selected === undefined) {
-      return;
-    }
-
-    let found = null;
-    React.Children.toArray(children).forEach(element => {
-      if (
-        React.isValidElement(element) &&
-        found === null &&
-        element.props.value === selected
-      ) {
-        const { value, display } = element.props;
-        found = display ?? value;
-      }
-    });
-    setDisplayed(found);
-  }, [children, selected]);
   return (
     <div className={styles.container}>
       <div
@@ -93,6 +95,22 @@ export const Dropdown = ({ selected, open, onSelect, onClick, children }) => {
   );
 };
 
+export const CountryItem = ({ code, country, onSelect }) => (
+  <div
+    className={styles.item}
+    role="menuitem"
+    tabIndex="0"
+    onClick={() => onSelect(code)}
+    onKeyDown={({ key }) => {
+      if (key === "Enter") {
+        onSelect(code);
+      }
+    }}
+  >
+    {flagLookup(code)} - {country}
+  </div>
+)
+
 export const CountryDropdown = ({
   countryLookup,
   selected,
@@ -154,35 +172,8 @@ export const CountryDropdown = ({
               }
             }}
           />
-          {found.map(({ item: { country, alpha2 } }) => (
-            <div
-              role="button"
-              tabIndex="0"
-              key={alpha2}
-              className={styles.item}
-              onClick={() => onSelectCallback(alpha2)}
-              onKeyDown={({ key }) => {
-                if (key === "Enter") {
-                  onSelectCallback(alpha2);
-                }
-              }}
-            >
-              {flagLookup(alpha2)} - {country}
-            </div>
-          ))}
-          <div
-            className={styles.item}
-            role="menuitem"
-            tabIndex="0"
-            onClick={() => onSelectCallback(null)}
-            onKeyDown={({ key }) => {
-              if (key === "Enter") {
-                onSelectCallback(null);
-              }
-            }}
-          >
-            {flagLookup(null)} - All Countries
-          </div>
+          {found.map(({ item: { country, alpha2 } }) => <CountryItem key={alpha2} code={alpha2} country={country}onSelect={onSelectCallback} />)}
+          <CountryItem code={null} country="All Countries" onSelect={onSelectCallback} />
         </div>
       </CSSTransition>
     </div>

client/src/tests/Dropdown.test.js

@@ -2,8 +2,10 @@ import React from "react";
 import { shallow } from "enzyme";
 import {
   CountryDropdown,
+  CountryItem,
   Dropdown,
   DropdownGroup,
+  findSelectedDisplay,
   Item,
 } from "../components/util/GameCreationForm/Dropdown";
 
@@ -122,6 +124,19 @@ describe("Dropdown", () => {
     expect(onClick).toHaveBeenCalled();
   });
 
+  it("cannot be selected with other keydowns", () => {
+    const onSelect = jest.fn();
+    const onClick = jest.fn();
+    const rendered = shallow(
+      <Dropdown {...{ onSelect, onClick }}>
+        <Item value="test1" />
+        <Item value="test2" />
+      </Dropdown>
+    );
+    rendered.find("div.button").first().simulate("keydown", { key: "Escape" });
+    expect(onClick).not.toHaveBeenCalled();
+  });
+
   it("responds to item selection", () => {
     const onSelect = jest.fn();
     const onClick = jest.fn();
@@ -135,8 +150,80 @@ describe("Dropdown", () => {
     expect(onSelect).toHaveBeenCalledWith("value");
     expect(rendered).toMatchSnapshot();
   });
+
+  describe("findSelectedDisplay", () => {
+    it("returns null when nothing is selected", () => {
+      expect(findSelectedDisplay()).toBeNull();
+    });
+
+    it("extracts the display of the selected element", () => {
+      expect(findSelectedDisplay("two", [
+        <Item display="1" value="one" />,
+        <Item display="2" value="two" />,
+        <Item display="3" value="three" />,
+      ])).toBe("2");
+    });
+
+    it("extracts the value of the selected element if there is no display", () => {
+      expect(findSelectedDisplay("two", [
+        <Item value="one" />,
+        <Item value="two" />,
+        <Item value="three" />,
+      ])).toBe("two");
+    });
+
+    it("returns null if there is no matching element", () => {
+      expect(findSelectedDisplay("four", [
+        <Item value="one" />,
+        <Item value="two" />,
+        <Item value="three" />,
+      ])).toBeNull();
+    });
+
+    it("returns the first display found if there is more than one matching element", () => {
+      expect(findSelectedDisplay("two", [
+        <Item display="1" value="one" />,
+        <Item display="2" value="two" />,
+        <Item display="3" value="three" />,
+        <Item display="4" value="two" />,
+      ])).toBe("2");
+    });
+  });
 });
 
+describe("CountryItem", () => {
+  it("renders", () => {
+    flagLookup.mockReturnValue("flag");
+    const rendered = shallow(<CountryItem code="test-code" country="test-country" />);
+    expect(rendered).toMatchSnapshot();
+    expect(flagLookup).toHaveBeenCalledWith("test-code");
+  });
+
+  it("responds to click", () => {
+    flagLookup.mockReturnValue("flag");
+    const onSelect = jest.fn();
+    const rendered = shallow(<CountryItem onSelect={onSelect} code="test-code" country="test-country" />);
+    rendered.simulate("click");
+    expect(onSelect).toHaveBeenCalledWith("test-code");
+  });
+
+  it("responds to keydown Enter", () => {
+    flagLookup.mockReturnValue("flag");
+    const onSelect = jest.fn();
+    const rendered = shallow(<CountryItem onSelect={onSelect} code="test-code" country="test-country" />);
+    rendered.simulate("keydown", { key:"Enter"});
+    expect(onSelect).toHaveBeenCalledWith("test-code");
+  });
+
+  it("responds to click", () => {
+    flagLookup.mockReturnValue("flag");
+    const onSelect = jest.fn();
+    const rendered = shallow(<CountryItem onSelect={onSelect} code="test-code" country="test-country" />);
+    rendered.simulate("keydown", { key:"Escape"});
+    expect(onSelect).not.toHaveBeenCalled();
+  });
+})
+
 describe("CountryDropdown", () => {
   it("renders closed", () => {
     flagLookup.mockReturnValue("flag");
@@ -227,28 +314,7 @@ describe("CountryDropdown", () => {
     expect(onClick).toHaveBeenCalled();
   });
 
-  it("can have a country selected", () => {
-    flagLookup.mockReturnValue("flag");
-    const countryLookup = jest.fn();
-    countryLookup.mockReturnValue([
-      { item: { country: "c1", alpha2: "a21" } },
-      { item: { country: "c2", alpha2: "a22" } },
-      { item: { country: "c3", alpha2: "a23" } },
-    ]);
-    const selected = "selected";
-    const onSelect = jest.fn();
-    const onClick = jest.fn();
-    const rendered = shallow(
-      <CountryDropdown
-        open
-        {...{ countryLookup, selected, onSelect, onClick }}
-      />
-    );
-    rendered.find("div.item").first().simulate("click");
-    expect(onSelect).toHaveBeenCalledWith("a21");
-  });
-
-  it("can have a country selected with Enter", () => {
+  it("cannot be selected with other keydowns", () => {
     flagLookup.mockReturnValue("flag");
     const countryLookup = jest.fn();
     countryLookup.mockReturnValue([
@@ -265,11 +331,11 @@ describe("CountryDropdown", () => {
         {...{ countryLookup, selected, onSelect, onClick }}
       />
     );
-    rendered.find("div.item").at(1).simulate("keydown", { key: "Enter" });
-    expect(onSelect).toHaveBeenCalledWith("a22");
+    rendered.find("div.button").first().simulate("keydown", { key: "Escape" });
+    expect(onClick).not.toHaveBeenCalled();
   });
 
-  it("can have world selected", () => {
+  it("can have a country selected", () => {
     flagLookup.mockReturnValue("flag");
     const countryLookup = jest.fn();
     countryLookup.mockReturnValue([
@@ -286,11 +352,11 @@ describe("CountryDropdown", () => {
         {...{ countryLookup, selected, onSelect, onClick }}
       />
     );
-    rendered.find("div.item").last().simulate("click");
-    expect(onSelect).toHaveBeenCalledWith(null);
+    rendered.find("CountryItem").first().prop("onSelect")();
+    expect(onSelect).toHaveBeenCalled();
   });
 
-  it("can have world selected with Enter", () => {
+  it("selects country based on searchbox", () => {
     flagLookup.mockReturnValue("flag");
     const countryLookup = jest.fn();
     countryLookup.mockReturnValue([
@@ -307,11 +373,14 @@ describe("CountryDropdown", () => {
         {...{ countryLookup, selected, onSelect, onClick }}
       />
     );
-    rendered.find("div.item").last().simulate("keydown", { key: "Enter" });
-    expect(onSelect).toHaveBeenCalledWith(null);
+    rendered
+      .find("input")
+      .first()
+      .simulate("change", { target: { value: "changed" } });
+    expect(countryLookup).toHaveBeenCalledWith("changed");
   });
 
-  it("selects country based on searchbox", () => {
+  it("can have first option selected with Enter", () => {
     flagLookup.mockReturnValue("flag");
     const countryLookup = jest.fn();
     countryLookup.mockReturnValue([
@@ -328,14 +397,11 @@ describe("CountryDropdown", () => {
         {...{ countryLookup, selected, onSelect, onClick }}
       />
     );
-    rendered
-      .find("input")
-      .first()
-      .simulate("change", { target: { value: "changed" } });
-    expect(countryLookup).toHaveBeenCalledWith("changed");
+    rendered.find("input").first().simulate("keydown", { key: "Enter" });
+    expect(onSelect).toHaveBeenCalledWith("a21");
   });
 
-  it("can have first option selected with Enter", () => {
+  it("can have previously selected option selected with Escape", () => {
     flagLookup.mockReturnValue("flag");
     const countryLookup = jest.fn();
     countryLookup.mockReturnValue([
@@ -352,11 +418,11 @@ describe("CountryDropdown", () => {
         {...{ countryLookup, selected, onSelect, onClick }}
       />
     );
-    rendered.find("input").first().simulate("keydown", { key: "Enter" });
-    expect(onSelect).toHaveBeenCalledWith("a21");
+    rendered.find("input").first().simulate("keydown", { key: "Escape" });
+    expect(onSelect).toHaveBeenCalledWith("selected");
   });
 
-  it("can have previously selected option selected with Escape", () => {
+  it("does not select from other keydowns", () => {
     flagLookup.mockReturnValue("flag");
     const countryLookup = jest.fn();
     countryLookup.mockReturnValue([
@@ -373,8 +439,8 @@ describe("CountryDropdown", () => {
         {...{ countryLookup, selected, onSelect, onClick }}
       />
     );
-    rendered.find("input").first().simulate("keydown", { key: "Escape" });
-    expect(onSelect).toHaveBeenCalledWith("selected");
+    rendered.find("input").first().simulate("keydown", { key: "Tab" });
+    expect(onSelect).not.toHaveBeenCalled();
   });
 });
 
@@ -404,4 +470,29 @@ describe("Item", () => {
     rendered.simulate("keydown", { key: "Enter" });
     expect(onSelect).toHaveBeenCalledWith(value, display);
   });
+
+  it("does not respond to other keydowns", () => {
+    const onSelect = jest.fn();
+    const display = "display";
+    const value = "value";
+    const rendered = shallow(<Item {...{ onSelect, display, value }} />);
+    rendered.simulate("keydown", { key: "Escape" });
+    expect(onSelect).not.toHaveBeenCalled();
+  });
+
+  it("responds to click with value when no display", () => {
+    const onSelect = jest.fn();
+    const value = "value";
+    const rendered = shallow(<Item {...{ onSelect, value }} />);
+    rendered.simulate("click");
+    expect(onSelect).toHaveBeenCalledWith(value, value);
+  });
+
+  it("responds to click with children when no display or value", () => {
+    const onSelect = jest.fn();
+    const child = <div>child</div>;
+    const rendered = shallow(<Item {...{ onSelect }}>{child}</Item>);
+    rendered.simulate("click");
+    expect(onSelect).toHaveBeenCalledWith(child, child);
+  });
 });

client/src/tests/__snapshots__/Dropdown.test.js.snap

@@ -44,16 +44,11 @@ exports[`CountryDropdown renders 1`] = `
         type="text"
         value=""
       />
-      <div
-        className="item"
-        onClick={[Function]}
-        onKeyDown={[Function]}
-        role="menuitem"
-        tabIndex="0"
-      >
-        flag
-         - All Countries
-      </div>
+      <CountryItem
+        code={null}
+        country="All Countries"
+        onSelect={[Function]}
+      />
     </div>
   </CSSTransition>
 </div>
@@ -102,16 +97,11 @@ exports[`CountryDropdown renders closed 1`] = `
         type="text"
         value=""
       />
-      <div
-        className="item"
-        onClick={[Function]}
-        onKeyDown={[Function]}
-        role="menuitem"
-        tabIndex="0"
-      >
-        flag
-         - All Countries
-      </div>
+      <CountryItem
+        code={null}
+        country="All Countries"
+        onSelect={[Function]}
+      />
     </div>
   </CSSTransition>
 </div>
@@ -161,57 +151,48 @@ exports[`CountryDropdown renders with countries 1`] = `
         type="text"
         value=""
       />
-      <div
-        className="item"
+      <CountryItem
+        code="a21"
+        country="c1"
         key="a21"
-        onClick={[Function]}
-        onKeyDown={[Function]}
-        role="button"
-        tabIndex="0"
-      >
-        flag
-         - 
-        c1
-      </div>
-      <div
-        className="item"
+        onSelect={[Function]}
+      />
+      <CountryItem
+        code="a22"
+        country="c2"
         key="a22"
-        onClick={[Function]}
-        onKeyDown={[Function]}
-        role="button"
-        tabIndex="0"
-      >
-        flag
-         - 
-        c2
-      </div>
-      <div
-        className="item"
+        onSelect={[Function]}
+      />
+      <CountryItem
+        code="a23"
+        country="c3"
         key="a23"
-        onClick={[Function]}
-        onKeyDown={[Function]}
-        role="button"
-        tabIndex="0"
-      >
-        flag
-         - 
-        c3
-      </div>
-      <div
-        className="item"
-        onClick={[Function]}
-        onKeyDown={[Function]}
-        role="menuitem"
-        tabIndex="0"
-      >
-        flag
-         - All Countries
-      </div>
+        onSelect={[Function]}
+      />
+      <CountryItem
+        code={null}
+        country="All Countries"
+        onSelect={[Function]}
+      />
     </div>
   </CSSTransition>
 </div>
 `;
 
+exports[`CountryItem renders 1`] = `
+<div
+  className="item"
+  onClick={[Function]}
+  onKeyDown={[Function]}
+  role="menuitem"
+  tabIndex="0"
+>
+  flag
+   - 
+  test-country
+</div>
+`;
+
 exports[`Dropdown renders closed 1`] = `
 <div
   className="container"