Show group markers in search (#355)

closes rdar://90655322

* feat: show group markers in search

* fix: make sure deprecated items stay hidden, even if a groupMarker labels deprecated items.

Author: Dobromir Hristov

src/components/Navigator.vue

@@ -176,6 +176,8 @@ export default {
       let items = [];
       const len = childrenNodes.length;
       let index;
+      // reference to the last label node
+      let groupMarkerNode = null;
       for (index = 0; index < len; index += 1) {
         // get the children
         const { children, ...node } = childrenNodes[index];
@@ -185,6 +187,13 @@ export default {
         node.uid = this.hashCode(`${parentUID}+${node.path}_${depth}_${index}`);
         // store the parent uid
         node.parent = parentUID;
+        // store the current groupMarker reference
+        if (node.type === TopicTypes.groupMarker) {
+          groupMarkerNode = node;
+        } else if (groupMarkerNode) {
+          // push the current node to the group marker before it
+          groupMarkerNode.childUIDs.push(node.uid);
+        }
         // store which item it is
         node.index = index;
         // store how many siblings it has

src/components/Navigator/NavigatorCard.vue

@@ -251,7 +251,6 @@ export default {
       /** @type {NavigatorFlatItem[]} */
       nodesToRender: [],
       activeUID: null,
-      resetScroll: false,
       store: QuickNavigationStore,
       lastFocusTarget: null,
       NO_RESULTS,
@@ -331,10 +330,11 @@ export default {
         FILTER_TAGS_TO_LABELS[tag] || ChangeNames[tag] || tag
       )),
       set(values) {
+        // guard against accidental clearings
+        if (!this.selectedTags.length && !values.length) return;
         this.selectedTags = values.map(label => (
           FILTER_LABELS_TO_TAGS[label] || ChangeNameToType[label] || label
         ));
-        this.resetScroll = true;
       },
     },
     filterPattern: ({ debouncedFilter }) => (!debouncedFilter
@@ -374,7 +374,7 @@ export default {
      */
     filteredChildren({
       hasFilter, children, filterPattern, selectedTags,
-      apiChangesObject, apiChanges, deprecatedHidden,
+      apiChangesObject, apiChanges,
     }) {
       if (!hasFilter) return [];
       const tagsSet = new Set(selectedTags);
@@ -398,15 +398,13 @@ export default {
         }
         // find items, that have API changes
         const hasAPIChanges = apiChanges ? apiChangesObject[path] : true;
-        // group markers are hidden when filtering, unless "Hide Deprecated" is ON.
-        const isGroupMarker = deprecatedHidden ? false : type === TopicTypes.groupMarker;
         // make sure groupMarker's dont get matched
-        return titleMatch && tagMatch && hasAPIChanges && !isGroupMarker;
+        return titleMatch && tagMatch && hasAPIChanges;
       });
     },
     /**
      * Returns a Set of all nodes that match a filter, along with their parents.
-     * @returns NavigatorFlatItem[]
+     * @returns Set<NavigatorFlatItem>
      */
     filteredChildrenUpToRootSet: ({ filteredChildren, getParents }) => new Set(
       filteredChildren.flatMap(({ uid }) => getParents(uid)),
@@ -416,26 +414,30 @@ export default {
      * This is used on both toggling, as well as on navigation and filtering.
      * @return {Object.<string, NavigatorFlatItem>}
      */
-    renderableChildNodesMap({ filteredChildrenUpToRootSet, childrenMap, hasFilter }) {
+    renderableChildNodesMap({
+      filteredChildrenUpToRootSet, childrenMap, hasFilter,
+      getAllChildren, convertChildrenArrayToObject, removeDeprecated,
+    }) {
       if (!hasFilter) return childrenMap;
       let all = [];
       // create a set of all matches and their parents
       filteredChildrenUpToRootSet.forEach((current) => {
-        // if it has no children, just add it
+        // if it's a plain end node, just add it
         if (!current.childUIDs.length) {
           all.push(current);
           return;
         }
+        // check if none of the child items of this parent are matching
         const noChildrenMatch = !current.childUIDs.some(uid => (
           filteredChildrenUpToRootSet.has(childrenMap[uid])
         ));
-        all = all.concat(noChildrenMatch ? this.getAllChildren(current.uid) : current);
+        // if no children are matching, add all to the list, otherwise just the current parent
+        all = all.concat(noChildrenMatch ? removeDeprecated(getAllChildren(current.uid)) : current);
       });
-
-      return this.convertChildrenArrayToObject(all);
+      return convertChildrenArrayToObject(all);
     },
     /**
-     * Creates a computed for the two items, that the openNodes calc depends on
+     * Creates a computed for the items, that the openNodes calc depends on
      */
     nodeChangeDeps: ({
       filteredChildren, activePathChildren, debouncedFilter, selectedTags,
@@ -454,9 +456,7 @@ export default {
      * If we enable multiple tags, this should be an include instead.
      * @returns boolean
      */
-    deprecatedHidden: ({ selectedTags, debouncedFilter }) => (
-      selectedTags[0] === HIDE_DEPRECATED_TAG && !debouncedFilter.length
-    ),
+    deprecatedHidden: ({ selectedTags }) => selectedTags[0] === HIDE_DEPRECATED_TAG,
     apiChangesObject() {
       return this.apiChanges || {};
     },
@@ -486,16 +486,13 @@ export default {
       this.filter = '';
       this.debouncedFilter = '';
       this.selectedTags = [];
-      this.resetScroll = true;
     },
     scrollToFocus() {
       this.$refs.scroller.scrollToItem(this.focusedIndex);
     },
     debounceInput: debounce(function debounceInput(value) {
       // store the new filter value
       this.debouncedFilter = value;
-      // note to the component, that we want to reset the scroll
-      this.resetScroll = true;
       // reset the last focus target
       this.lastFocusTarget = null;
     }, 500),
@@ -524,7 +521,11 @@ export default {
       // decide which items are open
       // if "Hide Deprecated" is picked, there is no filter,
       // or navigate to page while filtering, we open the items leading to the activeUID
-      const nodes = this.deprecatedHidden || (pageChange && this.hasFilter) || !this.hasFilter
+      const nodes = (
+        (this.deprecatedHidden && !this.debouncedFilter.length)
+        || (pageChange && this.hasFilter)
+        || !this.hasFilter
+      )
         ? activePathChildren
         // get all parents of the current filter match, excluding it in the process
         : filteredChildren.flatMap(({ uid }) => this.getParents(uid).slice(0, -1));
@@ -698,6 +699,15 @@ export default {
       return (item.childUIDs || [])
         .map(child => this.childrenMap[child]);
     },
+    /**
+     * Removes deprecated items from a list
+     * @param {NavigatorFlatItem[]} items
+     * @returns {NavigatorFlatItem[]}
+     */
+    removeDeprecated(items) {
+      if (!this.deprecatedHidden) return items;
+      return items.filter(({ deprecated }) => !deprecated);
+    },
     /**
      * Stores all the nodes we should render at this point.
      * This gets called everytime you open/close a node,
@@ -860,7 +870,7 @@ export default {
       await waitFrames(1);
       if (!this.$refs.scroller) return;
       // if we are filtering, it makes more sense to scroll to top of list
-      if (this.resetScroll) {
+      if (this.hasFilter) {
         this.$refs.scroller.scrollToItem(0);
         return;
       }
@@ -951,7 +961,6 @@ export default {
      */
     setActiveUID(uid) {
       this.activeUID = uid;
-      this.resetScroll = false;
     },
     /**
      * Handles the `navigate` event from NavigatorCardItem, guarding from selecting an item,

src/components/Navigator/NavigatorCardItem.vue

@@ -163,7 +163,7 @@ export default {
   },
   computed: {
     isGroupMarker: ({ item: { type } }) => type === TopicTypes.groupMarker,
-    isParent: ({ item }) => !!item.childUIDs.length,
+    isParent: ({ item, isGroupMarker }) => !!item.childUIDs.length && !isGroupMarker,
     parentLabel: ({ item }) => `label-parent-${item.uid}`,
     siblingsLabel: ({ item }) => `label-${item.uid}`,
     usageLabel: ({ item }) => `usage-${item.uid}`,

tests/unit/components/Navigator.spec.js

@@ -22,11 +22,19 @@ jest.mock('docc-render/utils/throttle', () => jest.fn(v => v));
 const technology = {
   title: 'FooTechnology',
   children: [
+    {
+      title: 'Group Marker',
+      type: TopicTypes.groupMarker,
+    },
     {
       title: 'Child0',
       path: '/foo/child0',
       type: 'article',
       children: [
+        {
+          title: 'Group Marker, Child 0',
+          type: TopicTypes.groupMarker,
+        },
         {
           title: 'Child0_GrandChild0',
           path: '/foo/child0/grandchild0',
@@ -284,100 +292,127 @@ describe('Navigator', () => {
   it('flattens deeply nested children and provides them to the NavigatorCard', () => {
     const wrapper = createWrapper();
     expect(wrapper.find(NavigatorCard).props('children')).toEqual([
-      // root
       {
         childUIDs: [
-          745124197,
-          746047719,
-          746971241,
+          551503844,
+          -97593391,
         ],
         depth: 0,
         index: 0,
-        type: 'article',
-        siblingsCount: 2,
+        parent: INDEX_ROOT_KEY,
+        siblingsCount: 3,
+        title: 'Group Marker',
+        type: 'groupMarker',
+        uid: -196255993,
+      },
+      {
+        childUIDs: [
+          -361407047,
+          1438225895,
+          1439149417,
+          1440072939,
+        ],
+        depth: 0,
+        index: 1,
         parent: INDEX_ROOT_KEY,
         path: '/foo/child0',
+        siblingsCount: 3,
         title: 'Child0',
-        uid: 551503843,
+        type: 'article',
+        uid: 551503844,
       },
       {
-        childUIDs: [],
+        childUIDs: [
+          1438225895,
+          1439149417,
+          1440072939,
+        ],
         depth: 1,
         index: 0,
-        type: 'tutorial',
-        parent: 551503843,
-        siblingsCount: 3,
+        parent: 551503844,
+        siblingsCount: 4,
+        title: 'Group Marker, Child 0',
+        type: 'groupMarker',
+        uid: -361407047,
+      },
+      {
+        childUIDs: [],
+        depth: 1,
+        index: 1,
+        parent: 551503844,
         path: '/foo/child0/grandchild0',
+        siblingsCount: 4,
         title: 'Child0_GrandChild0',
-        uid: 745124197,
+        type: 'tutorial',
+        uid: 1438225895,
       },
       {
         childUIDs: [
-          1489150959,
+          305326087,
         ],
         depth: 1,
-        index: 1,
-        type: 'tutorial',
-        parent: 551503843,
-        siblingsCount: 3,
+        index: 2,
+        parent: 551503844,
         path: '/foo/child0/grandchild1',
+        siblingsCount: 4,
         title: 'Child0_GrandChild1',
-        uid: 746047719,
+        type: 'tutorial',
+        uid: 1439149417,
       },
       {
         childUIDs: [],
         depth: 2,
         index: 0,
-        type: 'tutorial',
-        parent: 746047719,
-        siblingsCount: 1,
+        parent: 1439149417,
         path: '/foo/child0/grandchild0/greatgrandchild0',
+        siblingsCount: 1,
         title: 'Child0_GrandChild0_GreatGrandChild0',
-        uid: 1489150959,
+        type: 'tutorial',
+        uid: 305326087,
       },
       {
         childUIDs: [],
         depth: 1,
-        index: 2,
-        type: 'tutorial',
-        parent: 551503843,
-        siblingsCount: 3,
+        index: 3,
+        parent: 551503844,
         path: '/foo/child0/grandchild2',
+        siblingsCount: 4,
         title: 'Child0_GrandChild2',
-        uid: 746971241,
+        type: 'tutorial',
+        uid: 1440072939,
       },
       {
         childUIDs: [
-          -134251586,
+          -827353283,
         ],
         depth: 0,
-        index: 1,
-        type: 'tutorial',
+        index: 2,
         parent: INDEX_ROOT_KEY,
-        siblingsCount: 2,
         path: '/foo/child1/',
+        siblingsCount: 3,
         title: 'Child1',
-        uid: -97593392,
+        type: 'tutorial',
+        uid: -97593391,
       },
       {
         childUIDs: [],
         depth: 1,
         index: 0,
-        type: 'method',
-        parent: -97593392,
-        siblingsCount: 1,
+        parent: -97593391,
         path: '/foo/child1/grandchild0',
+        siblingsCount: 1,
         title: 'Child1_GrandChild0',
-        uid: -134251586,
+        type: 'method',
+        uid: -827353283,
       },
     ]);
   });
 
   it('removes the `beta` flag from children, if the technology is a `beta`', () => {
     const technologyClone = clone(technology);
     technologyClone.beta = true;
-    technologyClone.children[0].beta = true;
-    technologyClone.children[0].children[0].beta = true;
+    technologyClone.children[1].beta = true;
+    technologyClone.children[1].children[0].beta = true;
     const wrapper = createWrapper({
       propsData: {
         technology: technologyClone,
@@ -388,12 +423,12 @@ describe('Navigator', () => {
 
   it('removes the `beta` flag from children, if the parent is a `beta`', () => {
     const technologyClone = clone(technology);
-    technologyClone.children[0].beta = true;
-    technologyClone.children[0].children[0].beta = true;
+    technologyClone.children[1].beta = true;
+    technologyClone.children[1].children[1].beta = true;
     // case where the direct parent is NOT `Beta`, but an ancestor is
-    technologyClone.children[0].children[1].children[0].beta = true;
+    technologyClone.children[1].children[2].children[0].beta = true;
     // set an end node as beta
-    technologyClone.children[1].children[0].beta = true;
+    technologyClone.children[2].children[0].beta = true;
     const wrapper = createWrapper({
       propsData: {
         technology: technologyClone,