From de56cc94ce11c1392a9f50dcd5801f2707fe6a4c Mon Sep 17 00:00:00 2001 From: Stanislav Lashmanov Date: Wed, 28 Aug 2024 17:34:22 +0400 Subject: [PATCH 1/3] Add Expand and Collapse all files buttons to merge requests Also supported with hotkeys: ';' and 'shift+;' Changelog: added --- .../behaviors/shortcuts/keybindings.js | 13 ++ .../javascripts/diffs/components/app.vue | 29 +++-- .../diffs/components/compare_versions.vue | 35 ------ .../diffs/components/diff_app_controls.vue | 111 ++++++++++++++++++ .../diffs/components/diff_stats.vue | 8 +- app/assets/javascripts/diffs/store/actions.js | 8 ++ .../javascripts/diffs/store/mutation_types.js | 2 + .../javascripts/diffs/store/mutations.js | 8 ++ .../diffs/stores/legacy_diffs/actions.js | 8 ++ .../diffs/stores/legacy_diffs/mutations.js | 8 ++ .../javascripts/helpers/diffs_helper.js | 4 - ee/spec/frontend/diffs/components/app_spec.js | 1 + locale/gitlab.pot | 3 + ...comments_on_whitespace_hidden_diff_spec.rb | 2 +- spec/frontend/diffs/components/app_spec.js | 37 +++++- .../diffs/components/compare_versions_spec.js | 25 ++-- .../components/diff_app_controls_spec.js | 90 ++++++++++++++ .../diffs/components/diff_stats_spec.js | 14 +-- spec/frontend/diffs/store/actions_spec.js | 34 ++++++ spec/frontend/diffs/store/mutations_spec.js | 16 +++ .../diffs/stores/legacy_diffs/actions_spec.js | 34 ++++++ .../stores/legacy_diffs/mutations_spec.js | 16 +++ spec/frontend/helpers/diffs_helper_spec.js | 37 ------ 23 files changed, 427 insertions(+), 116 deletions(-) create mode 100644 app/assets/javascripts/diffs/components/diff_app_controls.vue create mode 100644 spec/frontend/diffs/components/diff_app_controls_spec.js diff --git a/app/assets/javascripts/behaviors/shortcuts/keybindings.js b/app/assets/javascripts/behaviors/shortcuts/keybindings.js index 8aa7db18c7ffdd..a60395e5b2e99d 100644 --- a/app/assets/javascripts/behaviors/shortcuts/keybindings.js +++ b/app/assets/javascripts/behaviors/shortcuts/keybindings.js @@ -487,6 +487,19 @@ export const MR_COPY_SOURCE_BRANCH_NAME = { defaultKeys: ['b'], }; +export const MR_EXPAND_ALL_FILES = { + id: 'mergeRequests.expandAllFiles', + description: __('Expand all files'), + defaultKeys: [';'], +}; + +export const MR_COLLAPSE_ALL_FILES = { + id: 'mergeRequests.collapseAllFiles', + description: __('Collapse all files'), + // eslint-disable-next-line @gitlab/require-i18n-strings + defaultKeys: ['shift+;'], +}; + export const MR_COMMITS_NEXT_COMMIT = { id: 'mergeRequestCommits.nextCommit', description: __('Next commit'), diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 81fa13235d1678..f1f0217b8440fe 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -16,7 +16,6 @@ import { } from '~/behaviors/shortcuts/keybindings'; import { createAlert } from '~/alert'; import { InternalEvents } from '~/tracking'; -import { isSingleViewStyle } from '~/helpers/diffs_helper'; import { helpPagePath } from '~/helpers/help_page_helper'; import { parseBoolean, handleLocationHash } from '~/lib/utils/common_utils'; import { BV_HIDE_TOOLTIP, DEFAULT_DEBOUNCE_AND_THROTTLE_MS } from '~/lib/utils/constants'; @@ -60,6 +59,7 @@ import HiddenFilesWarning from './hidden_files_warning.vue'; import NoChanges from './no_changes.vue'; import VirtualScrollerScrollSync from './virtual_scroller_scroll_sync'; import DiffsFileTree from './diffs_file_tree.vue'; +import DiffAppControls from './diff_app_controls.vue'; import getMRCodequalityAndSecurityReports from './graphql/get_mr_codequality_and_security_reports.query.graphql'; export const FINDINGS_STATUS_PARSED = 'PARSED'; @@ -71,6 +71,7 @@ export default { FINDINGS_STATUS_PARSED, FINDINGS_STATUS_ERROR, components: { + DiffAppControls, DiffsFileTree, FindingsDrawer, DynamicScroller, @@ -238,6 +239,8 @@ export default { 'targetBranchName', 'branchName', 'showTreeList', + 'addedLines', + 'removedLines', ]), ...mapGetters('diffs', [ 'whichCollapsedTypes', @@ -317,6 +320,9 @@ export default { }; return throttle(hide, 100); }, + hasChanges() { + return this.diffFiles.length > 0; + }, }, watch: { commit(newCommit, oldCommit) { @@ -450,6 +456,8 @@ export default { 'disableVirtualScroller', 'fetchLinkedFile', 'toggleTreeList', + 'expandAllFiles', + 'collapseAllFiles', ]), ...mapActions('findingsDrawer', ['setDrawer']), closeDrawer() { @@ -530,12 +538,6 @@ export default { refetchDiffData({ refetchMeta = true } = {}) { this.fetchData({ toggleTree: false, fetchMeta: refetchMeta }); }, - needsReload() { - return this.diffFiles.length && isSingleViewStyle(this.diffFiles[0]); - }, - needsFirstLoad() { - return !this.diffFiles.length; - }, fetchData({ toggleTree = true, fetchMeta = true } = {}) { if (this.linkedFileUrl && this.linkedFileStatus !== 'loaded') { this.linkedFileStatus = 'loading'; @@ -749,7 +751,18 @@ export default {
- +
+ + +
diff --git a/app/assets/javascripts/diffs/components/diff_app_controls.vue b/app/assets/javascripts/diffs/components/diff_app_controls.vue new file mode 100644 index 00000000000000..66cbcdcde394a5 --- /dev/null +++ b/app/assets/javascripts/diffs/components/diff_app_controls.vue @@ -0,0 +1,111 @@ + + + diff --git a/app/assets/javascripts/diffs/components/diff_stats.vue b/app/assets/javascripts/diffs/components/diff_stats.vue index 35e71714a82fe6..baab0a86ad760f 100644 --- a/app/assets/javascripts/diffs/components/diff_stats.vue +++ b/app/assets/javascripts/diffs/components/diff_stats.vue @@ -20,7 +20,7 @@ export default { type: Number, required: true, }, - diffFilesCountText: { + diffsCount: { type: String, required: false, default: null, @@ -28,13 +28,13 @@ export default { }, computed: { diffFilesLength() { - return parseInt(this.diffFilesCountText, 10); + return parseInt(this.diffsCount, 10); }, filesText() { return n__('file', 'files', this.diffFilesLength); }, isCompareVersionsHeader() { - return Boolean(this.diffFilesCountText); + return Boolean(this.diffsCount); }, hasDiffFiles() { return isNumber(this.diffFilesLength) && this.diffFilesLength >= 0; @@ -63,7 +63,7 @@ export default {
- {{ diffFilesCountText }} {{ filesText }} + {{ diffsCount }} {{ filesText }}
{ export const toggleAllDiffDiscussions = ({ commit, getters }) => { commit(types.SET_EXPAND_ALL_DIFF_DISCUSSIONS, !getters.allDiffDiscussionsExpanded); }; + +export const expandAllFiles = ({ commit }) => { + commit(types.SET_COLLAPSED_STATE_FOR_ALL_FILES, { collapsed: false }); +}; + +export const collapseAllFiles = ({ commit }) => { + commit(types.SET_COLLAPSED_STATE_FOR_ALL_FILES, { collapsed: true }); +}; diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index a9e5a47c7bb115..7b02ff49e0bcb5 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -59,3 +59,5 @@ export const DISABLE_VIRTUAL_SCROLLING = 'DISABLE_VIRTUAL_SCROLLING'; export const TOGGLE_FILE_COMMENT_FORM = 'TOGGLE_FILE_COMMENT_FORM'; export const SET_FILE_COMMENT_FORM = 'SET_FILE_COMMENT_FORM'; export const ADD_DRAFT_TO_FILE = 'ADD_DRAFT_TO_FILE'; + +export const SET_COLLAPSED_STATE_FOR_ALL_FILES = 'SET_COLLAPSED_STATE_FOR_ALL_FILES'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 712d889781d345..65771f6b568f04 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -427,4 +427,12 @@ export default { [types.SET_LINKED_FILE_HASH](state, fileHash) { state.linkedFileHash = fileHash; }, + [types.SET_COLLAPSED_STATE_FOR_ALL_FILES](state, { collapsed }) { + state.diffFiles.forEach((file) => { + const { viewer } = file; + if (!viewer) return; + viewer.automaticallyCollapsed = false; + viewer.manuallyCollapsed = collapsed; + }); + }, }; diff --git a/app/assets/javascripts/diffs/stores/legacy_diffs/actions.js b/app/assets/javascripts/diffs/stores/legacy_diffs/actions.js index a134a7273b2dfa..a974ff6ca2aae3 100644 --- a/app/assets/javascripts/diffs/stores/legacy_diffs/actions.js +++ b/app/assets/javascripts/diffs/stores/legacy_diffs/actions.js @@ -1122,3 +1122,11 @@ export function unlinkFile() { export function toggleAllDiffDiscussions() { this[types.SET_EXPAND_ALL_DIFF_DISCUSSIONS](!this.allDiffDiscussionsExpanded); } + +export function expandAllFiles() { + this[types.SET_COLLAPSED_STATE_FOR_ALL_FILES]({ collapsed: false }); +} + +export function collapseAllFiles() { + this[types.SET_COLLAPSED_STATE_FOR_ALL_FILES]({ collapsed: true }); +} diff --git a/app/assets/javascripts/diffs/stores/legacy_diffs/mutations.js b/app/assets/javascripts/diffs/stores/legacy_diffs/mutations.js index d56b24dd9d6761..44c9ee9b79f985 100644 --- a/app/assets/javascripts/diffs/stores/legacy_diffs/mutations.js +++ b/app/assets/javascripts/diffs/stores/legacy_diffs/mutations.js @@ -420,4 +420,12 @@ export default { [types.SET_LINKED_FILE_HASH](fileHash) { this.linkedFileHash = fileHash; }, + [types.SET_COLLAPSED_STATE_FOR_ALL_FILES]({ collapsed }) { + this.diffFiles.forEach((file) => { + const { viewer } = file; + if (!viewer) return; + viewer.automaticallyCollapsed = false; + viewer.manuallyCollapsed = collapsed; + }); + }, }; diff --git a/app/assets/javascripts/helpers/diffs_helper.js b/app/assets/javascripts/helpers/diffs_helper.js index e733b3109f352b..b67a49c577e597 100644 --- a/app/assets/javascripts/helpers/diffs_helper.js +++ b/app/assets/javascripts/helpers/diffs_helper.js @@ -6,10 +6,6 @@ export function hasParallelLines(diffFile) { return diffFile?.parallel_diff_lines?.length > 0; } -export function isSingleViewStyle(diffFile) { - return !hasParallelLines(diffFile) || !hasInlineLines(diffFile); -} - export function hasDiff(diffFile) { return hasInlineLines(diffFile) || hasParallelLines(diffFile); } diff --git a/ee/spec/frontend/diffs/components/app_spec.js b/ee/spec/frontend/diffs/components/app_spec.js index 0a123f9c541974..242402f4c0d8dc 100644 --- a/ee/spec/frontend/diffs/components/app_spec.js +++ b/ee/spec/frontend/diffs/components/app_spec.js @@ -39,6 +39,7 @@ describe('diffs/components/app', () => { }, }; store.getters['findingsDrawer/activeDrawer'] = {}; + store.getters['diffs/diffFiles'] = []; store.getters['diffs/flatBlobsList'] = []; store.getters['diffs/isBatchLoading'] = false; store.getters['diffs/isBatchLoadingError'] = false; diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 7e288299ca9697..99699cfdcc842a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -13437,6 +13437,9 @@ msgstr "" msgid "Collapse %{section} merge requests" msgstr "" +msgid "Collapse all files" +msgstr "" + msgid "Collapse eligible approvers" msgstr "" diff --git a/spec/features/merge_request/user_comments_on_whitespace_hidden_diff_spec.rb b/spec/features/merge_request/user_comments_on_whitespace_hidden_diff_spec.rb index 7c5d448e82cd13..0ce42d977e7af7 100644 --- a/spec/features/merge_request/user_comments_on_whitespace_hidden_diff_spec.rb +++ b/spec/features/merge_request/user_comments_on_whitespace_hidden_diff_spec.rb @@ -130,7 +130,7 @@ context 'when commenting on collapsed line combinations that are not present in the real diff' do before do - find_all('[data-testid="expand-icon"]').first.click + find_all('[aria-label="Expand all lines"]').first.click click_diff_line( find_by_scrolling('div[data-path="files/js/breadcrumbs.js"] .left-side a[data-linenumber="15"]') diff --git a/spec/frontend/diffs/components/app_spec.js b/spec/frontend/diffs/components/app_spec.js index 9f41a3e05ad868..bdc33e4bcc1a79 100644 --- a/spec/frontend/diffs/components/app_spec.js +++ b/spec/frontend/diffs/components/app_spec.js @@ -18,6 +18,7 @@ import DiffFile from '~/diffs/components/diff_file.vue'; import NoChanges from '~/diffs/components/no_changes.vue'; import FindingsDrawer from '~/diffs/components/shared/findings_drawer.vue'; import DiffsFileTree from '~/diffs/components/diffs_file_tree.vue'; +import DiffAppControls from '~/diffs/components/diff_app_controls.vue'; import CollapsedFilesWarning from '~/diffs/components/collapsed_files_warning.vue'; import HiddenFilesWarning from '~/diffs/components/hidden_files_warning.vue'; @@ -68,7 +69,7 @@ describe('diffs/components/app', () => { provisions = {}, baseConfig = {}, actions = {}, - }) => { + } = {}) => { fakeApollo = createMockApollo([ [getMRCodequalityAndSecurityReports, codeQualityAndSastQueryHandlerSuccess], ]); @@ -499,13 +500,43 @@ describe('diffs/components/app', () => { }); expect(wrapper.findComponent(CompareVersions).exists()).toBe(true); - expect(wrapper.findComponent(CompareVersions).props()).toEqual( + }); + + it('should render app controls component', () => { + createComponent({ + extendStore: ({ state }) => { + state.diffs.diffFiles = diffsMockData; + state.diffs.realSize = '10'; + state.diffs.addedLines = 15; + state.diffs.removedLines = 20; + }, + }); + + expect(wrapper.findComponent(DiffAppControls).exists()).toBe(true); + expect(wrapper.findComponent(DiffAppControls).props()).toEqual( expect.objectContaining({ - diffFilesCountText: null, + hasChanges: true, + diffsCount: '10', + addedLines: 15, + removedLines: 20, }), ); }); + it('collapses all files', async () => { + createComponent(); + const spy = jest.spyOn(store, 'dispatch'); + await wrapper.findComponent(DiffAppControls).vm.$emit('collapseAllFiles'); + expect(spy).toHaveBeenCalledWith('diffs/collapseAllFiles', undefined); + }); + + it('expands all files', async () => { + createComponent(); + jest.spyOn(store, 'dispatch'); + await wrapper.findComponent(DiffAppControls).vm.$emit('expandAllFiles'); + expect(store.dispatch).toHaveBeenCalledWith('diffs/expandAllFiles', undefined); + }); + describe('warnings', () => { describe('hidden files', () => { it('should render hidden files warning if render overflow warning is present', () => { diff --git a/spec/frontend/diffs/components/compare_versions_spec.js b/spec/frontend/diffs/components/compare_versions_spec.js index 2355f1dff0d983..9480754161e3c0 100644 --- a/spec/frontend/diffs/components/compare_versions_spec.js +++ b/spec/frontend/diffs/components/compare_versions_spec.js @@ -22,7 +22,7 @@ describe('CompareVersions', () => { const targetBranchName = 'tmp-wine-dev'; const { commit } = getDiffWithCommit; - const createWrapper = (props = {}, commitArgs = {}, createCommit = true) => { + const createWrapper = (commitArgs = {}, createCommit = true) => { if (createCommit) { store.state.diffs.commit = { ...store.state.diffs.commit, ...commitArgs }; } @@ -31,11 +31,6 @@ describe('CompareVersions', () => { mocks: { $store: store, }, - propsData: { - mergeRequestDiffs: diffsMockData, - diffFilesCountText: '1', - ...props, - }, }); }; const findCompareSourceDropdown = () => wrapper.find('.mr-version-dropdown'); @@ -77,7 +72,7 @@ describe('CompareVersions', () => { describe('template', () => { beforeEach(() => { - createWrapper({}, {}, false); + createWrapper({}, false); }); it('should render Tree List toggle button with correct attribute values', () => { @@ -147,7 +142,7 @@ describe('CompareVersions', () => { describe('without neighbor commits', () => { beforeEach(() => { - createWrapper({ commit: { ...commit, prev_commit_id: null, next_commit_id: null } }); + createWrapper({ ...commit, prev_commit_id: null, next_commit_id: null }); }); it('does not render any navigation buttons', () => { @@ -165,20 +160,16 @@ describe('CompareVersions', () => { prev_commit_id: 'prev', }; - createWrapper({}, mrCommit); + createWrapper(mrCommit); }); it('renders the commit navigation buttons', () => { expect(getCommitNavButtonsElement().exists()).toEqual(true); - createWrapper({ - commit: { ...mrCommit, next_commit_id: null }, - }); + createWrapper({ ...mrCommit, next_commit_id: null }); expect(getCommitNavButtonsElement().exists()).toEqual(true); - createWrapper({ - commit: { ...mrCommit, prev_commit_id: null }, - }); + createWrapper({ ...mrCommit, prev_commit_id: null }); expect(getCommitNavButtonsElement().exists()).toEqual(true); }); @@ -204,7 +195,7 @@ describe('CompareVersions', () => { }); it('renders a disabled button when there is no prev commit', () => { - createWrapper({}, { ...mrCommit, prev_commit_id: null }); + createWrapper({ ...mrCommit, prev_commit_id: null }); const button = getPrevCommitNavElement(); @@ -234,7 +225,7 @@ describe('CompareVersions', () => { }); it('renders a disabled button when there is no next commit', () => { - createWrapper({}, { ...mrCommit, next_commit_id: null }); + createWrapper({ ...mrCommit, next_commit_id: null }); const button = getNextCommitNavElement(); diff --git a/spec/frontend/diffs/components/diff_app_controls_spec.js b/spec/frontend/diffs/components/diff_app_controls_spec.js new file mode 100644 index 00000000000000..5be8294de795f7 --- /dev/null +++ b/spec/frontend/diffs/components/diff_app_controls_spec.js @@ -0,0 +1,90 @@ +import { shallowMount } from '@vue/test-utils'; +import { GlButton } from '@gitlab/ui'; +import DiffAppControls from '~/diffs/components/diff_app_controls.vue'; +import DiffStats from '~/diffs/components/diff_stats.vue'; +import SettingsDropdown from '~/diffs/components/settings_dropdown.vue'; +import { + keysFor, + MR_COLLAPSE_ALL_FILES, + MR_EXPAND_ALL_FILES, +} from '~/behaviors/shortcuts/keybindings'; +import { Mousetrap } from '~/lib/mousetrap'; + +const DEFAULT_PROPS = { + diffsCount: '5', + addedLines: 10, + removedLines: 5, +}; + +describe('DiffAppControls', () => { + let wrapper; + + const createComponent = (props = {}) => { + wrapper = shallowMount(DiffAppControls, { + propsData: { + ...DEFAULT_PROPS, + ...props, + }, + }); + }; + + const findButtonByIcon = (icon) => + wrapper + .findAllComponents(GlButton) + .filter((buttonWrapper) => buttonWrapper.props('icon') === icon) + .at(0); + + describe('when has changes', () => { + beforeEach(() => { + createComponent({ hasChanges: true }); + }); + + it('renders diff stats', () => { + expect(wrapper.findComponent(DiffStats).exists()).toBe(true); + expect(wrapper.findComponent(DiffStats).props()).toMatchObject({ + diffFilesCountText: DEFAULT_PROPS.diffsCount, + addedLines: DEFAULT_PROPS.addedLines, + removedLines: DEFAULT_PROPS.removedLines, + }); + }); + + it('renders expand buttons', () => { + expect(findButtonByIcon('expand').exists()).toBe(true); + expect(findButtonByIcon('collapse').exists()).toBe(true); + }); + + it('emits expandAllFiles', () => { + findButtonByIcon('expand').vm.$emit('click'); + expect(wrapper.emitted('expandAllFiles')).toStrictEqual([[]]); + }); + + it('emits expandAllFiles on hotkey', () => { + Mousetrap.trigger(keysFor(MR_EXPAND_ALL_FILES)[0]); + expect(wrapper.emitted('expandAllFiles')).toStrictEqual([[]]); + }); + + it('emits collapseAllFiles', () => { + findButtonByIcon('collapse').vm.$emit('click'); + expect(wrapper.emitted('collapseAllFiles')).toStrictEqual([[]]); + }); + + it('emits collapseAllFiles on hotkey', () => { + Mousetrap.trigger(keysFor(MR_COLLAPSE_ALL_FILES)[0]); + expect(wrapper.emitted('collapseAllFiles')).toStrictEqual([[]]); + }); + + it('renders settings', () => { + expect(wrapper.findComponent(SettingsDropdown).exists()).toBe(true); + }); + }); + + describe('when has no changes', () => { + beforeEach(() => { + createComponent({ hasChanges: false }); + }); + + it('renders settings', () => { + expect(wrapper.findComponent(SettingsDropdown).exists()).toBe(true); + }); + }); +}); diff --git a/spec/frontend/diffs/components/diff_stats_spec.js b/spec/frontend/diffs/components/diff_stats_spec.js index 3a04547fa69e1b..737654b1406a4d 100644 --- a/spec/frontend/diffs/components/diff_stats_spec.js +++ b/spec/frontend/diffs/components/diff_stats_spec.js @@ -28,15 +28,15 @@ describe('diff_stats', () => { describe('diff stats group', () => { const findDiffStatsGroup = () => wrapper.findAll('.diff-stats-group'); - it('is not rendered if diffFilesCountText is empty', () => { + it('is not rendered if diffsCount is empty', () => { createComponent(); expect(findDiffStatsGroup().length).toBe(2); }); - it('is not rendered if diffFilesCountText is not a number', () => { + it('is not rendered if diffsCount is not a number', () => { createComponent({ - diffFilesCountText: null, + diffsCount: null, }); expect(findDiffStatsGroup().length).toBe(2); @@ -95,7 +95,7 @@ describe('diff_stats', () => { const oneFileChanged = '0'; createComponent({ - diffFilesCountText: oneFileChanged, + diffsCount: oneFileChanged, }); expect(findIcon('doc-code').textContent.trim()).toBe(`${oneFileChanged} files`); @@ -105,7 +105,7 @@ describe('diff_stats', () => { const oneFileChanged = '1'; createComponent({ - diffFilesCountText: oneFileChanged, + diffsCount: oneFileChanged, }); expect(findIcon('doc-code').textContent.trim()).toBe(`${oneFileChanged} file`); @@ -113,7 +113,7 @@ describe('diff_stats', () => { it('shows amount of files change with plural "files" when multiple files are changed', () => { createComponent({ - diffFilesCountText: DIFF_FILES_COUNT, + diffsCount: DIFF_FILES_COUNT, }); expect(findIcon('doc-code').textContent.trim()).toContain(`${DIFF_FILES_COUNT} files`); @@ -121,7 +121,7 @@ describe('diff_stats', () => { it('shows amount of files change with plural "files" when files changed is truncated', () => { createComponent({ - diffFilesCountText: DIFF_FILES_COUNT_TRUNCATED, + diffsCount: DIFF_FILES_COUNT_TRUNCATED, }); expect(findIcon('doc-code').textContent.trim()).toContain( diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js index 8339909436a8ac..b8c62bb06e249b 100644 --- a/spec/frontend/diffs/store/actions_spec.js +++ b/spec/frontend/diffs/store/actions_spec.js @@ -2218,4 +2218,38 @@ describe('DiffsStoreActions', () => { testAction(diffActions.unlinkFile, undefined, {}, [], []); }); }); + + describe('expandAllFiles', () => { + it('triggers mutation', () => { + testAction( + diffActions.expandAllFiles, + undefined, + {}, + [ + { + type: types.SET_COLLAPSED_STATE_FOR_ALL_FILES, + payload: { collapsed: false }, + }, + ], + [], + ); + }); + }); + + describe('collapseAllFiles', () => { + it('triggers mutation', () => { + testAction( + diffActions.collapseAllFiles, + undefined, + {}, + [ + { + type: types.SET_COLLAPSED_STATE_FOR_ALL_FILES, + payload: { collapsed: true }, + }, + ], + [], + ); + }); + }); }); diff --git a/spec/frontend/diffs/store/mutations_spec.js b/spec/frontend/diffs/store/mutations_spec.js index 1d9e51e05e6b77..8c71e47e9698f9 100644 --- a/spec/frontend/diffs/store/mutations_spec.js +++ b/spec/frontend/diffs/store/mutations_spec.js @@ -1349,4 +1349,20 @@ describe('DiffsStoreMutations', () => { expect(state.linkedFileHash).toBe(file.file_hash); }); }); + + describe('SET_COLLAPSED_STATE_FOR_ALL_FILES', () => { + it('sets collapsed state for all files', () => { + const state = { + diffFiles: [getDiffFileMock(), getDiffFileMock()], + }; + + mutations[types.SET_COLLAPSED_STATE_FOR_ALL_FILES](state, { collapsed: true }); + + expect( + state.diffFiles.every( + ({ viewer }) => viewer.manuallyCollapsed && !viewer.automaticallyCollapsed, + ), + ).toBe(true); + }); + }); }); diff --git a/spec/frontend/diffs/stores/legacy_diffs/actions_spec.js b/spec/frontend/diffs/stores/legacy_diffs/actions_spec.js index 597b93b8c259fc..c40539ac8d8fac 100644 --- a/spec/frontend/diffs/stores/legacy_diffs/actions_spec.js +++ b/spec/frontend/diffs/stores/legacy_diffs/actions_spec.js @@ -2219,4 +2219,38 @@ describe.skip('DiffsStoreActions', () => { testAction(diffActions.unlinkFile, undefined, {}, [], []); }); }); + + describe('expandAllFiles', () => { + it('triggers mutation', () => { + testAction( + diffActions.expandAllFiles, + undefined, + {}, + [ + { + type: types.SET_COLLAPSED_STATE_FOR_ALL_FILES, + payload: { collapsed: false }, + }, + ], + [], + ); + }); + }); + + describe('collapseAllFiles', () => { + it('triggers mutation', () => { + testAction( + diffActions.collapseAllFiles, + undefined, + {}, + [ + { + type: types.SET_COLLAPSED_STATE_FOR_ALL_FILES, + payload: { collapsed: true }, + }, + ], + [], + ); + }); + }); }); diff --git a/spec/frontend/diffs/stores/legacy_diffs/mutations_spec.js b/spec/frontend/diffs/stores/legacy_diffs/mutations_spec.js index 77917730a9c1f6..11a778b69f8251 100644 --- a/spec/frontend/diffs/stores/legacy_diffs/mutations_spec.js +++ b/spec/frontend/diffs/stores/legacy_diffs/mutations_spec.js @@ -1350,4 +1350,20 @@ describe.skip('DiffsStoreMutations', () => { expect(state.linkedFileHash).toBe(file.file_hash); }); }); + + describe('SET_COLLAPSED_STATE_FOR_ALL_FILES', () => { + it('sets collapsed state for all files', () => { + const state = { + diffFiles: [getDiffFileMock(), getDiffFileMock()], + }; + + mutations[types.SET_COLLAPSED_STATE_FOR_ALL_FILES](state, { collapsed: true }); + + expect( + state.diffFiles.every( + ({ viewer }) => viewer.manuallyCollapsed && !viewer.automaticallyCollapsed, + ), + ).toBe(true); + }); + }); }); diff --git a/spec/frontend/helpers/diffs_helper_spec.js b/spec/frontend/helpers/diffs_helper_spec.js index ba5c5ce8534d0c..7cf02a69da1515 100644 --- a/spec/frontend/helpers/diffs_helper_spec.js +++ b/spec/frontend/helpers/diffs_helper_spec.js @@ -56,43 +56,6 @@ describe('diffs helper', () => { }); }); - describe('isSingleViewStyle', () => { - it('is true when the file has at least 1 inline line but no parallel lines for any reason', () => { - const noParallelLines = getDiffFile({ parallel_diff_lines: undefined }); - const emptyParallelLines = getDiffFile({ parallel_diff_lines: [] }); - - expect(diffsHelper.isSingleViewStyle(noParallelLines)).toBe(true); - expect(diffsHelper.isSingleViewStyle(emptyParallelLines)).toBe(true); - }); - - it('is true when the file has at least 1 parallel line but no inline lines for any reason', () => { - const noInlineLines = getDiffFile({ highlighted_diff_lines: undefined }); - const emptyInlineLines = getDiffFile({ highlighted_diff_lines: [] }); - - expect(diffsHelper.isSingleViewStyle(noInlineLines)).toBe(true); - expect(diffsHelper.isSingleViewStyle(emptyInlineLines)).toBe(true); - }); - - it('is true when the file does not have any inline lines or parallel lines for any reason', () => { - const noLines = getDiffFile({ - highlighted_diff_lines: undefined, - parallel_diff_lines: undefined, - }); - const emptyLines = getDiffFile({ - highlighted_diff_lines: [], - parallel_diff_lines: [], - }); - - expect(diffsHelper.isSingleViewStyle(noLines)).toBe(true); - expect(diffsHelper.isSingleViewStyle(emptyLines)).toBe(true); - expect(diffsHelper.isSingleViewStyle()).toBe(true); - }); - - it('is false when the file has both inline and parallel lines', () => { - expect(diffsHelper.isSingleViewStyle(getDiffFile())).toBe(false); - }); - }); - describe.each` context | inline | parallel | expected ${'only has inline lines'} | ${['line']} | ${undefined} | ${true} -- GitLab From 55b66c226268fab882bff4b9a6370a8d0d7cdf5d Mon Sep 17 00:00:00 2001 From: Stanislav Lashmanov Date: Fri, 6 Sep 2024 19:27:57 +0400 Subject: [PATCH 2/3] Address review feedback --- .../javascripts/diffs/components/app.vue | 2 +- .../diffs/components/compare_versions.vue | 20 +++++------ .../diffs/components/diff_app_controls.vue | 22 ++++++------ spec/frontend/diffs/components/app_spec.js | 17 +++++++--- .../diffs/components/compare_versions_spec.js | 34 +++++++++---------- .../components/diff_app_controls_spec.js | 2 +- 6 files changed, 49 insertions(+), 48 deletions(-) diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index f1f0217b8440fe..61eb9c55ff46bf 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -752,7 +752,7 @@ export default {
- +