(#3299) fixed nesting of Headers

The NavLinkBuilder was modifying arrays in place,
as well as returning the modifications. While this was probably not
the cause of the error, I fixed this by not returning the modifications
anymore.

I changed the algorithm from counting the depth up, to counting the
order down to 0. Thus, I could remove the depth entirely.

samples/react-page-navigator/src/Service/NavLinkBuilder.test.ts

@@ -20,7 +20,8 @@ describe("The NavLinkBuilder without a preceding collapsable header", () => {
         name: "xyz",
     };
 
-    const actual = navLinkBuilder.build([], lnk, h1, depth);
+    const actual = [];
+    navLinkBuilder.build(actual, lnk, h1);
 
     expect(actual).toMatchSnapshot();
   });
@@ -33,8 +34,9 @@ describe("The NavLinkBuilder without a preceding collapsable header", () => {
       name: "abc",
     };
 
-    let actual = navLinkBuilder.build([], lnk1, h1, depth);
+    const actual = [];
-    actual = navLinkBuilder.build(actual, lnk2, h1, depth);
+    navLinkBuilder.build(actual, lnk1, h1);
+    navLinkBuilder.build(actual, lnk2, h1);
 
     expect(actual).toMatchSnapshot();
   })
@@ -47,8 +49,9 @@ describe("The NavLinkBuilder without a preceding collapsable header", () => {
       name: "abc",
     };
 
-    let actual = navLinkBuilder.build([], lnk1, h1, depth);
+    const actual = [];
-    actual = navLinkBuilder.build(actual, lnk2, h2, depth);
+    navLinkBuilder.build(actual, lnk1, h1);
+    navLinkBuilder.build(actual, lnk2, h2);
 
     expect(actual).toMatchSnapshot();
   });
@@ -67,10 +70,11 @@ describe("The NavLinkBuilder without a preceding collapsable header", () => {
       name: "abc.1",
     };
 
-    let actual = navLinkBuilder.build([], lnk1, h1, depth);
+    const actual = [];
-    actual = navLinkBuilder.build(actual, lnk11, h2, depth);
+    navLinkBuilder.build(actual, lnk1, h1);
-    actual = navLinkBuilder.build(actual, lnk2, h1, depth);
+    navLinkBuilder.build(actual, lnk11, h2);
-    actual = navLinkBuilder.build(actual, lnk21, h2, depth);
+    navLinkBuilder.build(actual, lnk2, h1);
+    navLinkBuilder.build(actual, lnk21, h2);
 
     expect(actual).toMatchSnapshot();
   });
@@ -89,127 +93,15 @@ describe("The NavLinkBuilder without a preceding collapsable header", () => {
       name: "geh",
     };
 
-    let actual = navLinkBuilder.build([], lnk1, h1, depth);
+    const actual = [];
-    actual = navLinkBuilder.build(actual, lnk2, h2, depth);
+    navLinkBuilder.build(actual, lnk1, h1);
-    actual = navLinkBuilder.build(actual, lnk3, h3, depth);
+    navLinkBuilder.build(actual, lnk2, h2);
-    actual = navLinkBuilder.build(actual, lnk4, h4, depth);
+    navLinkBuilder.build(actual, lnk3, h3);
+    navLinkBuilder.build(actual, lnk4, h4);
 
     expect(actual).toMatchSnapshot();
   });
 
-  it.skip("should not nest two h2", () => {
-    const lnk1: IMockLink = {
-      name: "h1",
-    };
-    const lnk2: IMockLink = {
-      name: "h2",
-    };
-    const lnk3: IMockLink = {
-      name: "h2",
-    };
-    const lnk4: IMockLink = {
-      name: "h3",
-    };
-
-    let actual = navLinkBuilder.build([], lnk1, h1, depth);
-    actual = navLinkBuilder.build(actual, lnk2, h2, depth);
-    actual = navLinkBuilder.build(actual, lnk3, h2, depth);
-    actual = navLinkBuilder.build(actual, lnk4, h3, depth);
-
-    expect(actual).toMatchSnapshot();
-  });
-
-  it.skip("should not nest two h3", () => {
-    const lnk0: IMockLink = {
-      name: "h1",
-    };
-    const lnk1: IMockLink = {
-      name: "h1",
-    };
-    const lnk2: IMockLink = {
-      name: "h2",
-    };
-    const lnk3: IMockLink = {
-      name: "h2",
-    };
-    const lnk4: IMockLink = {
-      name: "h3",
-    };
-    const lnk5: IMockLink = {
-      name: "h3",
-    };
-    const lnk6: IMockLink = {
-      name: "h4",
-    };
-
-    let actual = navLinkBuilder.build([], lnk0, h1, depth);
-    actual = navLinkBuilder.build(actual, lnk1, h1, depth);
-    actual = navLinkBuilder.build(actual, lnk2, h2, depth);
-    actual = navLinkBuilder.build(actual, lnk3, h2, depth);
-    actual = navLinkBuilder.build(actual, lnk4, h3, depth);
-    actual = navLinkBuilder.build(actual, lnk5, h3, depth);
-    actual = navLinkBuilder.build(actual, lnk6, h4, depth);
-
-    expect(actual).toMatchSnapshot();
-  });
-});
-
-describe("The NavLinkBuilder with a collapsable header", () => {
-  let head: IMockLink;
-  const depth = DEPTH_COLLAPSABLE_HEADER;
-  const h1 = depth;
-  const h2 = h1+1;
-  const h3 = h2+1;
-  const h4 = h3+1;
-
-  beforeEach(() => {
-    head = {
-      name: "head",
-    };
-  });
-
-  it("should add a single item to the heading", () => {
-    const lnk: IMockLink = {
-      name: "xyz",
-    };
-
-    const actual = navLinkBuilder.build([ head ], lnk, h1, depth);
-
-    expect(actual).toMatchSnapshot();
-  });
-
-  it("should add a two items on the same level", () => {
-    const lnk1: IMockLink = {
-      name: "xyz",
-    };
-    const lnk2: IMockLink = {
-      name: "abc",
-    };
-
-    let actual = navLinkBuilder.build([ head ], lnk1, h1, depth);
-    actual = navLinkBuilder.build(actual, lnk2, h1, depth);
-
-    expect(actual).toMatchSnapshot();
-  })
-
-  it("should add a one item in a collapsable section two inside that one", () => {
-    const lnk1: IMockLink = {
-      name: "xyz",
-    };
-    const lnk2: IMockLink = {
-      name: "abc",
-    };
-    const lnk3: IMockLink = {
-      name: "def",
-    };
-
-    let actual = navLinkBuilder.build([ head ], lnk1, h1, depth);
-    actual = navLinkBuilder.build(actual, lnk2, h2, depth);
-    actual = navLinkBuilder.build(actual, lnk3, h2, depth);
-
-    expect(actual).toMatchSnapshot();
-  })
-
   it("should not nest two h2", () => {
     const lnk1: IMockLink = {
       name: "h1",
@@ -224,10 +116,11 @@ describe("The NavLinkBuilder with a collapsable header", () => {
       name: "h3",
     };
 
-    let actual = navLinkBuilder.build([ head ], lnk1, h1, depth);
+    const actual = []
-    actual = navLinkBuilder.build(actual, lnk2, h2, depth);
+    navLinkBuilder.build(actual, lnk1, h1);
-    actual = navLinkBuilder.build(actual, lnk3, h2, depth);
+    navLinkBuilder.build(actual, lnk2, h2);
-    actual = navLinkBuilder.build(actual, lnk4, h3, depth);
+    navLinkBuilder.build(actual, lnk3, h2);
+    navLinkBuilder.build(actual, lnk4, h3);
 
     expect(actual).toMatchSnapshot();
   });
@@ -255,13 +148,132 @@ describe("The NavLinkBuilder with a collapsable header", () => {
       name: "h4",
     };
 
-    let actual = navLinkBuilder.build([ head ], lnk0, h1, depth);
+    const actual = [];
-    actual = navLinkBuilder.build(actual, lnk1, h1, depth);
+    navLinkBuilder.build(actual, lnk0, h1);
-    actual = navLinkBuilder.build(actual, lnk2, h2, depth);
+    navLinkBuilder.build(actual, lnk1, h1);
-    actual = navLinkBuilder.build(actual, lnk3, h2, depth);
+    navLinkBuilder.build(actual, lnk2, h2);
-    actual = navLinkBuilder.build(actual, lnk4, h3, depth);
+    navLinkBuilder.build(actual, lnk3, h2);
-    actual = navLinkBuilder.build(actual, lnk5, h3, depth);
+    navLinkBuilder.build(actual, lnk4, h3);
-    actual = navLinkBuilder.build(actual, lnk6, h4, depth);
+    navLinkBuilder.build(actual, lnk5, h3);
+    navLinkBuilder.build(actual, lnk6, h4);
+
+    expect(actual).toMatchSnapshot();
+  });
+});
+
+describe("The NavLinkBuilder with a collapsable header", () => {
+  let head: IMockLink;
+  const depth = DEPTH_COLLAPSABLE_HEADER;
+  const h1 = depth;
+  const h2 = h1+1;
+  const h3 = h2+1;
+  const h4 = h3+1;
+
+  beforeEach(() => {
+    head = {
+      name: "head",
+    };
+  });
+
+  it("should add a single item to the heading", () => {
+    const lnk: IMockLink = {
+      name: "xyz",
+    };
+
+    const actual = [ head ];
+    navLinkBuilder.build(actual, lnk, h1);
+
+    expect(actual).toMatchSnapshot();
+  });
+
+  it("should add a two items on the same level", () => {
+    const lnk1: IMockLink = {
+      name: "xyz",
+    };
+    const lnk2: IMockLink = {
+      name: "abc",
+    };
+
+    const actual = [ head ];
+    navLinkBuilder.build(actual, lnk1, h1);
+    navLinkBuilder.build(actual, lnk2, h1);
+
+    expect(actual).toMatchSnapshot();
+  })
+
+  it("should add a one item in a collapsable section two inside that one", () => {
+    const lnk1: IMockLink = {
+      name: "xyz",
+    };
+    const lnk2: IMockLink = {
+      name: "abc",
+    };
+    const lnk3: IMockLink = {
+      name: "def",
+    };
+
+    const actual = [ head ];
+    navLinkBuilder.build(actual, lnk1, h1);
+    navLinkBuilder.build(actual, lnk2, h2);
+    navLinkBuilder.build(actual, lnk3, h2);
+
+    expect(actual).toMatchSnapshot();
+  })
+
+  it("should not nest two h2", () => {
+    const lnk1: IMockLink = {
+      name: "h1",
+    };
+    const lnk2: IMockLink = {
+      name: "h2",
+    };
+    const lnk3: IMockLink = {
+      name: "h2",
+    };
+    const lnk4: IMockLink = {
+      name: "h3",
+    };
+
+    const actual = [ head ];
+    navLinkBuilder.build(actual, lnk1, h1);
+    navLinkBuilder.build(actual, lnk2, h2);
+    navLinkBuilder.build(actual, lnk3, h2);
+    navLinkBuilder.build(actual, lnk4, h3);
+
+    expect(actual).toMatchSnapshot();
+  });
+
+  it("should not nest two h3", () => {
+    const lnk0: IMockLink = {
+      name: "h1",
+    };
+    const lnk1: IMockLink = {
+      name: "h1",
+    };
+    const lnk2: IMockLink = {
+      name: "h2",
+    };
+    const lnk3: IMockLink = {
+      name: "h2",
+    };
+    const lnk4: IMockLink = {
+      name: "h3",
+    };
+    const lnk5: IMockLink = {
+      name: "h3",
+    };
+    const lnk6: IMockLink = {
+      name: "h4",
+    };
+
+    const actual = [ head ];
+    navLinkBuilder.build(actual, lnk0, h1);
+    navLinkBuilder.build(actual, lnk1, h1);
+    navLinkBuilder.build(actual, lnk2, h2);
+    navLinkBuilder.build(actual, lnk3, h2);
+    navLinkBuilder.build(actual, lnk4, h3);
+    navLinkBuilder.build(actual, lnk5, h3);
+    navLinkBuilder.build(actual, lnk6, h4);
 
     expect(actual).toMatchSnapshot();
   });

samples/react-page-navigator/src/Service/NavLinkBuilder.ts

@@ -4,34 +4,24 @@ export interface IHierarchyEntry<T extends IHierarchyEntry<T>> {
 
 export class navLinkBuilder {
 /**
-   * Nests a new nav link within the nav links tree
+ * Nests a new nav link within the nav links tree. Modifies the current tree IN PLACE.
  * @param currentLinks current nav links
  * @param newLink the new nav link to be added to the structure
  * @param order place order of the new link
-   * @param depth sequence depth
  * @returns navLinks
  */
-   public static build<T extends IHierarchyEntry<T>>(currentLinks: T[], newLink: T, order: number, depth: number): T[] {
+ public static build<T extends IHierarchyEntry<T>>(currentLinks: T[], newLink: T, order: number) {
   const lastIndex = currentLinks.length - 1;
 
-    if (lastIndex === -1) {
+  if (lastIndex < 0 || order <= 0) {
-      return [newLink];
+    currentLinks.push(newLink);
+    return;
   }
 
   const lastTopLevelLink = currentLinks[lastIndex];
   lastTopLevelLink.links = lastTopLevelLink.links || [];
 
-    if (lastTopLevelLink.links.length === 0 || order === depth) {
+  order--;
-      if (order !== depth || depth !== 0) {
+  this.build(lastTopLevelLink.links, newLink, order);
-        lastTopLevelLink.links.push(newLink);
-      } else {
-        currentLinks.push(newLink);
-      }
-    } else {
-      depth++;
-      currentLinks[lastIndex].links.concat(this.build(currentLinks[lastIndex].links, newLink, order, depth));
-    }
-
-    return currentLinks;
 }
 }

samples/react-page-navigator/src/Service/SPService.ts

@@ -100,7 +100,7 @@ export class SPService {
 
             // Add link to nav element
             const newNavLink: INavLink = { name: headingValue, key: anchorUrl, url: anchorUrl, links: [], isExpanded: true };
-            anchorLinks = navLinkBuilder.build<INavLink>(anchorLinks, newNavLink, headingOrder, hasCollapsableHeader ? 1 : 0);
+            navLinkBuilder.build<INavLink>(anchorLinks, newNavLink, headingOrder);
           });
         }
       });

samples/react-page-navigator/src/Service/__snapshots__/NavLinkBuilder.test.ts.snap

@@ -160,7 +160,6 @@ Array [
 exports[`The NavLinkBuilder without a preceding collapsable header should add a two items on the same level 1`] = `
 Array [
   Object {
-    "links": Array [],
     "name": "xyz",
   },
   Object {
@@ -195,7 +194,6 @@ Array [
   Object {
     "links": Array [
       Object {
-        "links": Array [],
         "name": "h2",
       },
       Object {
@@ -215,13 +213,11 @@ Array [
 exports[`The NavLinkBuilder without a preceding collapsable header should not nest two h3 1`] = `
 Array [
   Object {
-    "links": Array [],
     "name": "h1",
   },
   Object {
     "links": Array [
       Object {
-        "links": Array[],
         "name": "h2",
       },
       Object {