Remove immediate parent in breadcrumbs if symbol belongs in a group of overloads (#790) rdar://125174518 and rdar://125092351

Remove immediate parent in breadcrumbs if symbol belongs in a group of overloads (#790)
rdar://125174518 and rdar://125092351

Author: hqhhuang

src/components/DocumentationTopic.vue

@@ -31,11 +31,11 @@
         </template>
         <slot name="above-title" />
         <Hierarchy
-          v-if="hierarchyItems.length && !enableMinimized"
+          v-if="parentTopics.length && !enableMinimized && !isTargetIDE"
           :currentTopicTitle="title"
           :isSymbolDeprecated="isSymbolDeprecated"
           :isSymbolBeta="isSymbolBeta"
-          :parentTopicIdentifiers="hierarchyItems"
+          :parentTopics="parentTopics"
           :currentTopicTags="tags"
         />
         <LanguageSwitcher
@@ -89,7 +89,7 @@
           :class="{ 'no-primary-content': !hasPrimaryContent && enhanceBackground }"
         >
           <div
-            v-if="hasPrimaryContent || hasOtherDeclarations"
+            v-if="hasPrimaryContent || showOtherDeclarations"
             :class="['container', { 'minimized-container': enableMinimized }]"
           >
             <div
@@ -111,7 +111,7 @@
                 <ContentNode :content="downloadNotAvailableSummary" />
               </Aside>
             </div>
-            <div v-if="hasOtherDeclarations" class="declaration-list-menu">
+            <div v-if="showOtherDeclarations" class="declaration-list-menu">
               <button
                 class="declaration-list-toggle"
                 @click="toggleDeclList"
@@ -174,6 +174,7 @@ import Language from 'docc-render/constants/Language';
 import metadata from 'theme/mixins/metadata';
 import { buildUrl } from 'docc-render/utils/url-helper';
 import { normalizeRelativePath } from 'docc-render/utils/assets';
+import { last } from 'docc-render/utils/arrays';
 
 import AppStore from 'docc-render/stores/AppStore';
 import Aside from 'docc-render/components/ContentNode/Aside.vue';
@@ -448,6 +449,30 @@ export default {
     pageDescription: ({ abstract, extractFirstParagraphText }) => (
       abstract ? extractFirstParagraphText(abstract) : null
     ),
+    parentTopics: ({
+      hierarchyItems,
+      references,
+      hasOtherDeclarations,
+      pageTitle,
+    }) => {
+      const parentTopics = hierarchyItems.reduce((all, id) => {
+        const reference = references[id];
+        if (reference) {
+          const { title, url } = reference;
+          return all.concat({ title, url });
+        }
+        console.error(`Reference for "${id}" is missing`);
+        return all;
+      }, []);
+
+      // Overloaded symbols are auto-grouped under a group page with the same title
+      // We should omit showing their immediate parent to avoid confusion and duplication
+      const immediateParent = last(parentTopics);
+      if (hasOtherDeclarations && immediateParent?.title === pageTitle) {
+        parentTopics.pop();
+      }
+      return parentTopics;
+    },
     shouldShowLanguageSwitcher: ({
       objcPath,
       swiftPath,
@@ -553,9 +578,12 @@ export default {
     declarations({ primaryContentSections = [] }) {
       return primaryContentSections.filter(({ kind }) => kind === SectionKind.declarations);
     },
-    hasOtherDeclarations({ declarations = [], enableMinimized }) {
+    showOtherDeclarations({ enableMinimized, hasOtherDeclarations }) {
       // disable otherDeclarations in minimized mode
-      return !enableMinimized && declarations.length
+      return !enableMinimized && hasOtherDeclarations;
+    },
+    hasOtherDeclarations({ declarations = [] }) {
+      return !!declarations.length
         // there's always only 1 `declaration` at this level
         && declarations[0].declarations.some(decl => Object.prototype.hasOwnProperty.call(decl, 'otherDeclarations'));
     },

src/components/DocumentationTopic/Hero/Hierarchy.vue

@@ -116,7 +116,7 @@ export default {
       type: String,
       required: true,
     },
-    parentTopicIdentifiers: {
+    parentTopics: {
       type: Array,
       default: () => [],
     },
@@ -127,17 +127,6 @@ export default {
   },
   computed: {
     windowWidth: ({ store }) => store.state.contentWidth,
-    parentTopics() {
-      return this.parentTopicIdentifiers.reduce((all, id) => {
-        const reference = this.references[id];
-        if (reference) {
-          const { title, url } = reference;
-          return all.concat({ title, url });
-        }
-        console.error(`Reference for "${id}" is missing`);
-        return all;
-      }, []);
-    },
     root: ({ parentTopics }) => parentTopics[0],
     smallViewport: ({ windowWidth }) => (windowWidth < BreakpointAttributes.default.small.maxWidth),
     /**

src/components/Navigator.vue

@@ -24,7 +24,6 @@
       :render-filter-on-top="renderFilterOnTop"
       :api-changes="apiChanges"
       :navigator-references="navigatorReferences"
-      :isSpecificOverload="isSpecificOverload"
       @close="$emit('close')"
     >
       <template #filter><slot name="filter" /></template>
@@ -49,7 +48,6 @@ import NavigatorCard from 'theme/components/Navigator/NavigatorCard.vue';
 import LoadingNavigatorCard from 'theme/components/Navigator/LoadingNavigatorCard.vue';
 import { INDEX_ROOT_KEY } from 'docc-render/constants/sidebar';
 import { TopicTypes } from 'docc-render/constants/TopicTypes';
-import { last } from 'docc-render/utils/arrays';
 
 /**
  * @typedef NavigatorFlatItem
@@ -125,9 +123,6 @@ export default {
       type: Object,
       default: null,
     },
-    symbolKind: {
-      default: () => undefined,
-    },
   },
   computed: {
     // gets the paths for each parent in the breadcrumbs
@@ -154,21 +149,6 @@ export default {
       }
       return parentTopicReferences.slice(itemsToSlice).map(r => r.url).concat(path);
     },
-    /**
-     * Symbol pages always have a symbolKind
-     */
-    isSymbol: ({ symbolKind }) => !!symbolKind,
-    /**
-     * Only symbol pages with overloads can have a valid hash:
-     * less than 5 char, only lower case letter and number
-     */
-    isSpecificOverload({ $route: { path }, isSymbol }) {
-      // Ensure the path does not have a trailing slash
-      // eslint-disable-next-line no-param-reassign
-      path = path.replace(/\/$/, '');
-      const hash = isSymbol ? last(path.split('-')) : '';
-      return /^[a-z0-9]{1,5}$/.test(hash);
-    },
     /**
      * The root item is always a module
      */

src/components/Navigator/NavigatorCard.vue

@@ -226,10 +226,6 @@ export default {
       type: Boolean,
       default: false,
     },
-    isSpecificOverload: {
-      type: Boolean,
-      default: false,
-    },
   },
   mixins: [
     keyboardNavigation, filteredChildrenMixin, tagsProvider,
@@ -914,74 +910,37 @@ export default {
         if (lastActivePathItem === currentActiveItem.path) {
           return;
         }
-        // try to match current open item in its surroundings, starting with active item
-        // set found match as active item
-        if (this.matchInSurroundingItems(this.activeUID, lastActivePathItem)) return;
+        // Get the surrounding items
+        const siblings = getSiblings(this.activeUID, this.childrenMap, this.children);
+        const children = getChildren(this.activeUID, this.childrenMap, this.children);
+        const parents = getParents(this.activeUID, this.childrenMap);
+        // try to match if any of the `siblings`,`children` or any of the `parents`,
+        // match the current open item
+        const matchingItem = [...children, ...siblings, ...parents]
+          .find(child => child.path === lastActivePathItem);
 
-        if (this.isSpecificOverload) {
-          // if no match, try again to match with generic item
-          // Needed for continuing to highlight current generic page
-          // when selecting an overload from dropdown that's also specifically curated in elsewhere
-          const genericItem = this.getGenericPath(lastActivePathItem);
-          if (this.matchInSurroundingItems(this.activeUID, genericItem)) return;
+        // set the match as an active item
+        if (matchingItem) {
+          this.setActiveUID(matchingItem.uid);
+          return;
         }
       }
-      // There is no match to base upon, so we need to search the whole tree
-      // by matching each level of the hierachy in activePath
+      // There is no match to base upon, so we need to search
+      // across the activePath for the active item.
       const activePathChildren = this.pathsToFlatChildren(activePath);
-      // if there are items, set new active UID
+      // if there are items, set the new active UID
       if (activePathChildren.length) {
-        const lastChildrenUID = last(activePathChildren).uid;
-
-        if (last(activePathChildren).path !== lastActivePathItem && this.isSpecificOverload) {
-          // if item is not found in the tree and its a specific overloaded symbol page
-          // try to match with its generics page instead
-          const genericItem = this.getGenericPath(lastActivePathItem);
-          if (this.matchInSurroundingItems(lastChildrenUID, genericItem)) return;
-        }
-
-        // Set new active UID to the last matched item
-        // Note: if a match is not found, last matched ancestor is highlighted
-        this.setActiveUID(lastChildrenUID);
+        this.setActiveUID(activePathChildren[activePathChildren.length - 1].uid);
         return;
       }
-      // if there is an activeUID, but still no match found in tree
-      // unset it, as we probably navigated back to the root
+      // if there is an activeUID, unset it, as we probably navigated back to the root
       if (this.activeUID) {
         this.setActiveUID(null);
         return;
       }
       // Just track the open nodes, as setting the activeUID as null wont do anything.
       this.trackOpenNodes(this.nodeChangeDeps);
     },
-    /**
-     * Try to match if any of the `siblings`,`children`, `parents`
-     * of the given UID match the given path.
-     * Set active UID once a match is found
-     */
-    matchInSurroundingItems(UID, path) {
-      // Get the surrounding items
-      const siblings = getSiblings(UID, this.childrenMap, this.children);
-      const children = getChildren(UID, this.childrenMap, this.children);
-      const parents = getParents(UID, this.childrenMap);
-
-      const matchingItem = [...children, ...siblings, ...parents]
-        .find(child => child.path === path);
-
-      // set the match as an active item
-      if (matchingItem) {
-        this.setActiveUID(matchingItem.uid);
-        return true;
-      }
-      return false;
-    },
-    /**
-     * Parse out the generic path given a
-     * specific overload path with a valid hash
-     */
-    getGenericPath(path) {
-      return path.slice(0, path.lastIndexOf('-'));
-    },
     /**
      * Updates the current focusIndex, based on where the activeUID is.
      * If not in the rendered items, we set it to 0.

src/views/DocumentationTopic.vue

@@ -62,7 +62,6 @@
                     :error-fetching="slotProps.errorFetching"
                     :api-changes="slotProps.apiChanges"
                     :references="topicProps.references"
-                    :symbolKind="topicProps.symbolKind"
                     :navigator-references="slotProps.references"
                     :scrollLockID="scrollLockID"
                     :render-filter-on-top="breakpoint !== BreakpointName.large"

tests/unit/components/DocumentationTopic.spec.js

@@ -108,6 +108,26 @@ const sampleCodeDownload = {
   },
 };
 
+const itemFoo = {
+  title: 'Foo',
+  url: '/documentation/foo',
+};
+
+const itemBar = {
+  title: 'Bar',
+  url: '/documentation/bar',
+};
+
+const hierarchyItems = [
+  'topic://foo',
+  'topic://bar',
+];
+
+const hierarchyItemsReferences = {
+  'topic://foo': itemFoo,
+  'topic://bar': itemBar,
+};
+
 const propsData = {
   abstract: [abstract],
   conformance: { constraints: [], availabilityPrefix: [] },
@@ -153,10 +173,7 @@ const propsData = {
     path: 'foo',
     query: {},
   },
-  hierarchyItems: [
-    'topic://foo',
-    'topic://bar',
-  ],
+  hierarchyItems: [],
 };
 
 const hasOtherDeclSection = {
@@ -319,17 +336,71 @@ describe('DocumentationTopic', () => {
   });
 
   it('renders a Hierarchy', () => {
+    wrapper.setProps({
+      references: hierarchyItemsReferences,
+      hierarchyItems,
+    });
     const hierarchy = wrapper.find(Hierarchy);
     expect(hierarchy.exists()).toBe(true);
     expect(hierarchy.props()).toEqual({
       currentTopicTitle: propsData.title,
-      parentTopicIdentifiers: propsData.hierarchyItems,
+      parentTopics: [itemFoo, itemBar],
       isSymbolBeta: false,
       isSymbolDeprecated: false,
       currentTopicTags: propsData.tags,
     });
   });
 
+  it('does not render `Hierarchy` in IDE', () => {
+    wrapper = shallowMount(DocumentationTopic, {
+      propsData: {
+        ...propsData,
+        references: hierarchyItemsReferences,
+        hierarchyItems,
+      },
+      provide: {
+        isTargetIDE: true,
+        store: mockStore,
+      },
+    });
+
+    const hierarchy = wrapper.find(Hierarchy);
+    expect(hierarchy.exists()).toBe(false);
+  });
+
+  it('renders `Hierarchy` without its immediate parent if its within overload group', () => {
+    wrapper.setProps({
+      references: hierarchyItemsReferences,
+      hierarchyItems,
+      primaryContentSections: [
+        ...propsData.primaryContentSections,
+        hasOtherDeclSection,
+      ],
+    });
+
+    let hierarchy = wrapper.find(Hierarchy);
+    // Don't hide immediate parent yet if has other declarations but different titles
+    expect(hierarchy.props()).toHaveProperty('parentTopics', [itemFoo, itemBar]);
+
+    // Hide immediate parent if has same title as parent and has other declarations
+    wrapper.setProps({ title: itemBar.title });
+    hierarchy = wrapper.find(Hierarchy);
+    expect(hierarchy.props()).toHaveProperty('parentTopics', [itemFoo]);
+  });
+
+  it('`Hierarchy` continues working, if a reference is missing', () => {
+    const errorSpy = jest.spyOn(console, 'error').mockReturnValue('');
+    wrapper.setProps({
+      references: { 'topic://foo': itemFoo }, // set without `Bar` reference data
+      hierarchyItems,
+    });
+
+    const hierarchy = wrapper.find(Hierarchy);
+    expect(hierarchy.exists()).toBe(true);
+    expect(errorSpy).toHaveBeenCalledTimes(1);
+    expect(errorSpy).toHaveBeenCalledWith('Reference for "topic://bar" is missing');
+  });
+
   it('does not render a Hierarchy if hierarchyItems is empty or enableMinimized is true', () => {
     wrapper.setProps({ hierarchyItems: [] });
     expect(wrapper.find(Hierarchy).exists()).toBe(false);