Make reviewer changes to expand epics in roadmap feature

Made changes as a result of MR reviewer comments

doc/user/group/roadmap/index.md

@@ -17,7 +17,7 @@ When you hover over an epic bar, a popover appears with its title, start and due
 completed.
 
 You can expand epics that contain child epics to show their child epics in the roadmap.
-You can click the chevron **{angle-down}** next to the epic title to expand and collapse the child epics.
+You can click the chevron **{chevron-down}** next to the epic title to expand and collapse the child epics.
 
 ![roadmap view](img/roadmap_view_v12_9.png)
 

ee/app/assets/javascripts/roadmap/components/epic_item_details.vue

 <script>
 import { GlIcon, GlTooltip } from '@gitlab/ui';
-import { __ } from '~/locale';
+import { __, n__ } from '~/locale';
 import eventHub from '../event_hub';
 
 export default {
@@ -30,14 +30,19 @@ export default {
       return this.epic.isChildEpic || !this.epic?.children?.edges?.length;
     },
     expandIconName() {
-      return this.epic.isChildEpicShowing ? 'angle-down' : 'angle-right';
+      return this.epic.isChildEpicShowing ? 'chevron-down' : 'chevron-right';
     },
     expandIconLabel() {
-      return this.epic.isChildEpicShowing ? __('Collapse') : __('Expand');
+      return this.epic.isChildEpicShowing ? __('Collapse child epics') : __('Expand child epics');
     },
     childEpicsCount() {
       return this.epic.isChildEpic ? '-' : this.epic?.children?.edges?.length || 0;
     },
+    childEpicsCountText() {
+      return Number.isInteger(this.childEpicsCount)
+        ? n__(`%d child epic`, `%d child epics`, this.childEpicsCount)
+        : '';
+    },
   },
   methods: {
     toggleIsEpicExpanded() {
@@ -48,41 +53,37 @@ export default {
 </script>
 
 <template>
-  <div class="epic-details-cell d-flex p-2" data-qa-selector="epic_details_cell">
-    <div
+  <div class="epic-details-cell d-flex align-items-start p-2" data-qa-selector="epic_details_cell">
+    <button
       :class="{ invisible: isExpandIconHidden }"
-      class="epic-details-cell-expand-icon cursor-pointer"
-      tabindex="0"
+      class="btn-link"
+      :aria-label="expandIconLabel"
       @click="toggleIsEpicExpanded"
-      @keydown.enter="toggleIsEpicExpanded"
     >
-      <gl-icon
-        :name="expandIconName"
-        class="text-secondary width"
-        :aria-label="expandIconLabel"
-        :size="12"
-      />
-    </div>
+      <gl-icon :name="expandIconName" class="text-secondary" aria-hidden="true" />
+    </button>
     <div class="overflow-hidden flex-grow-1" :class="[epic.isChildEpic ? 'ml-4 mr-2' : 'mx-2']">
       <a :href="epic.webUrl" :title="epic.title" class="epic-title d-block text-body bold">
         {{ epic.title }}
       </a>
-      <div class="epic-group-timeframe text-secondary">
-        <span v-if="isEpicGroupDifferent" :title="epic.groupFullName" class="epic-group">
-          {{ epic.groupName }} &middot;
-        </span>
-        <span class="epic-timeframe" :title="timeframeString">{{ timeframeString }}</span>
+      <div class="epic-group-timeframe d-flex text-secondary">
+        <p v-if="isEpicGroupDifferent" :title="epic.groupFullName" class="epic-group">
+          {{ epic.groupName }}
+        </p>
+        <span class="mx-1" aria-hidden="true">&middot;</span>
+        <p class="epic-timeframe" :title="timeframeString">{{ timeframeString }}</p>
       </div>
     </div>
     <div
       ref="childEpicsCount"
-      :class="['text-secondary', 'text-nowrap', { invisible: epic.isChildEpic }]"
+      :class="{ invisible: epic.isChildEpic }"
+      class="d-flex text-secondary text-nowrap"
     >
-      <gl-icon name="epic" class="align-text-bottom" />
-      {{ childEpicsCount }}
+      <gl-icon name="epic" class="align-text-bottom mr-1" aria-hidden="true" />
+      <p class="m-0" :aria-label="childEpicsCountText">{{ childEpicsCount }}</p>
     </div>
     <gl-tooltip v-if="!epic.isChildEpic" :target="() => $refs.childEpicsCount">
-      {{ n__(`%d child epic`, `%d child epics`, childEpicsCount) }}
+      {{ childEpicsCountText }}
     </gl-tooltip>
   </div>
 </template>

ee/app/assets/javascripts/roadmap/components/epic_item_timeline.vue

@@ -142,6 +142,11 @@ export default {
           )
         : 0;
     },
+    epicWeightPercentageText() {
+      return sprintf(__(`%{percentage}%% weight completed`), {
+        percentage: this.epicWeightPercentage,
+      });
+    },
     popoverWeightText() {
       if (this.epic.descendantWeightSum) {
         return sprintf(__('%{completedWeight} of %{totalWeight} weight completed'), {
@@ -170,19 +175,18 @@ export default {
         :class="['epic-bar', 'rounded', { 'epic-bar-child-epic': epic.isChildEpic }]"
       >
         <div class="epic-bar-inner px-2 py-1" :style="timelineBarInnerStyle">
-          <p class="epic-bar-title text-nowrap text-truncate mb-0">
-            {{ timelineBarTitle }}
-          </p>
+          <p class="epic-bar-title text-nowrap text-truncate m-0">{{ timelineBarTitle }}</p>
 
           <div v-if="!isTimelineBarSmall" class="d-flex align-items-center">
             <gl-progress-bar
               class="epic-bar-progress flex-grow-1 mr-1"
               :value="epicWeightPercentage"
+              aria-hidden="true"
             />
-            <span class="gl-font-size-small d-flex align-items-center text-nowrap">
+            <div class="gl-font-size-small d-flex align-items-center text-nowrap">
               <icon class="append-right-2" :size="12" name="weight" />
-              {{ epicWeightPercentage }}%
-            </span>
+              <p class="m-0" :aria-label="epicWeightPercentageText">{{ epicWeightPercentage }}%</p>
+            </div>
           </div>
         </div>
       </a>

ee/app/assets/javascripts/roadmap/store/actions.js

@@ -59,10 +59,9 @@ export const fetchGroupEpics = (
       variables,
     })
     .then(({ data }) => {
-      const { group } = data;
       const edges = epicIid
-        ? (group.epic && group.epic.children.edges) || []
-        : (group.epics && group.epics.edges) || [];
+        ? data?.group?.epic?.children?.edges || []
+        : data?.group?.epics?.edges || [];
 
       return epicUtils.extractGroupEpics(edges);
     });
@@ -89,9 +88,9 @@ export const receiveEpicsSuccess = (
       formattedEpic.children.edges = formattedEpic.children.edges
         .map(epicUtils.flattenGroupProperty)
         .map(epicUtils.addIsChildEpicTrueProperty)
-        .map(e =>
+        .map(childEpic =>
           roadmapItemUtils.formatRoadmapItemDetails(
-            e,
+            childEpic,
             getters.timeframeStartDate,
             getters.timeframeEndDate,
           ),
@@ -214,9 +213,9 @@ export const refreshEpicDates = ({ commit, state, getters }) => {
   const epics = state.epics.map(epic => {
     // Update child epic dates too
     if (epic?.children?.edges?.length > 0) {
-      epic.children.edges.map(e =>
+      epic.children.edges.map(childEpic =>
         roadmapItemUtils.processRoadmapItemDates(
-          e,
+          childEpic,
           getters.timeframeStartDate,
           getters.timeframeEndDate,
         ),

ee/app/assets/javascripts/roadmap/utils/epic_utils.js

@@ -7,7 +7,7 @@ export const gqClient = createGqClient(
   },
 );
 
-export const flattenGroupProperty = ({ node, epicNode = node }) => ({
+export const flattenGroupProperty = ({ node: epicNode }) => ({
   ...epicNode,
   // We can get rid of below two lines
   // by updating `epic_item_details.vue`

ee/app/assets/stylesheets/pages/roadmap.scss

@@ -305,19 +305,11 @@ html.group-epics-roadmap-html {
   }
 }
 
-.epic-details-cell-expand-icon {
-  height: 20px;
-}
-
 .epic-title,
 .epic-group-timeframe {
   @include text-truncate;
 }
 
-.epic-group:hover {
-  cursor: pointer;
-}
-
 .epic-timeline-cell {
   position: relative;
   width: $timeline-cell-width;

ee/spec/javascripts/roadmap/components/epic_item_details_spec.js

@@ -27,7 +27,7 @@ const getTitle = wrapper => wrapper.find('.epic-title');
 
 const getGroupName = wrapper => wrapper.find('.epic-group');
 
-const getExpandIconDiv = wrapper => wrapper.find('.epic-details-cell-expand-icon');
+const getExpandIconDiv = wrapper => wrapper.find('.btn-link');
 
 const getChildEpicsCount = wrapper => wrapper.find({ ref: 'childEpicsCount' });
 
@@ -36,6 +36,7 @@ describe('EpicItemDetails', () => {
 
   afterEach(() => {
     wrapper.destroy();
+    wrapper = null;
   });
 
   describe('epic title', () => {
@@ -107,13 +108,13 @@ describe('EpicItemDetails', () => {
 
   describe('epic', () => {
     describe('expand icon', () => {
-      it('is hidden when epic has no sub-epics', () => {
+      it('is hidden when epic has no child epics', () => {
         wrapper = createComponent();
 
         expect(getExpandIconDiv(wrapper).classes()).toContain('invisible');
       });
 
-      it('is shown when epic has sub-epics', () => {
+      it('is shown when epic has child epics', () => {
         const epic = {
           ...mockFormattedEpic,
           children: {
@@ -125,36 +126,36 @@ describe('EpicItemDetails', () => {
         expect(getExpandIconDiv(wrapper).classes()).not.toContain('invisible');
       });
 
-      it('shows "angle-right" icon when sub-epics are not expanded', () => {
+      it('shows "chevron-right" icon when child epics are not expanded', () => {
         wrapper = createComponent();
 
-        expect(wrapper.find(GlIcon).attributes('name')).toBe('angle-right');
+        expect(wrapper.find(GlIcon).attributes('name')).toBe('chevron-right');
       });
 
-      it('shows "angle-down" icon when sub-epics are expanded', () => {
+      it('shows "chevron-down" icon when child epics are expanded', () => {
         const epic = {
           ...mockFormattedEpic,
           isChildEpicShowing: true,
         };
         wrapper = createComponent(epic);
 
-        expect(wrapper.find(GlIcon).attributes('name')).toBe('angle-down');
+        expect(wrapper.find(GlIcon).attributes('name')).toBe('chevron-down');
       });
 
-      it('has "Expand" label when sub-epics are not expanded', () => {
+      it('has "Expand child epics" label when child epics are not expanded', () => {
         wrapper = createComponent();
 
-        expect(wrapper.find(GlIcon).attributes('aria-label')).toBe('Expand');
+        expect(getExpandIconDiv(wrapper).attributes('aria-label')).toBe('Expand child epics');
       });
 
-      it('has "Collapse" label when sub-epics are expanded', () => {
+      it('has "Collapse child epics" label when child epics are expanded', () => {
         const epic = {
           ...mockFormattedEpic,
           isChildEpicShowing: true,
         };
         wrapper = createComponent(epic);
 
-        expect(wrapper.find(GlIcon).attributes('aria-label')).toBe('Collapse');
+        expect(getExpandIconDiv(wrapper).attributes('aria-label')).toBe('Collapse child epics');
       });
 
       it('emits toggleIsEpicExpanded event when clicked', () => {
@@ -175,7 +176,7 @@ describe('EpicItemDetails', () => {
         expect(eventHub.$emit).toHaveBeenCalledWith('toggleIsEpicExpanded', id);
       });
 
-      it('is hidden when it is sub-epic', () => {
+      it('is hidden when it is child epic', () => {
         const epic = {
           ...mockFormattedEpic,
           isChildEpic: true,
@@ -186,8 +187,8 @@ describe('EpicItemDetails', () => {
       });
     });
 
-    describe('sub-epics count', () => {
-      it('shows the correct count of sub-epics', () => {
+    describe('child epics count', () => {
+      it('shows the correct count of child epics', () => {
         const epic = {
           ...mockFormattedEpic,
           children: {
@@ -199,7 +200,7 @@ describe('EpicItemDetails', () => {
         expect(getChildEpicsCount(wrapper).text()).toBe('2');
       });
 
-      it('shows the count as 0 when there are no sub-epics', () => {
+      it('shows the count as 0 when there are no child epics', () => {
         wrapper = createComponent();
 
         expect(getChildEpicsCount(wrapper).text()).toBe('0');
@@ -217,7 +218,7 @@ describe('EpicItemDetails', () => {
         expect(wrapper.find(GlTooltip).text()).toBe('1 child epic');
       });
 
-      it('is hidden when it is a sub-epic', () => {
+      it('is hidden when it is a child epic', () => {
         const epic = {
           ...mockFormattedEpic,
           isChildEpic: true,

ee/spec/javascripts/roadmap/components/epic_item_timeline_spec.js

@@ -33,30 +33,27 @@ describe('EpicItemTimelineComponent', () => {
 
   afterEach(() => {
     wrapper.destroy();
+    wrapper = null;
   });
 
   describe('epic bar', () => {
-    it('shows the title', () => {
+    beforeEach(() => {
       wrapper = createComponent();
+    });
 
+    it('shows the title', () => {
       expect(getEpicBar(wrapper).text()).toContain(mockFormattedEpic.title);
     });
 
     it('shows the progress bar with correct value', () => {
-      wrapper = createComponent();
-
       expect(wrapper.find(GlProgressBar).attributes('value')).toBe('60');
     });
 
     it('shows the percentage', () => {
-      wrapper = createComponent();
-
       expect(getEpicBar(wrapper).text()).toContain('60%');
     });
 
     it('contains a link to the epic', () => {
-      wrapper = createComponent();
-
       expect(getEpicBar(wrapper).attributes('href')).toBe(mockFormattedEpic.webUrl);
     });
   });

ee/spec/javascripts/roadmap/components/epics_list_section_spec.js

 import { shallowMount, createLocalVue } from '@vue/test-utils';
-import Vue from 'vue';
 import VirtualList from 'vue-virtual-scroll-list';
 import epicsListSectionComponent from 'ee/roadmap/components/epics_list_section.vue';
 import EpicItem from 'ee/roadmap/components/epic_item.vue';
@@ -78,6 +77,7 @@ describe('EpicsListSectionComponent', () => {
 
   afterEach(() => {
     wrapper.destroy();
+    wrapper = null;
   });
 
   describe('data', () => {
@@ -265,7 +265,7 @@ describe('EpicsListSectionComponent', () => {
     });
   });
 
-  it('expands to show sub-epics when epic is toggled', done => {
+  it('expands to show child epics when epic is toggled', done => {
     const epic = mockEpics[0];
     wrapper = createComponent();
 
@@ -273,7 +273,8 @@ describe('EpicsListSectionComponent', () => {
 
     wrapper.vm.toggleIsEpicExpanded(epic.id);
 
-    Vue.nextTick()
+    wrapper.vm
+      .$nextTick()
       .then(() => {
         const expected = mockEpics.length + epic.children.edges.length;
 

ee/spec/javascripts/roadmap/utils/epic_utils_spec.js

@@ -19,15 +19,26 @@ describe('extractGroupEpics', () => {
 });
 
 describe('addIsChildEpicTrueProperty', () => {
-  it('adds `isChildEpic` property with value `true`', () => {
+  const title = 'Lorem ipsum dolar sit';
+  const description = 'Beatae suscipit dolorum nihil quidem est accusamus';
   const obj = {
-      title: 'Lorem ipsum dolar sit',
+    title,
+    description,
   };
+  let newObj;
 
-    const newObj = epicUtils.addIsChildEpicTrueProperty(obj);
+  beforeEach(() => {
+    newObj = epicUtils.addIsChildEpicTrueProperty(obj);
+  });
 
+  it('adds `isChildEpic` property with value `true`', () => {
     expect(newObj.isChildEpic).toBe(true);
   });
+
+  it('has original properties in returned object', () => {
+    expect(newObj.title).toBe(title);
+    expect(newObj.description).toBe(description);
+  });
 });
 
 describe('generateKey', () => {

locale/gitlab.pot

@@ -395,6 +395,9 @@ msgstr ""
 msgid "%{openedIssues} open, %{closedIssues} closed"
 msgstr ""
 
+msgid "%{percentage}%% weight completed"
+msgstr ""
+
 msgid "%{percent}%% complete"
 msgstr ""
 
@@ -4988,6 +4991,9 @@ msgstr ""
 msgid "Collapse approvers"
 msgstr ""
 
+msgid "Collapse child epics"
+msgstr ""
+
 msgid "Collapse sidebar"
 msgstr ""
 
@@ -8321,6 +8327,9 @@ msgstr ""
 msgid "Expand approvers"
 msgstr ""
 
+msgid "Expand child epics"
+msgstr ""
+
 msgid "Expand down"
 msgstr ""