Commit 74891bf4 authored by Coung Ngo's avatar Coung Ngo

Make reviewer changes to expand epics in roadmap feature

Made changes as a result of MR reviewer comments
parent 7e0d2613
<script> <script>
import { GlIcon, GlTooltip } from '@gitlab/ui'; import { GlNewButton, GlIcon, GlTooltip } from '@gitlab/ui';
import { __, n__ } from '~/locale'; import { __, n__ } from '~/locale';
import eventHub from '../event_hub'; import eventHub from '../event_hub';
export default { export default {
components: { components: {
GlNewButton,
GlIcon, GlIcon,
GlTooltip, GlTooltip,
}, },
...@@ -27,7 +28,7 @@ export default { ...@@ -27,7 +28,7 @@ export default {
return this.currentGroupId !== this.epic.groupId; return this.currentGroupId !== this.epic.groupId;
}, },
isExpandIconHidden() { isExpandIconHidden() {
return this.epic.isChildEpic || !this.epic?.children?.edges?.length; return this.epic.isChildEpic || !this.epic.children?.edges?.length;
}, },
expandIconName() { expandIconName() {
return this.epic.isChildEpicShowing ? 'chevron-down' : 'chevron-right'; return this.epic.isChildEpicShowing ? 'chevron-down' : 'chevron-right';
...@@ -36,7 +37,7 @@ export default { ...@@ -36,7 +37,7 @@ export default {
return this.epic.isChildEpicShowing ? __('Collapse child epics') : __('Expand child epics'); return this.epic.isChildEpicShowing ? __('Collapse child epics') : __('Expand child epics');
}, },
childEpicsCount() { childEpicsCount() {
return this.epic.isChildEpic ? '-' : this.epic?.children?.edges?.length || 0; return this.epic.isChildEpic ? '-' : this.epic.children?.edges?.length || 0;
}, },
childEpicsCountText() { childEpicsCountText() {
return Number.isInteger(this.childEpicsCount) return Number.isInteger(this.childEpicsCount)
...@@ -54,14 +55,14 @@ export default { ...@@ -54,14 +55,14 @@ export default {
<template> <template>
<div class="epic-details-cell d-flex align-items-start p-2" data-qa-selector="epic_details_cell"> <div class="epic-details-cell d-flex align-items-start p-2" data-qa-selector="epic_details_cell">
<button <gl-new-button
:class="{ invisible: isExpandIconHidden }" :class="{ invisible: isExpandIconHidden }"
class="btn-link" variant="link"
:aria-label="expandIconLabel" :aria-label="expandIconLabel"
@click="toggleIsEpicExpanded" @click="toggleIsEpicExpanded"
> >
<gl-icon :name="expandIconName" class="text-secondary" aria-hidden="true" /> <gl-icon :name="expandIconName" class="text-secondary" aria-hidden="true" />
</button> </gl-new-button>
<div class="overflow-hidden flex-grow-1" :class="[epic.isChildEpic ? 'ml-4 mr-2' : 'mx-2']"> <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"> <a :href="epic.webUrl" :title="epic.title" class="epic-title d-block text-body bold">
{{ epic.title }} {{ epic.title }}
......
...@@ -172,7 +172,8 @@ export default { ...@@ -172,7 +172,8 @@ export default {
:id="generateKey(epic)" :id="generateKey(epic)"
:href="epic.webUrl" :href="epic.webUrl"
:style="timelineBarStyles(epic)" :style="timelineBarStyles(epic)"
:class="['epic-bar', 'rounded', { 'epic-bar-child-epic': epic.isChildEpic }]" :class="{ 'epic-bar-child-epic': epic.isChildEpic }"
class="epic-bar rounded"
> >
<div class="epic-bar-inner px-2 py-1" :style="timelineBarInnerStyle"> <div class="epic-bar-inner px-2 py-1" :style="timelineBarInnerStyle">
<p class="epic-bar-title text-nowrap text-truncate m-0">{{ timelineBarTitle }}</p> <p class="epic-bar-title text-nowrap text-truncate m-0">{{ timelineBarTitle }}</p>
......
...@@ -63,17 +63,13 @@ export default { ...@@ -63,17 +63,13 @@ export default {
}; };
}, },
displayedEpics() { displayedEpics() {
const displayedEpics = []; return this.epics.reduce((acc, epic) => {
acc.push(epic);
this.epics.forEach(epic => {
displayedEpics.push(epic);
if (epic.isChildEpicShowing) { if (epic.isChildEpicShowing) {
displayedEpics.push(...epic.children.edges); acc.push(...epic.children.edges);
} }
}); return acc;
}, []);
return displayedEpics;
}, },
}, },
mounted() { mounted() {
...@@ -88,7 +84,7 @@ export default { ...@@ -88,7 +84,7 @@ export default {
window.removeEventListener('resize', this.syncClientWidth); window.removeEventListener('resize', this.syncClientWidth);
}, },
methods: { methods: {
...mapActions(['setBufferSize']), ...mapActions(['setBufferSize', 'toggleExpandedEpic']),
initMounted() { initMounted() {
this.roadmapShellEl = this.$root.$el && this.$root.$el.firstChild; this.roadmapShellEl = this.$root.$el && this.$root.$el.firstChild;
this.setBufferSize(Math.ceil((window.innerHeight - this.$el.offsetTop) / EPIC_ITEM_HEIGHT)); this.setBufferSize(Math.ceil((window.innerHeight - this.$el.offsetTop) / EPIC_ITEM_HEIGHT));
...@@ -146,8 +142,7 @@ export default { ...@@ -146,8 +142,7 @@ export default {
}; };
}, },
toggleIsEpicExpanded(epicId) { toggleIsEpicExpanded(epicId) {
const epic = this.epics.find(e => e.id === epicId); this.toggleExpandedEpic(epicId);
epic.isChildEpicShowing = !epic.isChildEpicShowing;
}, },
generateKey, generateKey,
}, },
......
...@@ -84,7 +84,7 @@ export const receiveEpicsSuccess = ( ...@@ -84,7 +84,7 @@ export const receiveEpicsSuccess = (
formattedEpic.isChildEpicShowing = false; formattedEpic.isChildEpicShowing = false;
// Format child epics // Format child epics
if (formattedEpic?.children?.edges?.length > 0) { if (formattedEpic.children?.edges?.length > 0) {
formattedEpic.children.edges = formattedEpic.children.edges formattedEpic.children.edges = formattedEpic.children.edges
.map(epicUtils.flattenGroupProperty) .map(epicUtils.flattenGroupProperty)
.map(epicUtils.addIsChildEpicTrueProperty) .map(epicUtils.addIsChildEpicTrueProperty)
...@@ -212,7 +212,7 @@ export const extendTimeframe = ({ commit, state, getters }, { extendAs }) => { ...@@ -212,7 +212,7 @@ export const extendTimeframe = ({ commit, state, getters }, { extendAs }) => {
export const refreshEpicDates = ({ commit, state, getters }) => { export const refreshEpicDates = ({ commit, state, getters }) => {
const epics = state.epics.map(epic => { const epics = state.epics.map(epic => {
// Update child epic dates too // Update child epic dates too
if (epic?.children?.edges?.length > 0) { if (epic.children?.edges?.length > 0) {
epic.children.edges.map(childEpic => epic.children.edges.map(childEpic =>
roadmapItemUtils.processRoadmapItemDates( roadmapItemUtils.processRoadmapItemDates(
childEpic, childEpic,
...@@ -321,5 +321,8 @@ export const refreshMilestoneDates = ({ commit, state, getters }) => { ...@@ -321,5 +321,8 @@ export const refreshMilestoneDates = ({ commit, state, getters }) => {
export const setBufferSize = ({ commit }, bufferSize) => commit(types.SET_BUFFER_SIZE, bufferSize); export const setBufferSize = ({ commit }, bufferSize) => commit(types.SET_BUFFER_SIZE, bufferSize);
export const toggleExpandedEpic = ({ commit }, epicId) =>
commit(types.TOGGLE_EXPANDED_EPIC, epicId);
// prevent babel-plugin-rewire from generating an invalid default during karma tests // prevent babel-plugin-rewire from generating an invalid default during karma tests
export default () => {}; export default () => {};
...@@ -24,3 +24,5 @@ export const RECEIVE_MILESTONES_SUCCESS = 'RECEIVE_MILESTONES_SUCCESS'; ...@@ -24,3 +24,5 @@ export const RECEIVE_MILESTONES_SUCCESS = 'RECEIVE_MILESTONES_SUCCESS';
export const RECEIVE_MILESTONES_FAILURE = 'RECEIVE_MILESTONES_FAILURE'; export const RECEIVE_MILESTONES_FAILURE = 'RECEIVE_MILESTONES_FAILURE';
export const SET_BUFFER_SIZE = 'SET_BUFFER_SIZE'; export const SET_BUFFER_SIZE = 'SET_BUFFER_SIZE';
export const TOGGLE_EXPANDED_EPIC = 'TOGGLE_EXPANDED_EPIC';
...@@ -76,4 +76,9 @@ export default { ...@@ -76,4 +76,9 @@ export default {
[types.SET_BUFFER_SIZE](state, bufferSize) { [types.SET_BUFFER_SIZE](state, bufferSize) {
state.bufferSize = bufferSize; state.bufferSize = bufferSize;
}, },
[types.TOGGLE_EXPANDED_EPIC](state, epicId) {
const epic = state.epics.find(e => e.id === epicId);
epic.isChildEpicShowing = !epic.isChildEpicShowing;
},
}; };
import { GlIcon, GlTooltip } from '@gitlab/ui'; import { GlNewButton, GlIcon, GlTooltip } from '@gitlab/ui';
import { shallowMount } from '@vue/test-utils'; import { shallowMount } from '@vue/test-utils';
import EpicItemDetails from 'ee/roadmap/components/epic_item_details.vue'; import EpicItemDetails from 'ee/roadmap/components/epic_item_details.vue';
import eventHub from 'ee/roadmap/event_hub'; import eventHub from 'ee/roadmap/event_hub';
...@@ -27,7 +27,7 @@ const getTitle = wrapper => wrapper.find('.epic-title'); ...@@ -27,7 +27,7 @@ const getTitle = wrapper => wrapper.find('.epic-title');
const getGroupName = wrapper => wrapper.find('.epic-group'); const getGroupName = wrapper => wrapper.find('.epic-group');
const getExpandIconDiv = wrapper => wrapper.find('.btn-link'); const getExpandIconButton = wrapper => wrapper.find(GlNewButton);
const getChildEpicsCount = wrapper => wrapper.find({ ref: 'childEpicsCount' }); const getChildEpicsCount = wrapper => wrapper.find({ ref: 'childEpicsCount' });
...@@ -111,7 +111,7 @@ describe('EpicItemDetails', () => { ...@@ -111,7 +111,7 @@ describe('EpicItemDetails', () => {
it('is hidden when epic has no child epics', () => { it('is hidden when epic has no child epics', () => {
wrapper = createComponent(); wrapper = createComponent();
expect(getExpandIconDiv(wrapper).classes()).toContain('invisible'); expect(getExpandIconButton(wrapper).classes()).toContain('invisible');
}); });
it('is shown when epic has child epics', () => { it('is shown when epic has child epics', () => {
...@@ -123,7 +123,7 @@ describe('EpicItemDetails', () => { ...@@ -123,7 +123,7 @@ describe('EpicItemDetails', () => {
}; };
wrapper = createComponent(epic); wrapper = createComponent(epic);
expect(getExpandIconDiv(wrapper).classes()).not.toContain('invisible'); expect(getExpandIconButton(wrapper).classes()).not.toContain('invisible');
}); });
it('shows "chevron-right" icon when child epics are not expanded', () => { it('shows "chevron-right" icon when child epics are not expanded', () => {
...@@ -145,7 +145,7 @@ describe('EpicItemDetails', () => { ...@@ -145,7 +145,7 @@ describe('EpicItemDetails', () => {
it('has "Expand child epics" label when child epics are not expanded', () => { it('has "Expand child epics" label when child epics are not expanded', () => {
wrapper = createComponent(); wrapper = createComponent();
expect(getExpandIconDiv(wrapper).attributes('aria-label')).toBe('Expand child epics'); expect(getExpandIconButton(wrapper).attributes('aria-label')).toBe('Expand child epics');
}); });
it('has "Collapse child epics" label when child epics are expanded', () => { it('has "Collapse child epics" label when child epics are expanded', () => {
...@@ -155,7 +155,7 @@ describe('EpicItemDetails', () => { ...@@ -155,7 +155,7 @@ describe('EpicItemDetails', () => {
}; };
wrapper = createComponent(epic); wrapper = createComponent(epic);
expect(getExpandIconDiv(wrapper).attributes('aria-label')).toBe('Collapse child epics'); expect(getExpandIconButton(wrapper).attributes('aria-label')).toBe('Collapse child epics');
}); });
it('emits toggleIsEpicExpanded event when clicked', () => { it('emits toggleIsEpicExpanded event when clicked', () => {
...@@ -171,7 +171,7 @@ describe('EpicItemDetails', () => { ...@@ -171,7 +171,7 @@ describe('EpicItemDetails', () => {
}; };
wrapper = createComponent(epic); wrapper = createComponent(epic);
getExpandIconDiv(wrapper).trigger('click'); getExpandIconButton(wrapper).vm.$emit('click');
expect(eventHub.$emit).toHaveBeenCalledWith('toggleIsEpicExpanded', id); expect(eventHub.$emit).toHaveBeenCalledWith('toggleIsEpicExpanded', id);
}); });
...@@ -183,7 +183,7 @@ describe('EpicItemDetails', () => { ...@@ -183,7 +183,7 @@ describe('EpicItemDetails', () => {
}; };
wrapper = createComponent(epic); wrapper = createComponent(epic);
expect(getExpandIconDiv(wrapper).classes()).toContain('invisible'); expect(getExpandIconButton(wrapper).classes()).toContain('invisible');
}); });
}); });
......
...@@ -614,4 +614,17 @@ describe('Roadmap Vuex Actions', () => { ...@@ -614,4 +614,17 @@ describe('Roadmap Vuex Actions', () => {
); );
}); });
}); });
describe('toggleExpandedEpic', () => {
it('should perform TOGGLE_EXPANDED_EPIC mutation with epic ID payload', done => {
testAction(
actions.toggleExpandedEpic,
10,
state,
[{ type: types.TOGGLE_EXPANDED_EPIC, payload: 10 }],
[],
done,
);
});
});
}); });
...@@ -5,6 +5,8 @@ import defaultState from 'ee/roadmap/store/state'; ...@@ -5,6 +5,8 @@ import defaultState from 'ee/roadmap/store/state';
import { mockGroupId, basePath, epicsPath, mockSortedBy } from '../mock_data'; import { mockGroupId, basePath, epicsPath, mockSortedBy } from '../mock_data';
const getEpic = (epicId, epics) => epics.find(e => e.id === epicId);
describe('Roadmap Store Mutations', () => { describe('Roadmap Store Mutations', () => {
let state; let state;
...@@ -195,4 +197,30 @@ describe('Roadmap Store Mutations', () => { ...@@ -195,4 +197,30 @@ describe('Roadmap Store Mutations', () => {
expect(state.bufferSize).toBe(bufferSize); expect(state.bufferSize).toBe(bufferSize);
}); });
}); });
describe('TOGGLE_EXPANDED_EPIC', () => {
it('should toggle collapsed epic to an expanded epic', () => {
const epicId = 1;
const epics = [
{ id: 1, title: 'Collapsed epic', isChildEpicShowing: false },
{ id: 2, title: 'Expanded epic', isChildEpicShowing: true },
];
mutations[types.TOGGLE_EXPANDED_EPIC]({ ...state, epics }, epicId);
expect(getEpic(epicId, epics).isChildEpicShowing).toBe(true);
});
it('should toggle expanded epic to a collapsed epic', () => {
const epicId = 2;
const epics = [
{ id: 1, title: 'Collapsed epic', isChildEpicShowing: false },
{ id: 2, title: 'Expanded epic', isChildEpicShowing: true },
];
mutations[types.TOGGLE_EXPANDED_EPIC]({ ...state, epics }, epicId);
expect(getEpic(epicId, epics).isChildEpicShowing).toBe(false);
});
});
}); });
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment