[r96943502] fix: address feedback

Author: marinaaisa

src/components/ContentNode/SectionTitle.vue

@@ -10,51 +10,57 @@
 
 <template>
   <component
-   :id="anchor ? anchor : null"
+   :id="id"
    :is="tag"
    class="section-title"
   >
     <a
-      v-if="anchor || href"
-      :href="`#${anchor || href}`"
+      v-if="anchor"
+      :href="`#${anchor}`"
       class="header-anchor"
       aria-label="hidden"
+      @click="handleFocusAndScroll(anchor)"
     >#</a>
     <slot />
   </component>
 </template>
 
 <script>
+import scrollToElement from 'docc-render/mixins/scrollToElement';
+
 export default {
   name: 'SectionTitle',
+  mixins: [scrollToElement],
+  data() {
+    return {
+      id: null,
+    };
+  },
   props: {
     anchor: {
       type: String,
       required: false,
     },
-    href: {
-      type: String,
-      required: false,
-    },
     tag: {
       type: String,
       default: () => 'h2',
     },
   },
+  mounted() {
+    if (!this.anchor) return;
+    const element = document.getElementById(`${this.anchor}`);
+    // if there is no element in the document that has an id,
+    // add it to this component
+    if (!element) {
+      this.id = this.anchor;
+    }
+  },
 };
 </script>
 
 <style scoped lang="scss">
 @import 'docc-render/styles/_core.scss';
 
-.section-title {
-  scroll-margin-top: $nav-height + 1rem;
-
-  @include breakpoint(small, $scope: nav) {
-    scroll-margin-top: $nav-height-small + 1rem;
-  }
-}
-
 .section-title:hover {
   .header-anchor {
     opacity: 1;
@@ -68,7 +74,7 @@ export default {
   opacity: 0;
   text-decoration: none;
 
-  &:hover {
+  &:hover, &:focus {
     opacity: 1;
   }
 }

src/components/DocumentationTopic/ContentTable.vue

@@ -15,7 +15,7 @@
     :title="title"
   >
     <div class="container">
-      <SectionTitle class="title" :href="anchor">{{ title }}</SectionTitle>
+      <SectionTitle class="title" :anchor="anchor">{{ title }}</SectionTitle>
       <slot />
     </div>
   </OnThisPageSection>

src/components/DocumentationTopic/ContentTableSection.vue

@@ -25,6 +25,7 @@
 
 <script>
 import SectionTitle from 'docc-render/components/ContentNode/SectionTitle.vue';
+import { anchorize } from 'docc-render/utils/strings';
 
 export default {
   name: 'ContentTableSection',
@@ -36,7 +37,7 @@ export default {
     },
   },
   computed: {
-    anchor: ({ title }) => title.replaceAll(' ', '-'),
+    anchor: ({ title }) => anchorize(title),
   },
 };
 </script>

src/components/DocumentationTopic/PrimaryContent/Declaration.vue

@@ -14,7 +14,7 @@
     class="declaration"
     title="Declaration"
   >
-    <SectionTitle href="declaration">Declaration</SectionTitle>
+    <SectionTitle anchor="declaration">Declaration</SectionTitle>
     <template v-if="hasModifiedChanges">
       <DeclarationDiff
         :class="[changeClasses, multipleLinesClass]"

src/components/DocumentationTopic/PrimaryContent/Parameters.vue

@@ -14,7 +14,7 @@
     class="parameters"
     title="Parameters"
   >
-    <SectionTitle href="parameters">Parameters</SectionTitle>
+    <SectionTitle anchor="parameters">Parameters</SectionTitle>
     <dl>
       <template v-for="param in parameters">
         <dt class="param-name" :key="`${param.name}:name`">

src/components/DocumentationTopic/PrimaryContent/PropertyTable.vue

@@ -10,7 +10,7 @@
 
 <template>
   <OnThisPageSection :anchor="anchor" :title="title">
-    <SectionTitle :href="anchor">{{ title }}</SectionTitle>
+    <SectionTitle :anchor="anchor">{{ title }}</SectionTitle>
     <ParametersTable :parameters="properties" :changes="propertyChanges" class="property-table">
       <template slot="symbol" slot-scope="{ name, type, content, changes, deprecated }">
         <div class="property-name" :class="{ deprecated: deprecated }">

src/components/DocumentationTopic/PrimaryContent/RestBody.vue

@@ -10,7 +10,7 @@
 
 <template>
   <OnThisPageSection :anchor="anchor" :title="title">
-    <SectionTitle :href="anchor">{{ title }}</SectionTitle>
+    <SectionTitle :anchor="anchor">{{ title }}</SectionTitle>
     <ParametersTable :parameters="[bodyParam]" :changes="bodyChanges" keyBy="key">
       <template slot="symbol" slot-scope="{ type, content, changes, name }">
         <PossiblyChangedType

src/components/DocumentationTopic/PrimaryContent/RestEndpoint.vue

@@ -10,7 +10,7 @@
 
 <template>
   <OnThisPageSection :anchor="anchor" :title="title">
-    <SectionTitle :href="anchor">{{ title }}</SectionTitle>
+    <SectionTitle :anchor="anchor">{{ title }}</SectionTitle>
     <DeclarationSource :tokens="tokens" />
   </OnThisPageSection>
 </template>

src/components/DocumentationTopic/PrimaryContent/RestParameters.vue

@@ -10,7 +10,7 @@
 
 <template>
   <OnThisPageSection :anchor="anchor" :title="title">
-    <SectionTitle :href="anchor">{{ title }}</SectionTitle>
+    <SectionTitle :anchor="anchor">{{ title }}</SectionTitle>
     <ParametersTable :parameters="parameters" :changes="parameterChanges">
       <template slot="symbol" slot-scope="{ name, type, content, changes, deprecated }">
         <div class="param-name" :class="{ deprecated: deprecated }">

src/components/DocumentationTopic/PrimaryContent/RestResponses.vue

@@ -10,7 +10,7 @@
 
 <template>
   <OnThisPageSection :anchor="anchor" :title="title">
-    <SectionTitle :href="anchor">{{ title }}</SectionTitle>
+    <SectionTitle :anchor="anchor">{{ title }}</SectionTitle>
     <ParametersTable :parameters="responses" :changes="propertyChanges" key-by="status">
       <template slot="symbol" slot-scope="{ status, type, reason, content, changes }">
         <div class="response-name">

src/components/DocumentationTopic/TopicsTable.vue

@@ -16,7 +16,7 @@
       :title="section.title"
     >
       <template v-if="wrapTitle" slot="title">
-        <SectionTitle tag="h3" :href="anchor" class="title">
+        <SectionTitle tag="h3" :anchor="anchor" class="title">
           <WordBreak>{{ section.title }}</WordBreak>
         </SectionTitle>
       </template>

src/components/Tutorial/NavigationBar.vue

@@ -174,8 +174,8 @@ export default {
   methods: {
     onSelectSection(path) {
       // Manually scroll to the section to work around a vue-router bug: https://github.com/vuejs/vue-router/issues/1668
-      const hash = `#${path.split('#')[1]}`;
-      this.scrollToElement(hash);
+      const hash = path.split('#')[1];
+      this.handleFocusAndScroll(hash);
     },
   },
 };

src/components/TutorialsOverview/TutorialsNavigationLink.vue

@@ -13,7 +13,7 @@
     class="tutorials-navigation-link"
     :class="{ active }"
     :to="fragment"
-    @click.native="handleFocus"
+    @click.native="handleFocusAndScroll(fragment.hash)"
   >
     <slot />
   </router-link>
@@ -42,23 +42,6 @@ export default {
     fragment: ({ text, $route }) => ({ hash: anchorize(text), query: $route.query }),
     text: ({ $slots: { default: [{ text: slotText }] } }) => slotText.trim(),
   },
-  methods: {
-    /**
-     * Always scroll to the element and focus it.
-     * This ensures that the next tab target is inside
-     * and that it is in view, even if the hash is in the url
-     * @returns {Promise<void>}
-     */
-    async handleFocus() {
-      const { hash } = this.fragment;
-      const element = document.getElementById(hash);
-      if (!element) return;
-      // Focus scrolls to the element
-      element.focus();
-      // but we need to compensate for the navigation height
-      await this.scrollToElement(`#${hash}`);
-    },
-  },
 };
 </script>
 

src/mixins/scrollToElement.js

@@ -27,5 +27,19 @@ export default {
       window.scrollBy(-offset.x, -offset.y);
       return element;
     },
+    /**
+     * Always scroll to the element and focus it.
+     * This ensures that the next tab target is inside
+     * and that it is in view, even if the hash is in the url
+     * @returns {Promise<void>}
+    */
+    async handleFocusAndScroll(hash) {
+      const element = document.getElementById(hash);
+      if (!element) return;
+      // Focus scrolls to the element
+      element.focus();
+      // but we need to compensate for the navigation height
+      await this.scrollToElement(`#${hash}`);
+    },
   },
 };

tests/unit/components/ContentNode/SectionTitle.spec.js

@@ -30,26 +30,32 @@ describe('SectionTitle', () => {
     expect(wrapper.is('h3')).toBe(true);
   });
 
-  it('renders a section title with a header anchor and an id on the wrapper', () => {
+  it('renders a section title with a header anchor and an id on the wrapper', async () => {
     const wrapper = shallowMount(SectionTitle, {
       propsData: {
         tag: 'h2',
         anchor: 'title',
       },
       slots: { default: 'Title' },
     });
+    await wrapper.vm.$nextTick();
     expect(wrapper.text()).toBe('# Title');
     expect(wrapper.attributes('id')).toBe('title');
     const headerAnchor = wrapper.find('.header-anchor');
     expect(headerAnchor.attributes('href')).toBe('#title');
     expect(headerAnchor.attributes('aria-label')).toBe('hidden');
   });
 
-  it('renders a section title with a header anchor and no id on the wrapper', () => {
+  it('renders a section title with a header anchor and no id on the wrapper if it already exists on the document', () => {
+    // create element with id outside the component
+    const div = document.createElement('div');
+    div.innerHTML = '<div id="title>';
+    document.body.appendChild(div);
+
     const wrapper = shallowMount(SectionTitle, {
       propsData: {
         tag: 'h2',
-        href: 'title',
+        anchor: 'title',
       },
       slots: { default: 'Title' },
     });
@@ -60,7 +66,7 @@ describe('SectionTitle', () => {
     expect(headerAnchor.attributes('aria-label')).toBe('hidden');
   });
 
-  it('does not render anchor if there is no anchor or href props', () => {
+  it('does not render anchor if there is no anchor', () => {
     const wrapper = shallowMount(SectionTitle);
     expect(wrapper.find('.header-anchor').exists()).toBe(false);
   });

tests/unit/components/DocumentationTopic/TopicsTable.spec.js

@@ -145,6 +145,6 @@ describe('TopicsTable', () => {
     expect(wordBreak.text()).toEqual(propsData.sections[0].title);
     expect(sectionTitle.exists()).toBe(true);
     expect(sectionTitle.attributes('tag')).toBe('h3');
-    expect(sectionTitle.attributes('href')).toBe('topics');
+    expect(sectionTitle.attributes('anchor')).toBe('topics');
   });
 });

tests/unit/components/Tutorial/NavigationBar.spec.js

@@ -19,7 +19,7 @@ import { flushPromises } from '../../../../test-utils';
 
 jest.mock('docc-render/mixins/scrollToElement');
 
-scrollToElement.methods.scrollToElement.mockResolvedValue(true);
+scrollToElement.methods.handleFocusAndScroll.mockResolvedValue(true);
 
 const {
   PrimaryDropdown,
@@ -241,16 +241,16 @@ describe('NavigationBar', () => {
       const dropdown = wrapper.find(MobileDropdown);
       dropdown.vm.$emit('select-section', 'path/to/item#section-foo');
       await flushPromises();
-      expect(scrollToElement.methods.scrollToElement).toHaveBeenCalledTimes(1);
-      expect(scrollToElement.methods.scrollToElement).toHaveBeenCalledWith('#section-foo');
+      expect(scrollToElement.methods.handleFocusAndScroll).toHaveBeenCalledTimes(1);
+      expect(scrollToElement.methods.handleFocusAndScroll).toHaveBeenCalledWith('section-foo');
     });
 
     it('scrolls to a section, on `@select-section`, on SecondaryDropdown', async () => {
       const dropdown = wrapper.find(SecondaryDropdown);
       dropdown.vm.$emit('select-section', 'path/to/item#section-foo');
       await flushPromises();
-      expect(scrollToElement.methods.scrollToElement).toHaveBeenCalledTimes(1);
-      expect(scrollToElement.methods.scrollToElement).toHaveBeenCalledWith('#section-foo');
+      expect(scrollToElement.methods.handleFocusAndScroll).toHaveBeenCalledTimes(1);
+      expect(scrollToElement.methods.handleFocusAndScroll).toHaveBeenCalledWith('section-foo');
     });
 
     it('renders a "Primary Dropdown" with chapters', () => {

tests/unit/components/TutorialsOverview/TutorialsNavigationLink.spec.js

@@ -17,7 +17,7 @@ import TutorialsNavigationLink
 import scrollToElement from 'docc-render/mixins/scrollToElement';
 
 jest.mock('docc-render/mixins/scrollToElement', () => ({
-  methods: { scrollToElement: jest.fn() },
+  methods: { handleFocusAndScroll: jest.fn() },
 }));
 
 describe('TutorialsNavigationLink', () => {
@@ -79,16 +79,9 @@ describe('TutorialsNavigationLink', () => {
   });
 
   it('focuses the element when clicked, used for AX', async () => {
-    const mockObject = { focus: jest.fn() };
-    const getElementSpy = jest.spyOn(document, 'getElementById').mockReturnValue(mockObject);
-    const spy = scrollToElement.methods.scrollToElement.mockReturnValueOnce(mockObject);
     const link = wrapper.find(RouterLinkStub);
     link.trigger('click');
     await wrapper.vm.$nextTick();
-    expect(scrollToElement.methods.scrollToElement).toHaveBeenCalledWith('#hello-world');
-    expect(mockObject.focus).toHaveBeenCalledTimes(1);
-    expect(getElementSpy).toHaveBeenCalledTimes(1);
-    spy.mockRestore();
-    getElementSpy.mockRestore();
+    expect(scrollToElement.methods.handleFocusAndScroll).toHaveBeenCalledWith('hello-world');
   });
 });