diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 9218cc00303e25c9b813fe512804ded149cf9ea5..53eb8cd8eb8576285806e75bb25a13f9d3951c72 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -10,7 +10,10 @@ import PanelResizer from '~/vue_shared/components/panel_resizer.vue'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { isSingleViewStyle } from '~/helpers/diffs_helper'; import { updateHistory } from '~/lib/utils/url_utility'; -import eventHub from '../../notes/event_hub'; + +import notesEventHub from '../../notes/event_hub'; +import eventHub from '../event_hub'; + import CompareVersions from './compare_versions.vue'; import DiffFile from './diff_file.vue'; import NoChanges from './no_changes.vue'; @@ -22,6 +25,7 @@ import MergeConflictWarning from './merge_conflict_warning.vue'; import CollapsedFilesWarning from './collapsed_files_warning.vue'; import { diffsApp } from '../utils/performance'; +import { fileByFile } from '../utils/preferences'; import { TREE_LIST_WIDTH_STORAGE_KEY, @@ -34,6 +38,7 @@ import { ALERT_OVERFLOW_HIDDEN, ALERT_MERGE_CONFLICT, ALERT_COLLAPSED_FILES, + EVT_VIEW_FILE_BY_FILE, } from '../constants'; export default { @@ -114,7 +119,7 @@ export default { required: false, default: false, }, - viewDiffsFileByFile: { + fileByFileUserPreference: { type: Boolean, required: false, default: false, @@ -154,6 +159,7 @@ export default { 'conflictResolutionPath', 'canMerge', 'hasConflicts', + 'viewDiffsFileByFile', ]), ...mapGetters('diffs', ['whichCollapsedTypes', 'isParallelView', 'currentDiffIndex']), ...mapGetters(['isNotesFetched', 'getNoteableData']), @@ -254,7 +260,7 @@ export default { projectPath: this.projectPath, dismissEndpoint: this.dismissEndpoint, showSuggestPopover: this.showSuggestPopover, - viewDiffsFileByFile: this.viewDiffsFileByFile, + viewDiffsFileByFile: fileByFile(this.fileByFileUserPreference), }); if (this.shouldShow) { @@ -278,8 +284,10 @@ export default { created() { this.adjustView(); - eventHub.$once('fetchDiffData', this.fetchData); - eventHub.$on('refetchDiffData', this.refetchDiffData); + notesEventHub.$once('fetchDiffData', this.fetchData); + notesEventHub.$on('refetchDiffData', this.refetchDiffData); + eventHub.$on(EVT_VIEW_FILE_BY_FILE, this.fileByFileListener); + this.CENTERED_LIMITED_CONTAINER_CLASSES = CENTERED_LIMITED_CONTAINER_CLASSES; this.unwatchDiscussions = this.$watch( @@ -300,8 +308,10 @@ export default { beforeDestroy() { diffsApp.deinstrument(); - eventHub.$off('fetchDiffData', this.fetchData); - eventHub.$off('refetchDiffData', this.refetchDiffData); + eventHub.$off(EVT_VIEW_FILE_BY_FILE, this.fileByFileListener); + notesEventHub.$off('refetchDiffData', this.refetchDiffData); + notesEventHub.$off('fetchDiffData', this.fetchData); + this.removeEventListeners(); }, methods: { @@ -319,7 +329,11 @@ export default { 'scrollToFile', 'setShowTreeList', 'navigateToDiffFileIndex', + 'setFileByFile', ]), + fileByFileListener({ setting } = {}) { + this.setFileByFile({ fileByFile: setting }); + }, navigateToDiffFileNumber(number) { this.navigateToDiffFileIndex(number - 1); }, @@ -371,7 +385,7 @@ export default { } if (!this.isNotesFetched) { - eventHub.$emit('fetchNotesData'); + notesEventHub.$emit('fetchNotesData'); } }, setDiscussions() { diff --git a/app/assets/javascripts/diffs/components/settings_dropdown.vue b/app/assets/javascripts/diffs/components/settings_dropdown.vue index 590b2127e6bae690f5f56ade63e9bd11018ecde8..b8904de8049287673454c61a653e8b10ac8d9396 100644 --- a/app/assets/javascripts/diffs/components/settings_dropdown.vue +++ b/app/assets/javascripts/diffs/components/settings_dropdown.vue @@ -1,16 +1,38 @@ @@ -84,5 +109,10 @@ export default { {{ __('Show whitespace changes') }} +
+ + {{ $options.i18n.fileByFile }} + +
diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js index 79f8c08e389d01e1689aac07052789e357914ea7..07e27bd8e473246b02f8e0a1d23653c3d2510e0a 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -77,6 +77,11 @@ export const ALERT_COLLAPSED_FILES = 'collapsed'; export const DIFF_FILE_AUTOMATIC_COLLAPSE = 'automatic'; export const DIFF_FILE_MANUAL_COLLAPSE = 'manual'; +// Diff view single file mode +export const DIFF_FILE_BY_FILE_COOKIE_NAME = 'fileViewMode'; +export const DIFF_VIEW_FILE_BY_FILE = 'single'; +export const DIFF_VIEW_ALL_FILES = 'all'; + // State machine states export const STATE_IDLING = 'idle'; export const STATE_LOADING = 'loading'; @@ -98,6 +103,7 @@ export const RENAMED_DIFF_TRANSITIONS = { // MR Diffs known events export const EVT_EXPAND_ALL_FILES = 'mr:diffs:expandAllFiles'; +export const EVT_VIEW_FILE_BY_FILE = 'mr:diffs:preference:fileByFile'; export const EVT_PERF_MARK_FILE_TREE_START = 'mr:diffs:perf:fileTreeStart'; export const EVT_PERF_MARK_FILE_TREE_END = 'mr:diffs:perf:fileTreeEnd'; export const EVT_PERF_MARK_DIFF_FILES_START = 'mr:diffs:perf:filesStart'; diff --git a/app/assets/javascripts/diffs/i18n.js b/app/assets/javascripts/diffs/i18n.js index 4ec24d452bf4ad0d0eb4703c69f6090087f6893a..c4ac99ead9152ef6cd52ad8121b030710e3a5b30 100644 --- a/app/assets/javascripts/diffs/i18n.js +++ b/app/assets/javascripts/diffs/i18n.js @@ -16,3 +16,7 @@ export const DIFF_FILE = { autoCollapsed: __('Files with large changes are collapsed by default.'), expand: __('Expand file'), }; + +export const SETTINGS_DROPDOWN = { + fileByFile: __('Show one file at a time'), +}; diff --git a/app/assets/javascripts/diffs/index.js b/app/assets/javascripts/diffs/index.js index 06a138b1e13e7bd846db215549c1fb12abd6eb41..587220488beea00d046969b7761aa7c483f435c2 100644 --- a/app/assets/javascripts/diffs/index.js +++ b/app/assets/javascripts/diffs/index.js @@ -116,7 +116,7 @@ export default function initDiffsApp(store) { isFluidLayout: this.isFluidLayout, dismissEndpoint: this.dismissEndpoint, showSuggestPopover: this.showSuggestPopover, - viewDiffsFileByFile: this.viewDiffsFileByFile, + fileByFileUserPreference: this.viewDiffsFileByFile, }, }); }, diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 1a0e65bbb3eb90f1b87254a6c3e57a3cf41ec630..51d9fe12102b9a507b4f878e3f6a5148924f1c3c 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -44,6 +44,9 @@ import { EVT_PERF_MARK_FILE_TREE_START, EVT_PERF_MARK_FILE_TREE_END, EVT_PERF_MARK_DIFF_FILES_START, + DIFF_VIEW_FILE_BY_FILE, + DIFF_VIEW_ALL_FILES, + DIFF_FILE_BY_FILE_COOKIE_NAME, } from '../constants'; import { diffViewerModes } from '~/ide/constants'; import { isCollapsed } from '../diff_file'; @@ -57,6 +60,7 @@ export const setBaseConfig = ({ commit }, options) => { projectPath, dismissEndpoint, showSuggestPopover, + viewDiffsFileByFile, } = options; commit(types.SET_BASE_CONFIG, { endpoint, @@ -66,6 +70,7 @@ export const setBaseConfig = ({ commit }, options) => { projectPath, dismissEndpoint, showSuggestPopover, + viewDiffsFileByFile, }); }; @@ -694,3 +699,14 @@ export const navigateToDiffFileIndex = ({ commit, state }, index) => { commit(types.VIEW_DIFF_FILE, fileHash); }; + +export const setFileByFile = ({ commit }, { fileByFile }) => { + const fileViewMode = fileByFile ? DIFF_VIEW_FILE_BY_FILE : DIFF_VIEW_ALL_FILES; + commit(types.SET_FILE_BY_FILE, fileByFile); + + Cookies.set(DIFF_FILE_BY_FILE_COOKIE_NAME, fileViewMode); + + historyPushState( + mergeUrlParams({ [DIFF_FILE_BY_FILE_COOKIE_NAME]: fileViewMode }, window.location.href), + ); +}; diff --git a/app/assets/javascripts/diffs/store/modules/diff_state.js b/app/assets/javascripts/diffs/store/modules/diff_state.js index 001d9d9f5947d9eaf15e87d751ac17b3b1d722e0..c331e52c887ebd83f1e9af50b91d280564494d41 100644 --- a/app/assets/javascripts/diffs/store/modules/diff_state.js +++ b/app/assets/javascripts/diffs/store/modules/diff_state.js @@ -5,6 +5,8 @@ import { DIFF_VIEW_COOKIE_NAME, DIFF_WHITESPACE_COOKIE_NAME, } from '../../constants'; + +import { fileByFile } from '../../utils/preferences'; import { getDefaultWhitespace } from '../utils'; const viewTypeFromQueryString = getParameterValues('view')[0]; @@ -39,6 +41,7 @@ export default () => ({ highlightedRow: null, renderTreeList: true, showWhitespace: getDefaultWhitespace(whiteSpaceFromQueryString, whiteSpaceFromCookie), + viewDiffsFileByFile: fileByFile(), fileFinderVisible: false, dismissEndpoint: '', showSuggestPopover: true, diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index ed694419ab1100f845d8e25763561c35caa75064..251840287993ee7136e53c2c9e45297ce024102f 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -28,6 +28,7 @@ export const SET_HIGHLIGHTED_ROW = 'SET_HIGHLIGHTED_ROW'; export const SET_TREE_DATA = 'SET_TREE_DATA'; export const SET_RENDER_TREE_LIST = 'SET_RENDER_TREE_LIST'; export const SET_SHOW_WHITESPACE = 'SET_SHOW_WHITESPACE'; +export const SET_FILE_BY_FILE = 'SET_FILE_BY_FILE'; export const TOGGLE_FILE_FINDER_VISIBLE = 'TOGGLE_FILE_FINDER_VISIBLE'; export const REQUEST_FULL_DIFF = 'REQUEST_FULL_DIFF'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 7b08c5e75e1353e747a12641e6879abce2e44e74..69ae3f705e3326c1ba543f6d4833132c43aacafa 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -36,6 +36,7 @@ export default { projectPath, dismissEndpoint, showSuggestPopover, + viewDiffsFileByFile, } = options; Object.assign(state, { endpoint, @@ -45,6 +46,7 @@ export default { projectPath, dismissEndpoint, showSuggestPopover, + viewDiffsFileByFile, }); }, @@ -352,4 +354,7 @@ export default { [types.SET_SHOW_SUGGEST_POPOVER](state) { state.showSuggestPopover = false; }, + [types.SET_FILE_BY_FILE](state, fileByFile) { + state.viewDiffsFileByFile = fileByFile; + }, }; diff --git a/app/assets/javascripts/diffs/utils/preferences.js b/app/assets/javascripts/diffs/utils/preferences.js new file mode 100644 index 0000000000000000000000000000000000000000..e440de3350ae0d37ccffb344e60d88b2d2ebb9cc --- /dev/null +++ b/app/assets/javascripts/diffs/utils/preferences.js @@ -0,0 +1,22 @@ +import Cookies from 'js-cookie'; +import { getParameterValues } from '~/lib/utils/url_utility'; + +import { DIFF_FILE_BY_FILE_COOKIE_NAME, DIFF_VIEW_FILE_BY_FILE } from '../constants'; + +export function fileByFile(pref = false) { + const search = getParameterValues(DIFF_FILE_BY_FILE_COOKIE_NAME)?.[0]; + const cookie = Cookies.get(DIFF_FILE_BY_FILE_COOKIE_NAME); + let viewFileByFile = pref; + + // use the cookie first, if it exists + if (cookie) { + viewFileByFile = cookie === DIFF_VIEW_FILE_BY_FILE; + } + + // the search parameter of the URL should override, if it exists + if (search) { + viewFileByFile = search === DIFF_VIEW_FILE_BY_FILE; + } + + return viewFileByFile; +} diff --git a/changelogs/unreleased/feature-mr-file-by-file.yml b/changelogs/unreleased/feature-mr-file-by-file.yml new file mode 100644 index 0000000000000000000000000000000000000000..9d0f17b90592c3c01326ba3068e2de840f0d87a8 --- /dev/null +++ b/changelogs/unreleased/feature-mr-file-by-file.yml @@ -0,0 +1,5 @@ +--- +title: Toggle File-By-File setting from the MR settings dropdown +merge_request: 47726 +author: +type: added diff --git a/doc/user/project/merge_requests/reviewing_and_managing_merge_requests.md b/doc/user/project/merge_requests/reviewing_and_managing_merge_requests.md index 4968945b3c7c3dc4b9840f43e2755619cd083da2..1dbee52f6db8b99f7b54ade5bd56527f9ef1ddcf 100644 --- a/doc/user/project/merge_requests/reviewing_and_managing_merge_requests.md +++ b/doc/user/project/merge_requests/reviewing_and_managing_merge_requests.md @@ -92,6 +92,17 @@ From there, when reviewing merge requests' **Changes** tab, you will see only on ![File-by-file diff navigation](img/file_by_file_v13_2.png) +From [GitLab 13.7](https://gitlab.com/gitlab-org/gitlab/-/issues/233898) onwards, if you want to change +this behavior, you can do so from your **User preferences** (as explained above) or directly in a +merge request: + +1. Go to the merge request's **Changes** tab. +1. Click the cog icon (**{settings}**) to reveal the merge request's settings dropdown. +1. Select or unselect the checkbox **Show one file at a time** to change the setting accordingly. + +This change overrides the choice you made in your user preferences and persists until you clear your +browser's cookies or change this behavior again. + #### Enable or disable file-by-file diff navigation **(CORE ONLY)** File-by-file diff navigation is under development but ready for production use. It is diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b089c3e8e0a1c3c1c8d3010aca44f7659e1a80e5..0e469715559c897fd3f89787fda164f32f5a69e1 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -25056,6 +25056,9 @@ msgstr "" msgid "Show me the basics" msgstr "" +msgid "Show one file at a time" +msgstr "" + msgid "Show only direct members" msgstr "" diff --git a/spec/frontend/diffs/components/app_spec.js b/spec/frontend/diffs/components/app_spec.js index 01c789270dacbc122cf351b957e66758a20e1b28..416564b72c39d7b896e7d737efe58690ddf5edb8 100644 --- a/spec/frontend/diffs/components/app_spec.js +++ b/spec/frontend/diffs/components/app_spec.js @@ -17,10 +17,14 @@ import axios from '~/lib/utils/axios_utils'; import * as urlUtils from '~/lib/utils/url_utility'; import diffsMockData from '../mock_data/merge_request_diffs'; +import { EVT_VIEW_FILE_BY_FILE } from '~/diffs/constants'; + +import eventHub from '~/diffs/event_hub'; + const mergeRequestDiff = { version_index: 1 }; const TEST_ENDPOINT = `${TEST_HOST}/diff/endpoint`; -const COMMIT_URL = '[BASE URL]/OLD'; -const UPDATED_COMMIT_URL = '[BASE URL]/NEW'; +const COMMIT_URL = `${TEST_HOST}/COMMIT/OLD`; +const UPDATED_COMMIT_URL = `${TEST_HOST}/COMMIT/NEW`; function getCollapsedFilesWarning(wrapper) { return wrapper.find(CollapsedFilesWarning); @@ -61,7 +65,7 @@ describe('diffs/components/app', () => { changesEmptyStateIllustration: '', dismissEndpoint: '', showSuggestPopover: true, - viewDiffsFileByFile: false, + fileByFileUserPreference: false, ...props, }, provide, @@ -700,12 +704,14 @@ describe('diffs/components/app', () => { }); describe('file-by-file', () => { - it('renders a single diff', () => { - createComponent({ viewDiffsFileByFile: true }, ({ state }) => { + it('renders a single diff', async () => { + createComponent({ fileByFileUserPreference: true }, ({ state }) => { state.diffs.diffFiles.push({ file_hash: '123' }); state.diffs.diffFiles.push({ file_hash: '312' }); }); + await wrapper.vm.$nextTick(); + expect(wrapper.findAll(DiffFile).length).toBe(1); }); @@ -713,31 +719,37 @@ describe('diffs/components/app', () => { const fileByFileNav = () => wrapper.find('[data-testid="file-by-file-navigation"]'); const paginator = () => fileByFileNav().find(GlPagination); - it('sets previous button as disabled', () => { - createComponent({ viewDiffsFileByFile: true }, ({ state }) => { + it('sets previous button as disabled', async () => { + createComponent({ fileByFileUserPreference: true }, ({ state }) => { state.diffs.diffFiles.push({ file_hash: '123' }, { file_hash: '312' }); }); + await wrapper.vm.$nextTick(); + expect(paginator().attributes('prevpage')).toBe(undefined); expect(paginator().attributes('nextpage')).toBe('2'); }); - it('sets next button as disabled', () => { - createComponent({ viewDiffsFileByFile: true }, ({ state }) => { + it('sets next button as disabled', async () => { + createComponent({ fileByFileUserPreference: true }, ({ state }) => { state.diffs.diffFiles.push({ file_hash: '123' }, { file_hash: '312' }); state.diffs.currentDiffFileId = '312'; }); + await wrapper.vm.$nextTick(); + expect(paginator().attributes('prevpage')).toBe('1'); expect(paginator().attributes('nextpage')).toBe(undefined); }); - it("doesn't display when there's fewer than 2 files", () => { - createComponent({ viewDiffsFileByFile: true }, ({ state }) => { + it("doesn't display when there's fewer than 2 files", async () => { + createComponent({ fileByFileUserPreference: true }, ({ state }) => { state.diffs.diffFiles.push({ file_hash: '123' }); state.diffs.currentDiffFileId = '123'; }); + await wrapper.vm.$nextTick(); + expect(fileByFileNav().exists()).toBe(false); }); @@ -748,11 +760,13 @@ describe('diffs/components/app', () => { `( 'it calls navigateToDiffFileIndex with $index when $link is clicked', async ({ currentDiffFileId, targetFile }) => { - createComponent({ viewDiffsFileByFile: true }, ({ state }) => { + createComponent({ fileByFileUserPreference: true }, ({ state }) => { state.diffs.diffFiles.push({ file_hash: '123' }, { file_hash: '312' }); state.diffs.currentDiffFileId = currentDiffFileId; }); + await wrapper.vm.$nextTick(); + jest.spyOn(wrapper.vm, 'navigateToDiffFileIndex'); paginator().vm.$emit('input', targetFile); @@ -763,5 +777,24 @@ describe('diffs/components/app', () => { }, ); }); + + describe('control via event stream', () => { + it.each` + setting + ${true} + ${false} + `( + 'triggers the action with the new fileByFile setting - $setting - when the event with that setting is received', + async ({ setting }) => { + createComponent(); + await wrapper.vm.$nextTick(); + + eventHub.$emit(EVT_VIEW_FILE_BY_FILE, { setting }); + await wrapper.vm.$nextTick(); + + expect(store.state.diffs.viewDiffsFileByFile).toBe(setting); + }, + ); + }); }); }); diff --git a/spec/frontend/diffs/components/settings_dropdown_spec.js b/spec/frontend/diffs/components/settings_dropdown_spec.js index 72330d8efba93ccb582b93443447287d99faedba..a9b1c81d00e88602dfd0567bec8aee4e438e3907 100644 --- a/spec/frontend/diffs/components/settings_dropdown_spec.js +++ b/spec/frontend/diffs/components/settings_dropdown_spec.js @@ -2,12 +2,18 @@ import { mount, createLocalVue } from '@vue/test-utils'; import Vuex from 'vuex'; import diffModule from '~/diffs/store/modules'; import SettingsDropdown from '~/diffs/components/settings_dropdown.vue'; -import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '~/diffs/constants'; +import { + EVT_VIEW_FILE_BY_FILE, + PARALLEL_DIFF_VIEW_TYPE, + INLINE_DIFF_VIEW_TYPE, +} from '~/diffs/constants'; +import eventHub from '~/diffs/event_hub'; const localVue = createLocalVue(); localVue.use(Vuex); describe('Diff settings dropdown component', () => { + let wrapper; let vm; let actions; @@ -25,10 +31,15 @@ describe('Diff settings dropdown component', () => { extendStore(store); - vm = mount(SettingsDropdown, { + wrapper = mount(SettingsDropdown, { localVue, store, }); + vm = wrapper.vm; + } + + function getFileByFileCheckbox(vueWrapper) { + return vueWrapper.find('[data-testid="file-by-file"]'); } beforeEach(() => { @@ -41,14 +52,14 @@ describe('Diff settings dropdown component', () => { }); afterEach(() => { - vm.destroy(); + wrapper.destroy(); }); describe('tree view buttons', () => { it('list view button dispatches setRenderTreeList with false', () => { createComponent(); - vm.find('.js-list-view').trigger('click'); + wrapper.find('.js-list-view').trigger('click'); expect(actions.setRenderTreeList).toHaveBeenCalledWith(expect.anything(), false); }); @@ -56,7 +67,7 @@ describe('Diff settings dropdown component', () => { it('tree view button dispatches setRenderTreeList with true', () => { createComponent(); - vm.find('.js-tree-view').trigger('click'); + wrapper.find('.js-tree-view').trigger('click'); expect(actions.setRenderTreeList).toHaveBeenCalledWith(expect.anything(), true); }); @@ -68,8 +79,8 @@ describe('Diff settings dropdown component', () => { }); }); - expect(vm.find('.js-list-view').classes('selected')).toBe(true); - expect(vm.find('.js-tree-view').classes('selected')).toBe(false); + expect(wrapper.find('.js-list-view').classes('selected')).toBe(true); + expect(wrapper.find('.js-tree-view').classes('selected')).toBe(false); }); it('sets tree button as selected when renderTreeList is true', () => { @@ -79,8 +90,8 @@ describe('Diff settings dropdown component', () => { }); }); - expect(vm.find('.js-list-view').classes('selected')).toBe(false); - expect(vm.find('.js-tree-view').classes('selected')).toBe(true); + expect(wrapper.find('.js-list-view').classes('selected')).toBe(false); + expect(wrapper.find('.js-tree-view').classes('selected')).toBe(true); }); }); @@ -92,8 +103,8 @@ describe('Diff settings dropdown component', () => { }); }); - expect(vm.find('.js-inline-diff-button').classes('selected')).toBe(true); - expect(vm.find('.js-parallel-diff-button').classes('selected')).toBe(false); + expect(wrapper.find('.js-inline-diff-button').classes('selected')).toBe(true); + expect(wrapper.find('.js-parallel-diff-button').classes('selected')).toBe(false); }); it('sets parallel button as selected', () => { @@ -103,14 +114,14 @@ describe('Diff settings dropdown component', () => { }); }); - expect(vm.find('.js-inline-diff-button').classes('selected')).toBe(false); - expect(vm.find('.js-parallel-diff-button').classes('selected')).toBe(true); + expect(wrapper.find('.js-inline-diff-button').classes('selected')).toBe(false); + expect(wrapper.find('.js-parallel-diff-button').classes('selected')).toBe(true); }); it('calls setInlineDiffViewType when clicking inline button', () => { createComponent(); - vm.find('.js-inline-diff-button').trigger('click'); + wrapper.find('.js-inline-diff-button').trigger('click'); expect(actions.setInlineDiffViewType).toHaveBeenCalled(); }); @@ -118,7 +129,7 @@ describe('Diff settings dropdown component', () => { it('calls setParallelDiffViewType when clicking parallel button', () => { createComponent(); - vm.find('.js-parallel-diff-button').trigger('click'); + wrapper.find('.js-parallel-diff-button').trigger('click'); expect(actions.setParallelDiffViewType).toHaveBeenCalled(); }); @@ -132,7 +143,7 @@ describe('Diff settings dropdown component', () => { }); }); - expect(vm.find('#show-whitespace').element.checked).toBe(false); + expect(wrapper.find('#show-whitespace').element.checked).toBe(false); }); it('sets as checked when showWhitespace is true', () => { @@ -142,13 +153,13 @@ describe('Diff settings dropdown component', () => { }); }); - expect(vm.find('#show-whitespace').element.checked).toBe(true); + expect(wrapper.find('#show-whitespace').element.checked).toBe(true); }); it('calls setShowWhitespace on change', () => { createComponent(); - const checkbox = vm.find('#show-whitespace'); + const checkbox = wrapper.find('#show-whitespace'); checkbox.element.checked = true; checkbox.trigger('change'); @@ -159,4 +170,52 @@ describe('Diff settings dropdown component', () => { }); }); }); + + describe('file-by-file toggle', () => { + beforeEach(() => { + jest.spyOn(eventHub, '$emit'); + }); + + it.each` + fileByFile | checked + ${true} | ${true} + ${false} | ${false} + `( + 'sets { checked: $checked } if the fileByFile setting is $fileByFile', + async ({ fileByFile, checked }) => { + createComponent(store => { + Object.assign(store.state.diffs, { + viewDiffsFileByFile: fileByFile, + }); + }); + + await vm.$nextTick(); + + expect(vm.checked).toBe(checked); + }, + ); + + it.each` + start | emit + ${true} | ${false} + ${false} | ${true} + `( + 'when the file by file setting starts as $start, toggling the checkbox should emit an event set to $emit', + async ({ start, emit }) => { + createComponent(store => { + Object.assign(store.state.diffs, { + viewDiffsFileByFile: start, + }); + }); + + await vm.$nextTick(); + + getFileByFileCheckbox(wrapper).trigger('click'); + + await vm.$nextTick(); + + expect(eventHub.$emit).toHaveBeenCalledWith(EVT_VIEW_FILE_BY_FILE, { setting: emit }); + }, + ); + }); }); diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js index 0032fe926d0a48809dd002f72b7f55f99433bbb2..c1a7844d301ee8b4ec285af22e429e5cea107542 100644 --- a/spec/frontend/diffs/store/actions_spec.js +++ b/spec/frontend/diffs/store/actions_spec.js @@ -48,6 +48,7 @@ import { moveToNeighboringCommit, setCurrentDiffFileIdFromNote, navigateToDiffFileIndex, + setFileByFile, } from '~/diffs/store/actions'; import eventHub from '~/notes/event_hub'; import * as types from '~/diffs/store/mutation_types'; @@ -1455,4 +1456,20 @@ describe('DiffsStoreActions', () => { ); }); }); + + describe('setFileByFile', () => { + it.each` + value + ${true} + ${false} + `('commits SET_FILE_BY_FILE with the new value $value', ({ value }) => { + return testAction( + setFileByFile, + { fileByFile: value }, + { viewDiffsFileByFile: null }, + [{ type: types.SET_FILE_BY_FILE, payload: value }], + [], + ); + }); + }); }); diff --git a/spec/frontend/diffs/store/mutations_spec.js b/spec/frontend/diffs/store/mutations_spec.js index 2dda9a0ad7199dfa09d56a53c880251148b0bfa0..c061c5c25906c5b281f0bacef005d0062bcd830e 100644 --- a/spec/frontend/diffs/store/mutations_spec.js +++ b/spec/frontend/diffs/store/mutations_spec.js @@ -892,4 +892,18 @@ describe('DiffsStoreMutations', () => { expect(state.showSuggestPopover).toBe(false); }); }); + + describe('SET_FILE_BY_FILE', () => { + it.each` + value | opposite + ${true} | ${false} + ${false} | ${true} + `('sets viewDiffsFileByFile to $value', ({ value, opposite }) => { + const state = { viewDiffsFileByFile: opposite }; + + mutations[types.SET_FILE_BY_FILE](state, value); + + expect(state.viewDiffsFileByFile).toBe(value); + }); + }); }); diff --git a/spec/frontend/diffs/utils/preferences_spec.js b/spec/frontend/diffs/utils/preferences_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..a48db1d751240930a89d545c612deaedf07e183a --- /dev/null +++ b/spec/frontend/diffs/utils/preferences_spec.js @@ -0,0 +1,40 @@ +import Cookies from 'js-cookie'; +import { getParameterValues } from '~/lib/utils/url_utility'; + +import { fileByFile } from '~/diffs/utils/preferences'; +import { + DIFF_FILE_BY_FILE_COOKIE_NAME, + DIFF_VIEW_FILE_BY_FILE, + DIFF_VIEW_ALL_FILES, +} from '~/diffs/constants'; + +jest.mock('~/lib/utils/url_utility'); + +describe('diffs preferences', () => { + describe('fileByFile', () => { + it.each` + result | preference | cookie | searchParam + ${false} | ${false} | ${undefined} | ${undefined} + ${true} | ${true} | ${undefined} | ${undefined} + ${true} | ${false} | ${DIFF_VIEW_FILE_BY_FILE} | ${undefined} + ${false} | ${true} | ${DIFF_VIEW_ALL_FILES} | ${undefined} + ${true} | ${false} | ${undefined} | ${[DIFF_VIEW_FILE_BY_FILE]} + ${false} | ${true} | ${undefined} | ${[DIFF_VIEW_ALL_FILES]} + ${true} | ${false} | ${DIFF_VIEW_FILE_BY_FILE} | ${[DIFF_VIEW_FILE_BY_FILE]} + ${true} | ${true} | ${DIFF_VIEW_ALL_FILES} | ${[DIFF_VIEW_FILE_BY_FILE]} + ${false} | ${false} | ${DIFF_VIEW_ALL_FILES} | ${[DIFF_VIEW_ALL_FILES]} + ${false} | ${true} | ${DIFF_VIEW_FILE_BY_FILE} | ${[DIFF_VIEW_ALL_FILES]} + `( + 'should return $result when { preference: $preference, cookie: $cookie, search: $searchParam }', + ({ result, preference, cookie, searchParam }) => { + if (cookie) { + Cookies.set(DIFF_FILE_BY_FILE_COOKIE_NAME, cookie); + } + + getParameterValues.mockReturnValue(searchParam); + + expect(fileByFile(preference)).toBe(result); + }, + ); + }); +});