[TreeView] lazy load domains; store domain map in redux

[jasonleenaylor]

Fixes #664
Fixes #1555
Resolves #1546

This change is 

[codecov-commenter]

Codecov Report

Merging #1565 (2ed18a4) into master (4547526) will increase coverage by 27.65%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master    #1565       +/-   ##
===========================================
+ Coverage   52.12%   79.78%   +27.65%     
===========================================
  Files         269       37      -232     
  Lines        8222     3428     -4794     
  Branches      605        0      -605     
===========================================
- Hits         4286     2735     -1551     
+ Misses       3451      693     -2758     
+ Partials      485        0      -485     

Flag	Coverage Δ
backend	79.78% <ø> (ø)
frontend	?

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

Impacted Files	Coverage Δ
src/components/AppBar/NavigationButtons.tsx
src/components/DataEntry/DataEntryComponent.tsx
src/components/DataEntry/index.ts
src/components/TreeView/DomainTile.tsx
src/components/TreeView/TreeDepiction.tsx
src/components/TreeView/TreeSearch.tsx
src/components/TreeView/TreeSemanticDomain.ts
src/components/TreeView/TreeViewActions.ts
src/components/TreeView/TreeViewComponent.tsx
src/components/TreeView/TreeViewHeader.tsx
... and 220 more

---

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 4547526...2ed18a4. Read the comment docs.

[johnthagen]

---

src/components/DataEntry/DataEntryComponent.tsx, line 154 at r3 (raw file):

        />

        <Dialog fullScreen open={!!this.props.treeIsOpen}>

One example for the reason I am hesitant these kinds of terse implicit bool conversions: https://stackoverflow.com/questions/784929/what-is-the-not-not-operator-in-javascript#comment19202564_10597474

In this case, the !! is valid, but if we were to use it pervasively I think we could lead ourselves into a scenario where we lose type safety and bugs are easier to miss at compile time.

[johnthagen]

---

src/components/DataEntry/tests/spellChecker.test.ts, line 20 at r3 (raw file):

  });

  it("correctly detects a correctly spelled word", () => {

The grammar here is off now.

[johnthagen]

---

src/components/TreeView/TreeDepiction.tsx, line 150 at r3 (raw file):

    const childIds = this.props.currentDomain.childIds;
    const subdomains: ReactElement[] = [];
    for (let i = 0; i < childIds.length; i++) {

This could be written cleaner with:

    childIds.forEach((childId, i) => {
      
    });

[johnthagen]

---

src/components/TreeView/TreeSearch.tsx, line 84 at r3 (raw file):

      checkAgainst && target.toLowerCase() === checkAgainst.name.toLowerCase();
    for (const id in props.domainMap) {
      const domain = props.domainMap[id];

This can be simplified to

for (const domain of Object.values(props.domainMap))

[johnthagen]

---

src/components/TreeView/TreeViewActions.ts, line 44 at r3 (raw file):

  domainMap: DomainMap,
  parentId?: string
) {

Add : void?

[johnthagen]

---

src/components/TreeView/TreeViewActions.ts, line 54 at r3 (raw file):

}

// Parses a list of semantic domains (to be loaded from file)

Make a doc comment?

[johnthagen]

---

src/components/TreeView/TreeViewComponent.tsx, line 35 at r3 (raw file):

}

export function TreeView(props: TreeViewProps & LocalizeContextProps) {

: ReactElement ?

[johnthagen]

---

src/components/TreeView/TreeViewComponent.tsx, line 64 at r3 (raw file):

      const headString = props.translate("addWords.domain") as string;
      dispatch(updateTreeLanguage(newLang, headString));
    } // eslint-disable-next-line react-hooks/exhaustive-deps

Add comment for why suppressing this is safe?

[johnthagen]

---

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

          : event.key === Key.ArrowDown &&
            props.currentDomain.childIds.length === 1
          ? props.domainMap[props.currentDomain.childIds[0]]

This long chain is pretty challenging to follow. Is there a way we could use a switch (event.key) to possibly make this easier to follow?

[johnthagen]

---

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

          ? props.domainMap[props.currentDomain.childIds[0]]
          : undefined;
      if (domain) {

Consider domain !== undefined for clarity?

[johnthagen]

---

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

    const subdomains = [
      new TreeSemanticDomain("Bar", "5.1") as any,
      new TreeSemanticDomain("Baz", "5.2") as any,

Is it possible to avoid the use of any here?

[johnthagen]

Reviewed 25 of 25 files at r3, all commit messages.
Reviewable status: 16 of 25 files reviewed, 9 unresolved discussions (waiting on @jasonleenaylor and @johnthagen)

[johnthagen]

---

src/components/TreeView/TreeViewComponent.tsx, line 35 at r3 (raw file):

Previously, johnthagen wrote…

: ReactElement ?

Done

[johnthagen]

---

src/components/TreeView/TreeViewComponent.tsx, line 64 at r3 (raw file):

Previously, johnthagen wrote…

Add comment for why suppressing this is safe?

Done.

[johnthagen]

Reviewed 7 of 8 files at r4, 2 of 2 files at r5.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @jasonleenaylor)

[johnthagen]

I confirmed that #664, #1555, and #1546 (comment) are fixed.

[imnasnainaec]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jasonleenaylor)

---

src/components/DataEntry/DataEntryComponent.tsx, line 154 at r3 (raw file):

Previously, johnthagen wrote…

One example for the reason I am hesitant these kinds of terse implicit bool conversions: https://stackoverflow.com/questions/784929/what-is-the-not-not-operator-in-javascript#comment19202564_10597474

In this case, the !! is valid, but if we were to use it pervasively I think we could lead ourselves into a scenario where we lose type safety and bugs are easier to miss at compile time.

We have !! a handful of other times in the code. It could be a new issue.

---

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

Previously, johnthagen wrote…

Consider domain !== undefined for clarity?

This sort of implicit checking is currently ubiquitous in the frontend.

---

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

Previously, johnthagen wrote…

Is it possible to avoid the use of any here?

That's to enable deleting the childIds.

[imnasnainaec]

Reviewable status: 24 of 25 files reviewed, all discussions resolved (waiting on @jasonleenaylor and @johnthagen)

---

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

Previously, johnthagen wrote…

This long chain is pretty challenging to follow. Is there a way we could use a switch (event.key) to possibly make this easier to follow?

How's this look?

[johnthagen]

---

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

Previously, imnasnainaec (D. Ror.) wrote…

How's this look?

MUCH better!

[johnthagen]

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @jasonleenaylor)