Convert TreeViewHeader to a functional component

[jasonleenaylor]

• Update test suite to verify the functionality of the
  custom hook added into the TreeViewHeader.

---

This change is 

[codecov-io]

Codecov Report

Merging #836 (2585ff6) into master (41205b9) will increase coverage by 0.07%.
The diff coverage is 78.12%.

@@            Coverage Diff             @@
##           master     #836      +/-   ##
==========================================
+ Coverage   51.26%   51.33%   +0.07%     
==========================================
  Files         238      238              
  Lines        6611     6607       -4     
  Branches      421      419       -2     
==========================================
+ Hits         3389     3392       +3     
+ Misses       2909     2904       -5     
+ Partials      313      311       -2     

Flag	Coverage Δ
backend	55.84% <ø> (ø)
frontend	46.93% <78.12%> (+0.14%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/components/TreeView/TreeDepiction.tsx	67.30% <ø> (ø)
src/components/TreeView/TreeViewHeader.tsx	72.97% <78.12%> (+6.30%)	⬆️
.../MergeDupGoal/MergeDupStep/MergeDupStepReducer.tsx	51.94% <0.00%> (+1.29%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41205b9...2585ff6. Read the comment docs.

[imnasnainaec]

Reviewed 2 of 6 files at r1, 4 of 6 files at r2.
Reviewable status: 6 of 8 files reviewed, all discussions resolved

[imnasnainaec]

Reviewable status: 6 of 8 files reviewed, all discussions resolved

---

src/components/TreeView/tests/TreeViewHeader.test.tsx, line 32 at r2 (raw file):

  MOCK_ANIMATE.mockClear();
  MOCK_BOUNCE.mockClear();

How about jest.clearAllMocks();?

---

src/components/TreeView/tests/TreeViewHeader.test.tsx, line 41 at r2 (raw file):

    // Simulate the user typing 1.1
    const simulatedInput = { target: { value: "1.0" } } as React.ChangeEvent<

Comment and test mismatch.

---

src/components/TreeView/tests/TreeViewHeader.test.tsx, line 78 at r2 (raw file):

    const simulatedInput = { target: { value: TEST } } as React.ChangeEvent<
      HTMLTextAreaElement
    >;

Since this method is used multiple times, it could become a general-purpose function.
(Same for simulatedEnterKey.)

---

src/components/TreeView/tests/TreeViewHeader.test.tsx, line 80 at r2 (raw file):

    >;

    const MOCK_STOP_PROP = jest.fn();

MOCK_STOP_PROP is used so many times, it could go up with MOCK_ANIMATE and MOCK_BOUNCE.

---

src/components/TreeView/tests/TreeViewHeader.test.tsx, line 108 at r2 (raw file):

    const { result } = renderHook(() => useTreeViewNavigation(testProps));

    // Simulate the user typing 10

This comment shows up a number of times for non-"10" tests.

[imnasnainaec]

Reviewable status: 6 of 8 files reviewed, all discussions resolved (waiting on @jasonleenaylor)

---

src/components/TreeView/TreeViewHeader.tsx, line 142 at r2 (raw file):

Quoted 4 lines of code…

      } else if (event.key === "ArrowDown") {
        if (props.currentDomain.parentDomain)
          props.animate(props.currentDomain.parentDomain);
      }

(Missed in #508...) It should be "ArrowUp" that navigates to parentDomain.

---

src/components/TreeView/TreeViewHeader.tsx, line 192 at r2 (raw file):

while (parent.parentDomain !== undefined) parent = parent.parentDomain;

Our style guide says to avoid one-liner if statements. What about one-liner while?

[jasonleenaylor]

Reviewable status: 6 of 8 files reviewed, all discussions resolved (waiting on @jasonleenaylor)

---

src/components/TreeView/TreeViewHeader.tsx, line 142 at r2 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

      } else if (event.key === "ArrowDown") {
        if (props.currentDomain.parentDomain)
          props.animate(props.currentDomain.parentDomain);
      }

(Missed in #508...) It should be "ArrowUp" that navigates to parentDomain.

Yes, I thought I'd do that in a follow up PR.

---

src/components/TreeView/TreeViewHeader.tsx, line 192 at r2 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

while (parent.parentDomain !== undefined) parent = parent.parentDomain;

Our style guide says to avoid one-liner if statements. What about one-liner while?

I think those should be avoided as well.

---

src/components/TreeView/tests/TreeViewHeader.test.tsx, line 32 at r2 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

  MOCK_ANIMATE.mockClear();
  MOCK_BOUNCE.mockClear();

How about jest.clearAllMocks();?

Done.

---

src/components/TreeView/tests/TreeViewHeader.test.tsx, line 41 at r2 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

    // Simulate the user typing 1.1
    const simulatedInput = { target: { value: "1.0" } } as React.ChangeEvent<

Comment and test mismatch.

Done.

---

src/components/TreeView/tests/TreeViewHeader.test.tsx, line 78 at r2 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

    const simulatedInput = { target: { value: TEST } } as React.ChangeEvent<
      HTMLTextAreaElement
    >;

Since this method is used multiple times, it could become a general-purpose function.
(Same for simulatedEnterKey.)

Ah yes, I'd planned to do that before I got sidetracked trying to test the arrow keys at the component level. Done.

---

src/components/TreeView/tests/TreeViewHeader.test.tsx, line 80 at r2 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

MOCK_STOP_PROP is used so many times, it could go up with MOCK_ANIMATE and MOCK_BOUNCE.

Done.

[imnasnainaec]

Reviewed 1 of 2 files at r3.
Reviewable status: 7 of 8 files reviewed, all discussions resolved (waiting on @jasonleenaylor)

---

src/components/TreeView/tests/TreeViewHeader.test.tsx, line 292 at r3 (raw file):

Quoted 18 lines of code…

    keyDownHandler!.call(
      simulatedArrowKey as Event,
      simulatedArrowKey as Event
    );
    // verify that we would switch to the domain requested
    expect(MOCK_ANIMATE).toHaveBeenCalledWith(MockDomain.subdomains[0]);
  });
  it("typing up arrow key moves to parent domain", () => {
    render(<TreeViewHeader {...upOneWithBrothersProps} />);
    const keyDownHandler = eventListeners.get("keydown");
    expect(keyDownHandler).not.toBeUndefined();
    const simulatedArrowKey: Partial<KeyboardEvent> = {
      key: "ArrowUp",
    };
    keyDownHandler!.call(
      simulatedArrowKey as Event,
      simulatedArrowKey as Event
    );

Is the first argument in call being used in these cases? When I substitute {} or null in for the first simulatedArrowKey as Event, the tests all still pass.

[jasonleenaylor]

---

src/components/TreeView/tests/TreeViewHeader.test.tsx, line 292 at r3 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

    keyDownHandler!.call(
      simulatedArrowKey as Event,
      simulatedArrowKey as Event
    );
    // verify that we would switch to the domain requested
    expect(MOCK_ANIMATE).toHaveBeenCalledWith(MockDomain.subdomains[0]);
  });
  it("typing up arrow key moves to parent domain", () => {
    render(<TreeViewHeader {...upOneWithBrothersProps} />);
    const keyDownHandler = eventListeners.get("keydown");
    expect(keyDownHandler).not.toBeUndefined();
    const simulatedArrowKey: Partial<KeyboardEvent> = {
      key: "ArrowUp",
    };
    keyDownHandler!.call(
      simulatedArrowKey as Event,
      simulatedArrowKey as Event
    );

Is the first argument in call being used in these cases? When I substitute {} or null in for the first simulatedArrowKey as Event, the tests all still pass.

Good question, it seems that the first argument is the 'source' or the 'this' parameter, and the second is the event. I wasn't sure which was necessary and was just really excited that it worked. 😆
I'll do a bit more research to see what is really going on there.

[imnasnainaec]

Reviewable status: 7 of 8 files reviewed, all discussions resolved (waiting on @jasonleenaylor)

---

src/components/TreeView/tests/TreeViewHeader.test.tsx, line 47 at r4 (raw file):

  describe("searchAndSelectDomain", () => {
    function setupSimulatedInputTest(input: string) {
      // Simulate the user typing 10

The ghost of the 10 haunts you still.

[imnasnainaec]

Reviewed 1 of 1 files at r4.
Reviewable status: complete! all files reviewed, all discussions resolved

[imnasnainaec]

Reviewed 3 of 3 files at r5.
Reviewable status: complete! all files reviewed, all discussions resolved