diff options
author | Phil Hughes <me@iamphill.com> | 2018-11-20 12:37:57 +0000 |
---|---|---|
committer | Phil Hughes <me@iamphill.com> | 2018-11-22 12:04:18 +0000 |
commit | 93c614ce9243c0743176a6e406a55f9dcb9bedf3 (patch) | |
tree | ada50b52e78855a19b7b931514448eee8dceecc0 | |
parent | 2e3dab38295b7c36ab100f20c654fdfaf9b00885 (diff) | |
download | gitlab-ce-93c614ce9243c0743176a6e406a55f9dcb9bedf3.tar.gz |
Show toggle tree button on merge request commits
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/53019
-rw-r--r-- | app/assets/javascripts/diffs/components/app.vue | 55 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/components/commit_widget.vue | 2 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/components/compare_versions.vue | 23 | ||||
-rw-r--r-- | app/assets/stylesheets/pages/issuable.scss | 1 | ||||
-rw-r--r-- | app/assets/stylesheets/pages/merge_requests.scss | 1 | ||||
-rw-r--r-- | changelogs/unreleased/mr-file-tree-commit.yml | 5 | ||||
-rw-r--r-- | locale/gitlab.pot | 15 | ||||
-rw-r--r-- | spec/features/merge_request/user_sees_versions_spec.rb | 2 | ||||
-rw-r--r-- | spec/javascripts/diffs/components/app_spec.js | 40 | ||||
-rw-r--r-- | spec/javascripts/diffs/components/compare_versions_spec.js | 21 |
10 files changed, 58 insertions, 107 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 0b7838292b2..22da38ce7a5 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -55,8 +55,6 @@ export default { diffViewType: state => state.diffs.diffViewType, mergeRequestDiffs: state => state.diffs.mergeRequestDiffs, mergeRequestDiff: state => state.diffs.mergeRequestDiff, - latestVersionPath: state => state.diffs.latestVersionPath, - startVersion: state => state.diffs.startVersion, commit: state => state.diffs.commit, targetBranchName: state => state.diffs.targetBranchName, renderOverflowWarning: state => state.diffs.renderOverflowWarning, @@ -75,24 +73,6 @@ export default { path: '', }; }, - notAllCommentsDisplayed() { - if (this.commit) { - return __('Only comments from the following commit are shown below'); - } else if (this.startVersion) { - return __( - "Not all comments are displayed because you're comparing two versions of the diff.", - ); - } - return __( - "Not all comments are displayed because you're viewing an old version of the diff.", - ); - }, - showLatestVersion() { - if (this.commit) { - return __('Show latest version of the diff'); - } - return __('Show latest version'); - }, canCurrentUserFork() { return this.currentUser.can_fork === true && this.currentUser.can_create_merge_request; }, @@ -184,10 +164,8 @@ export default { <div v-if="isLoading" class="loading"><gl-loading-icon /></div> <div v-else id="diffs" :class="{ active: shouldShow }" class="diffs tab-pane"> <compare-versions - v-if="showCompareVersions" :merge-request-diffs="mergeRequestDiffs" :merge-request-diff="mergeRequestDiff" - :start-version="startVersion" :target-branch="targetBranch" /> @@ -200,33 +178,22 @@ export default { /> <div - v-if="commit || startVersion || (mergeRequestDiff && !mergeRequestDiff.latest)" - class="mr-version-controls" - > - <div class="content-block comments-disabled-notif clearfix"> - <i class="fa fa-info-circle"></i> {{ notAllCommentsDisplayed }} - <div class="pull-right"> - <a :href="latestVersionPath" class="btn btn-sm"> {{ showLatestVersion }} </a> - </div> - </div> - </div> - - <commit-widget v-if="commit" :commit="commit" /> - - <div :data-can-create-note="getNoteableData.current_user.can_create_note" class="files d-flex prepend-top-default" > <div v-show="showTreeList" class="diff-tree-list"><tree-list /></div> - <div v-if="diffFiles.length > 0" class="diff-files-holder"> - <diff-file - v-for="file in diffFiles" - :key="file.newPath" - :file="file" - :can-current-user-fork="canCurrentUserFork" - /> + <div class="diff-files-holder"> + <commit-widget v-if="commit" :commit="commit" /> + <template v-if="diffFiles.length > 0"> + <diff-file + v-for="file in diffFiles" + :key="file.newPath" + :file="file" + :can-current-user-fork="canCurrentUserFork" + /> + </template> + <no-changes v-else /> </div> - <no-changes v-else /> </div> </div> </div> diff --git a/app/assets/javascripts/diffs/components/commit_widget.vue b/app/assets/javascripts/diffs/components/commit_widget.vue index 3fb11813d68..d45f91c7023 100644 --- a/app/assets/javascripts/diffs/components/commit_widget.vue +++ b/app/assets/javascripts/diffs/components/commit_widget.vue @@ -28,7 +28,7 @@ export default { </script> <template> - <div class="info-well prepend-top-default"> + <div class="info-well w-100"> <div class="well-segment"> <ul class="blob-commit-info"> <commit-item :commit="commit" /> diff --git a/app/assets/javascripts/diffs/components/compare_versions.vue b/app/assets/javascripts/diffs/components/compare_versions.vue index 3ccd111f1cb..f75345d31f8 100644 --- a/app/assets/javascripts/diffs/components/compare_versions.vue +++ b/app/assets/javascripts/diffs/components/compare_versions.vue @@ -1,6 +1,6 @@ <script> import { mapActions, mapGetters, mapState } from 'vuex'; -import { GlTooltipDirective } from '@gitlab/ui'; +import { GlTooltipDirective, GlLink, GlButton } from '@gitlab/ui'; import { __ } from '~/locale'; import { getParameterValues, mergeUrlParams } from '~/lib/utils/url_utility'; import Icon from '~/vue_shared/components/icon.vue'; @@ -10,6 +10,8 @@ export default { components: { CompareVersionsDropdown, Icon, + GlLink, + GlButton, }, directives: { GlTooltip: GlTooltipDirective, @@ -21,12 +23,8 @@ export default { }, mergeRequestDiff: { type: Object, - required: true, - }, - startVersion: { - type: Object, required: false, - default: null, + default: () => ({}), }, targetBranch: { type: Object, @@ -35,7 +33,7 @@ export default { }, }, computed: { - ...mapState('diffs', ['commit', 'showTreeList']), + ...mapState('diffs', ['commit', 'showTreeList', 'startVersion', 'latestVersionPath']), ...mapGetters('diffs', ['isInlineView', 'isParallelView', 'hasCollapsedFile']), comparableDiffs() { return this.mergeRequestDiffs.slice(1); @@ -102,7 +100,18 @@ export default { class="mr-version-compare-dropdown" /> </div> + <div v-else-if="commit"> + {{ __('Viewing commit') }} + <gl-link :href="commit.commit_url" class="monospace">{{ commit.short_id }}</gl-link> + </div> <div class="inline-parallel-buttons d-none d-md-flex ml-auto"> + <gl-button + v-if="commit || startVersion" + :href="latestVersionPath" + class="append-right-8 js-latest-version" + > + {{ __('Show latest version') }} + </gl-button> <a v-show="hasCollapsedFile" class="btn btn-default append-right-8" @click="expandAllFiles"> {{ __('Expand all') }} </a> diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 415583cc1c7..78c5ae9ae63 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -22,7 +22,6 @@ .detail-page-header, .page-content-header, .commit-box, - .info-well, .commit-ci-menu, .files-changed-inner, .limited-header-width, diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index fa6afbf81de..b075009b57c 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -717,6 +717,7 @@ .mr-version-menus-container { display: flex; align-items: center; + flex-wrap: wrap; padding: 16px; z-index: 199; white-space: nowrap; diff --git a/changelogs/unreleased/mr-file-tree-commit.yml b/changelogs/unreleased/mr-file-tree-commit.yml new file mode 100644 index 00000000000..e0d47e6e61f --- /dev/null +++ b/changelogs/unreleased/mr-file-tree-commit.yml @@ -0,0 +1,5 @@ +--- +title: Show tree collapse button for merge request commit diffs +merge_request: +author: +type: fixed diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 7af36b049d8..71504692b01 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4252,12 +4252,6 @@ msgstr "" msgid "None" msgstr "" -msgid "Not all comments are displayed because you're comparing two versions of the diff." -msgstr "" - -msgid "Not all comments are displayed because you're viewing an old version of the diff." -msgstr "" - msgid "Not allowed to merge" msgstr "" @@ -4395,9 +4389,6 @@ msgstr "" msgid "Only admins" msgstr "" -msgid "Only comments from the following commit are shown below" -msgstr "" - msgid "Only mirror protected branches" msgstr "" @@ -5764,9 +5755,6 @@ msgstr "" msgid "Show latest version" msgstr "" -msgid "Show latest version of the diff" -msgstr "" - msgid "Show parent pages" msgstr "" @@ -7053,6 +7041,9 @@ msgstr "" msgid "View the documentation" msgstr "" +msgid "Viewing commit" +msgstr "" + msgid "Visibility and access controls" msgstr "" diff --git a/spec/features/merge_request/user_sees_versions_spec.rb b/spec/features/merge_request/user_sees_versions_spec.rb index 92db4f44098..f7512294bef 100644 --- a/spec/features/merge_request/user_sees_versions_spec.rb +++ b/spec/features/merge_request/user_sees_versions_spec.rb @@ -66,7 +66,6 @@ describe 'Merge request > User sees versions', :js do it 'shows comments that were last relevant at that version' do expect(page).to have_content '5 changed files' - expect(page).to have_content 'Not all comments are displayed' position = Gitlab::Diff::Position.new( old_path: ".gitmodules", @@ -112,7 +111,6 @@ describe 'Merge request > User sees versions', :js do ) expect(page).to have_content '4 changed files' expect(page).to have_content '15 additions 6 deletions' - expect(page).to have_content 'Not all comments are displayed' position = Gitlab::Diff::Position.new( old_path: ".gitmodules", diff --git a/spec/javascripts/diffs/components/app_spec.js b/spec/javascripts/diffs/components/app_spec.js index 3c9b5ee0176..1a0b7612ee9 100644 --- a/spec/javascripts/diffs/components/app_spec.js +++ b/spec/javascripts/diffs/components/app_spec.js @@ -3,7 +3,6 @@ import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper import { TEST_HOST } from 'spec/test_constants'; import App from '~/diffs/components/app.vue'; import createDiffsStore from '../create_diffs_store'; -import getDiffWithCommit from '../mock_data/diff_with_commit'; describe('diffs/components/app', () => { const oldMrTabs = window.mrTabs; @@ -40,43 +39,4 @@ describe('diffs/components/app', () => { it('does not show commit info', () => { expect(vm.$el).not.toContainElement('.blob-commit-info'); }); - - it('shows comments message, with commit', done => { - vm.$store.state.diffs.commit = getDiffWithCommit().commit; - - vm.$nextTick() - .then(() => { - expect(vm.$el).toContainText('Only comments from the following commit are shown below'); - expect(vm.$el).toContainElement('.blob-commit-info'); - }) - .then(done) - .catch(done.fail); - }); - - it('shows comments message, with old mergeRequestDiff', done => { - vm.$store.state.diffs.mergeRequestDiff = { latest: false }; - vm.$store.state.diffs.targetBranch = 'master'; - - vm.$nextTick() - .then(() => { - expect(vm.$el).toContainText( - "Not all comments are displayed because you're viewing an old version of the diff.", - ); - }) - .then(done) - .catch(done.fail); - }); - - it('shows comments message, with startVersion', done => { - vm.$store.state.diffs.startVersion = 'test'; - - vm.$nextTick() - .then(() => { - expect(vm.$el).toContainText( - "Not all comments are displayed because you're comparing two versions of the diff.", - ); - }) - .then(done) - .catch(done.fail); - }); }); diff --git a/spec/javascripts/diffs/components/compare_versions_spec.js b/spec/javascripts/diffs/components/compare_versions_spec.js index d9d7f61785f..75c66e9ca82 100644 --- a/spec/javascripts/diffs/components/compare_versions_spec.js +++ b/spec/javascripts/diffs/components/compare_versions_spec.js @@ -3,6 +3,7 @@ import CompareVersionsComponent from '~/diffs/components/compare_versions.vue'; import store from '~/mr_notes/stores'; import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; import diffsMockData from '../mock_data/merge_request_diffs'; +import getDiffWithCommit from '../mock_data/diff_with_commit'; describe('CompareVersions', () => { let vm; @@ -122,4 +123,24 @@ describe('CompareVersions', () => { expect(vm.isWhitespaceVisible()).toBe(true); }); }); + + describe('commit', () => { + beforeEach(done => { + vm.$store.state.diffs.commit = getDiffWithCommit().commit; + vm.mergeRequestDiffs = []; + + vm.$nextTick(done); + }); + + it('renders latest version button', () => { + expect(vm.$el.querySelector('.js-latest-version').textContent.trim()).toBe( + 'Show latest version', + ); + }); + + it('renders short commit ID', () => { + expect(vm.$el.textContent).toContain('Viewing commit'); + expect(vm.$el.textContent).toContain(vm.commit.short_id); + }); + }); }); |