diff --git a/app/assets/javascripts/notes/components/discussion_counter.vue b/app/assets/javascripts/notes/components/discussion_counter.vue index 577612de06a16167928ee7e6a4b0bf19b9bd4c2b..c28ac94b3ed1aac99f1fb81454dd0f63b1ac52be 100644 --- a/app/assets/javascripts/notes/components/discussion_counter.vue +++ b/app/assets/javascripts/notes/components/discussion_counter.vue @@ -1,5 +1,5 @@ <script> -import { mapGetters } from 'vuex'; +import { mapGetters, mapActions } from 'vuex'; import { GlTooltipDirective } from '@gitlab/ui'; import Icon from '~/vue_shared/components/icon.vue'; import discussionNavigation from '../mixins/discussion_navigation'; @@ -18,13 +18,11 @@ export default { 'getNoteableData', 'resolvableDiscussionsCount', 'unresolvedDiscussionsCount', + 'discussions', ]), isLoggedIn() { return this.getUserData.id; }, - hasNextButton() { - return this.isLoggedIn && !this.allResolved; - }, allResolved() { return this.unresolvedDiscussionsCount === 0; }, @@ -34,6 +32,21 @@ export default { resolvedDiscussionsCount() { return this.resolvableDiscussionsCount - this.unresolvedDiscussionsCount; }, + toggeableDiscussions() { + return this.discussions.filter(discussion => !discussion.individual_note); + }, + allExpanded() { + return this.toggeableDiscussions.every(discussion => discussion.expanded); + }, + }, + methods: { + ...mapActions(['setExpandDiscussions']), + handleExpandDiscussions() { + this.setExpandDiscussions({ + discussionIds: this.toggeableDiscussions.map(discussion => discussion.id), + expanded: !this.allExpanded, + }); + }, }, }; </script> @@ -44,8 +57,8 @@ export default { ref="discussionCounter" class="line-resolve-all-container full-width-mobile" > - <div class="full-width-mobile d-flex d-sm-block"> - <div :class="{ 'has-next-btn': hasNextButton }" class="line-resolve-all"> + <div class="full-width-mobile d-flex d-sm-flex"> + <div class="line-resolve-all"> <span :class="{ 'is-active': allResolved }" class="line-resolve-btn is-disabled" @@ -75,7 +88,7 @@ export default { <div v-if="isLoggedIn && !allResolved" class="btn-group btn-group-sm" role="group"> <button v-gl-tooltip - title="Jump to next unresolved thread" + :title="__('Jump to next unresolved thread')" class="btn btn-default discussion-next-btn" data-track-event="click_button" data-track-label="mr_next_unresolved_thread" @@ -85,6 +98,16 @@ export default { <icon name="comment-next" /> </button> </div> + <div v-if="isLoggedIn" class="btn-group btn-group-sm" role="group"> + <button + v-gl-tooltip + :title="__('Toggle all threads')" + class="btn btn-default toggle-all-discussions-btn" + @click="handleExpandDiscussions" + > + <icon :name="allExpanded ? 'angle-up' : 'angle-down'" /> + </button> + </div> </div> </div> </template> diff --git a/app/assets/javascripts/notes/stores/actions.js b/app/assets/javascripts/notes/stores/actions.js index 2e6719bb4fbe090639d4f7d51103f95799e69b94..accc37121d0d709c55e9d39c0d872b4b6612d061 100644 --- a/app/assets/javascripts/notes/stores/actions.js +++ b/app/assets/javascripts/notes/stores/actions.js @@ -46,6 +46,10 @@ export const setNotesFetchedState = ({ commit }, state) => export const toggleDiscussion = ({ commit }, data) => commit(types.TOGGLE_DISCUSSION, data); +export const setExpandDiscussions = ({ commit }, { discussionIds, expanded }) => { + commit(types.SET_EXPAND_DISCUSSIONS, { discussionIds, expanded }); +}; + export const fetchDiscussions = ({ commit, dispatch }, { path, filter, persistFilter }) => { const config = filter !== undefined @@ -54,6 +58,7 @@ export const fetchDiscussions = ({ commit, dispatch }, { path, filter, persistFi return axios.get(path, config).then(({ data }) => { commit(types.SET_INITIAL_DISCUSSIONS, data); + dispatch('updateResolvableDiscussionsCounts'); }); }; diff --git a/app/assets/javascripts/notes/stores/mutation_types.js b/app/assets/javascripts/notes/stores/mutation_types.js index 6554aee0d5b3ed69fe4008dbd45da7c13f0cab98..0cc59f9150cf7866758ae9401045944636b98f11 100644 --- a/app/assets/javascripts/notes/stores/mutation_types.js +++ b/app/assets/javascripts/notes/stores/mutation_types.js @@ -24,6 +24,7 @@ export const REMOVE_CONVERTED_DISCUSSION = 'REMOVE_CONVERTED_DISCUSSION'; export const COLLAPSE_DISCUSSION = 'COLLAPSE_DISCUSSION'; export const EXPAND_DISCUSSION = 'EXPAND_DISCUSSION'; export const TOGGLE_DISCUSSION = 'TOGGLE_DISCUSSION'; +export const SET_EXPAND_DISCUSSIONS = 'SET_EXPAND_DISCUSSIONS'; export const UPDATE_RESOLVABLE_DISCUSSIONS_COUNTS = 'UPDATE_RESOLVABLE_DISCUSSIONS_COUNTS'; export const SET_CURRENT_DISCUSSION_ID = 'SET_CURRENT_DISCUSSION_ID'; diff --git a/app/assets/javascripts/notes/stores/mutations.js b/app/assets/javascripts/notes/stores/mutations.js index c23ef93c0560647fae3d07f5e940a49ef0829757..68bf83945082c06c36aaa438262fe3ef25bc8707 100644 --- a/app/assets/javascripts/notes/stores/mutations.js +++ b/app/assets/javascripts/notes/stores/mutations.js @@ -190,6 +190,15 @@ export default { }); }, + [types.SET_EXPAND_DISCUSSIONS](state, { discussionIds, expanded }) { + if (discussionIds?.length) { + discussionIds.forEach(discussionId => { + const discussion = utils.findNoteObjectById(state.discussions, discussionId); + Object.assign(discussion, { expanded }); + }); + } + }, + [types.UPDATE_NOTE](state, note) { const noteObj = utils.findNoteObjectById(state.discussions, note.discussion_id); diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index aaecbd6ff0083e7c8eb5933863807e0fada08b65..f2b8433a99573889601abcc5dff97752deb5d614 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -842,11 +842,11 @@ $note-form-margin-left: 72px; white-space: nowrap; } - .btn-group { - margin-left: -4px; + .discussion-next-btn { + border-radius: 0; } - .discussion-next-btn { + .toggle-all-discussions-btn { border-top-left-radius: 0; border-bottom-left-radius: 0; } @@ -859,7 +859,6 @@ $note-form-margin-left: 72px; } &.discussion-create-issue-btn { - margin-left: -4px; border-radius: 0; border-right: 0; @@ -873,6 +872,10 @@ $note-form-margin-left: 72px; } } } + + &.discussion-next-btn { + border-right: 0; + } } } @@ -884,12 +887,9 @@ $note-form-margin-left: 72px; border: 1px solid $border-color; border-radius: $border-radius-default; font-size: $gl-btn-small-font-size; - - &.has-next-btn { - border-top-right-radius: 0; - border-bottom-right-radius: 0; - border-right: 0; - } + border-top-right-radius: 0; + border-bottom-right-radius: 0; + border-right: 0; .line-resolve-btn { margin-right: 5px; diff --git a/changelogs/unreleased/feat-add-toggle-all-discussions-button.yml b/changelogs/unreleased/feat-add-toggle-all-discussions-button.yml new file mode 100644 index 0000000000000000000000000000000000000000..803e517d2deeaf59300909caf25c198d9007a7b6 --- /dev/null +++ b/changelogs/unreleased/feat-add-toggle-all-discussions-button.yml @@ -0,0 +1,5 @@ +--- +title: Add toggle all discussions button to MRs +merge_request: 24670 +author: Martin Hobert & Diego Louzán +type: added diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5d2f32913b5e8199cf08818480b0562e7ccf31a0..cb8ba1baab70b48691f41db4bb9221449e2f0e40 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -21009,6 +21009,9 @@ msgstr "" msgid "Toggle Sidebar" msgstr "" +msgid "Toggle all threads" +msgstr "" + msgid "Toggle backtrace" msgstr "" diff --git a/spec/frontend/notes/components/discussion_counter_spec.js b/spec/frontend/notes/components/discussion_counter_spec.js index c9375df07e80ede473d8d4b280defaa6d8ab5754..77603c16f820da42b92517f8fd46bbb70977fb96 100644 --- a/spec/frontend/notes/components/discussion_counter_spec.js +++ b/spec/frontend/notes/components/discussion_counter_spec.js @@ -75,17 +75,66 @@ describe('DiscussionCounter component', () => { }); it.each` - title | resolved | hasNextBtn | isActive | icon | groupLength - ${'hasNextButton'} | ${false} | ${true} | ${false} | ${'check-circle'} | ${2} - ${'allResolved'} | ${true} | ${false} | ${true} | ${'check-circle-filled'} | ${0} - `('renders correctly if $title', ({ resolved, hasNextBtn, isActive, icon, groupLength }) => { + title | resolved | isActive | icon | groupLength + ${'not allResolved'} | ${false} | ${false} | ${'check-circle'} | ${3} + ${'allResolved'} | ${true} | ${true} | ${'check-circle-filled'} | ${1} + `('renders correctly if $title', ({ resolved, isActive, icon, groupLength }) => { updateStore({ resolvable: true, resolved }); wrapper = shallowMount(DiscussionCounter, { store, localVue }); - expect(wrapper.find(`.has-next-btn`).exists()).toBe(hasNextBtn); expect(wrapper.find(`.is-active`).exists()).toBe(isActive); expect(wrapper.find({ name: icon }).exists()).toBe(true); expect(wrapper.findAll('[role="group"').length).toBe(groupLength); }); }); + + describe('toggle all threads button', () => { + let toggleAllButton; + const updateStoreWithExpanded = expanded => { + const discussion = { ...discussionMock, expanded }; + store.commit(types.SET_INITIAL_DISCUSSIONS, [discussion]); + store.dispatch('updateResolvableDiscussionsCounts'); + wrapper = shallowMount(DiscussionCounter, { store, localVue }); + toggleAllButton = wrapper.find('.toggle-all-discussions-btn'); + }; + + afterEach(() => wrapper.destroy()); + + it('calls button handler when clicked', () => { + updateStoreWithExpanded(true); + + wrapper.setMethods({ handleExpandDiscussions: jest.fn() }); + toggleAllButton.trigger('click'); + + expect(wrapper.vm.handleExpandDiscussions).toHaveBeenCalledTimes(1); + }); + + it('collapses all discussions if expanded', () => { + updateStoreWithExpanded(true); + + expect(wrapper.vm.allExpanded).toBe(true); + expect(toggleAllButton.find({ name: 'angle-up' }).exists()).toBe(true); + + toggleAllButton.trigger('click'); + + return wrapper.vm.$nextTick().then(() => { + expect(wrapper.vm.allExpanded).toBe(false); + expect(toggleAllButton.find({ name: 'angle-down' }).exists()).toBe(true); + }); + }); + + it('expands all discussions if collapsed', () => { + updateStoreWithExpanded(false); + + expect(wrapper.vm.allExpanded).toBe(false); + expect(toggleAllButton.find({ name: 'angle-down' }).exists()).toBe(true); + + toggleAllButton.trigger('click'); + + return wrapper.vm.$nextTick().then(() => { + expect(wrapper.vm.allExpanded).toBe(true); + expect(toggleAllButton.find({ name: 'angle-up' }).exists()).toBe(true); + }); + }); + }); }); diff --git a/spec/frontend/notes/stores/mutation_spec.js b/spec/frontend/notes/stores/mutation_spec.js index ee772afbc031604c237318139ab639dbd6e62cf4..ea5658821b10c3d007f46c9597749390745313d6 100644 --- a/spec/frontend/notes/stores/mutation_spec.js +++ b/spec/frontend/notes/stores/mutation_spec.js @@ -329,6 +329,52 @@ describe('Notes Store mutations', () => { }); }); + describe('SET_EXPAND_DISCUSSIONS', () => { + it('should succeed when discussions are null', () => { + const state = {}; + + mutations.SET_EXPAND_DISCUSSIONS(state, { discussionIds: null, expanded: true }); + + expect(state).toEqual({}); + }); + + it('should succeed when discussions are empty', () => { + const state = {}; + + mutations.SET_EXPAND_DISCUSSIONS(state, { discussionIds: [], expanded: true }); + + expect(state).toEqual({}); + }); + + it('should open all closed discussions', () => { + const discussion1 = Object.assign({}, discussionMock, { id: 0, expanded: false }); + const discussion2 = Object.assign({}, discussionMock, { id: 1, expanded: true }); + const discussionIds = [discussion1.id, discussion2.id]; + + const state = { discussions: [discussion1, discussion2] }; + + mutations.SET_EXPAND_DISCUSSIONS(state, { discussionIds, expanded: true }); + + state.discussions.forEach(discussion => { + expect(discussion.expanded).toEqual(true); + }); + }); + + it('should close all opened discussions', () => { + const discussion1 = Object.assign({}, discussionMock, { id: 0, expanded: false }); + const discussion2 = Object.assign({}, discussionMock, { id: 1, expanded: true }); + const discussionIds = [discussion1.id, discussion2.id]; + + const state = { discussions: [discussion1, discussion2] }; + + mutations.SET_EXPAND_DISCUSSIONS(state, { discussionIds, expanded: false }); + + state.discussions.forEach(discussion => { + expect(discussion.expanded).toEqual(false); + }); + }); + }); + describe('UPDATE_NOTE', () => { it('should update a note', () => { const state = {