Merge branch 'ph/199725/diffHeaderActionDropdown' into 'master'

Move diff header actions into dropdown menu

Closes #199725

See merge request gitlab-org/gitlab!42483

app/assets/javascripts/diffs/components/diff_file_header.vue

@@ -2,32 +2,36 @@
 import { escape } from 'lodash';
 import { mapActions, mapGetters } from 'vuex';
 import {
-  GlDeprecatedButton,
   GlTooltipDirective,
   GlSafeHtmlDirective,
-  GlLoadingIcon,
   GlIcon,
   GlButton,
+  GlButtonGroup,
+  GlDropdown,
+  GlDropdownItem,
+  GlDropdownSectionHeader,
+  GlDropdownDivider,
 } from '@gitlab/ui';
 import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
 import FileIcon from '~/vue_shared/components/file_icon.vue';
 import { truncateSha } from '~/lib/utils/text_utility';
 import { __, s__, sprintf } from '~/locale';
 import { diffViewerModes } from '~/ide/constants';
-import EditButton from './edit_button.vue';
 import DiffStats from './diff_stats.vue';
 import { scrollToElement } from '~/lib/utils/common_utils';
 
 export default {
   components: {
-    GlLoadingIcon,
-    GlDeprecatedButton,
     ClipboardButton,
-    EditButton,
     GlIcon,
     FileIcon,
     DiffStats,
     GlButton,
+    GlButtonGroup,
+    GlDropdown,
+    GlDropdownItem,
+    GlDropdownSectionHeader,
+    GlDropdownDivider,
   },
   directives: {
     GlTooltip: GlTooltipDirective,
@@ -70,7 +74,7 @@ export default {
   },
   data() {
     return {
-      hasDropdownOpen: false,
+      moreActionsShown: false,
     };
   },
   computed: {
@@ -155,6 +159,13 @@ export default {
       }
       return s__('MRDiff|Show full file');
     },
+    showEditButton() {
+      return (
+        this.diffFile.blob?.readable_text &&
+        !this.diffFile.deleted_file &&
+        (this.diffFile.edit_path || this.diffFile.ide_edit_path)
+      );
+    },
   },
   methods: {
     ...mapActions('diffs', [
@@ -166,8 +177,11 @@ export default {
     handleToggleFile() {
       this.$emit('toggleFile');
     },
-    showForkMessage() {
-      this.$emit('showForkMessage');
+    showForkMessage(e) {
+      if (this.canCurrentUserFork && !this.diffFile.can_modify_blob) {
+        e.preventDefault();
+        this.$emit('showForkMessage');
+      }
     },
     handleFileNameClick(e) {
       const isLinkToOtherPage =
@@ -183,8 +197,8 @@ export default {
         }
       }
     },
-    setDropdownOpen(val) {
-      this.hasDropdownOpen = val;
+    setMoreActionsShown(val) {
+      this.moreActionsShown = val;
     },
   },
 };
@@ -193,8 +207,8 @@ export default {
 <template>
   <div
     ref="header"
+    :class="{ 'gl-z-dropdown-menu!': moreActionsShown }"
     class="js-file-title file-title file-title-flex-parent"
-    :class="{ 'gl-z-dropdown-menu!': hasDropdownOpen }"
     @click.self="handleToggleFile"
   >
     <div class="file-header-content">
@@ -255,96 +269,95 @@ export default {
 
     <div
       v-if="!diffFile.submodule && addMergeRequestButtons"
-      class="file-actions d-none d-sm-flex align-items-center flex-wrap"
+      class="file-actions d-flex align-items-center flex-wrap"
     >
       <diff-stats :added-lines="diffFile.added_lines" :removed-lines="diffFile.removed_lines" />
-      <div class="btn-group" role="group">
-        <template v-if="diffFile.blob && diffFile.blob.readable_text">
-          <span v-gl-tooltip.hover :title="s__('MergeRequests|Toggle comments for this file')">
-            <gl-deprecated-button
-              ref="toggleDiscussionsButton"
-              :disabled="!diffHasDiscussions(diffFile)"
-              :class="{ active: diffHasExpandedDiscussions(diffFile) }"
-              class="js-btn-vue-toggle-comments btn"
-              data-qa-selector="toggle_comments_button"
-              data-track-event="click_toggle_comments_button"
-              data-track-label="diff_toggle_comments_button"
-              data-track-property="diff_toggle_comments"
-              type="button"
-              @click="toggleFileDiscussionWrappers(diffFile)"
-            >
-              <gl-icon name="comment" />
-            </gl-deprecated-button>
-          </span>
-
-          <edit-button
-            v-if="!diffFile.deleted_file"
-            :can-current-user-fork="canCurrentUserFork"
-            :edit-path="diffFile.edit_path"
-            :ide-edit-path="diffFile.ide_edit_path"
-            :can-modify-blob="diffFile.can_modify_blob"
-            data-track-event="click_toggle_edit_button"
-            data-track-label="diff_toggle_edit_button"
-            data-track-property="diff_toggle_edit"
-            @showForkMessage="showForkMessage"
-            @open="setDropdownOpen(true)"
-            @close="setDropdownOpen(false)"
-          />
-        </template>
-
-        <a
-          v-if="diffFile.replaced_view_path"
-          ref="replacedFileButton"
-          v-safe-html="viewReplacedFileButtonText"
-          :href="diffFile.replaced_view_path"
-          class="btn view-file"
-        >
-        </a>
-        <gl-deprecated-button
-          v-if="!diffFile.is_fully_expanded"
-          ref="expandDiffToFullFileButton"
-          v-gl-tooltip.hover
-          :title="expandDiffToFullFileTitle"
-          class="expand-file"
-          data-track-event="click_toggle_view_full_button"
-          data-track-label="diff_toggle_view_full_button"
-          data-track-property="diff_toggle_view_full"
-          @click="toggleFullDiff(diffFile.file_path)"
-        >
-          <gl-loading-icon v-if="diffFile.isLoadingFullFile" color="dark" inline />
-          <gl-icon v-else-if="diffFile.isShowingFullFile" name="doc-changes" />
-          <gl-icon v-else name="doc-expand" />
-        </gl-deprecated-button>
-        <gl-deprecated-button
-          ref="viewButton"
-          v-gl-tooltip.hover
-          :href="diffFile.view_path"
-          target="_blank"
-          class="view-file"
-          data-track-event="click_toggle_view_sha_button"
-          data-track-label="diff_toggle_view_sha_button"
-          data-track-property="diff_toggle_view_sha"
-          :title="viewFileButtonText"
-        >
-          <gl-icon name="doc-text" />
-        </gl-deprecated-button>
-
-        <a
+      <gl-button-group class="gl-pt-0!">
+        <gl-button
           v-if="diffFile.external_url"
           ref="externalLink"
           v-gl-tooltip.hover
           :href="diffFile.external_url"
           :title="`View on ${diffFile.formatted_external_url}`"
           target="_blank"
-          rel="noopener noreferrer"
           data-track-event="click_toggle_external_button"
           data-track-label="diff_toggle_external_button"
           data-track-property="diff_toggle_external"
-          class="btn btn-file-option"
+          icon="external-link"
+        />
+        <gl-dropdown
+          v-gl-tooltip.hover.focus="__('More actions')"
+          right
+          toggle-class="btn-icon js-diff-more-actions"
+          class="gl-pt-0!"
+          @show="setMoreActionsShown(true)"
+          @hidden="setMoreActionsShown(false)"
         >
-          <gl-icon name="external-link" />
-        </a>
-      </div>
+          <template #button-content>
+            <gl-icon name="ellipsis_v" class="mr-0" />
+            <span class="sr-only">{{ __('More actions') }}</span>
+          </template>
+          <gl-dropdown-section-header>
+            {{ __('More actions') }}
+          </gl-dropdown-section-header>
+          <gl-dropdown-item
+            v-if="diffFile.replaced_view_path"
+            ref="replacedFileButton"
+            v-safe-html="viewReplacedFileButtonText"
+            :href="diffFile.replaced_view_path"
+            target="_blank"
+          />
+          <gl-dropdown-item ref="viewButton" :href="diffFile.view_path" target="_blank">
+            {{ viewFileButtonText }}
+          </gl-dropdown-item>
+          <template v-if="showEditButton">
+            <gl-dropdown-item
+              v-if="diffFile.edit_path"
+              ref="editButton"
+              :href="diffFile.edit_path"
+              class="js-edit-blob"
+              @click="showForkMessage"
+            >
+              {{ __('Edit in single-file editor') }}
+            </gl-dropdown-item>
+            <gl-dropdown-item
+              v-if="diffFile.edit_path"
+              ref="ideEditButton"
+              :href="diffFile.ide_edit_path"
+              class="js-ide-edit-blob"
+            >
+              {{ __('Edit in Web IDE') }}
+            </gl-dropdown-item>
+          </template>
+
+          <template v-if="!diffFile.viewer.collapsed">
+            <gl-dropdown-divider
+              v-if="!diffFile.is_fully_expanded || diffHasDiscussions(diffFile)"
+            />
+
+            <gl-dropdown-item
+              v-if="diffHasDiscussions(diffFile)"
+              ref="toggleDiscussionsButton"
+              data-qa-selector="toggle_comments_button"
+              @click="toggleFileDiscussionWrappers(diffFile)"
+            >
+              <template v-if="diffHasExpandedDiscussions(diffFile)">
+                {{ __('Hide comments on this file') }}
+              </template>
+              <template v-else>
+                {{ __('Show comments on this file') }}
+              </template>
+            </gl-dropdown-item>
+            <gl-dropdown-item
+              v-if="!diffFile.is_fully_expanded"
+              ref="expandDiffToFullFileButton"
+              @click="toggleFullDiff(diffFile.file_path)"
+            >
+              {{ expandDiffToFullFileTitle }}
+            </gl-dropdown-item>
+          </template>
+        </gl-dropdown>
+      </gl-button-group>
     </div>
 
     <div

app/assets/javascripts/diffs/components/diff_stats.vue

@@ -42,7 +42,7 @@ export default {
     class="diff-stats"
     :class="{
       'is-compare-versions-header d-none d-lg-inline-flex': isCompareVersionsHeader,
-      'd-inline-flex': !isCompareVersionsHeader,
+      'd-none d-sm-inline-flex': !isCompareVersionsHeader,
     }"
   >
     <div v-if="hasDiffFiles" class="diff-stats-group">

app/assets/javascripts/diffs/components/edit_button.vue

-<script>
-import { uniqueId } from 'lodash';
-import {
-  GlTooltipDirective,
-  GlIcon,
-  GlDeprecatedDropdown as GlDropdown,
-  GlDeprecatedDropdownItem as GlDropdownItem,
-} from '@gitlab/ui';
-import { __ } from '~/locale';
-
-export default {
-  components: {
-    GlDropdown,
-    GlDropdownItem,
-    GlIcon,
-  },
-  directives: {
-    GlTooltip: GlTooltipDirective,
-  },
-  props: {
-    editPath: {
-      type: String,
-      required: false,
-      default: '',
-    },
-    ideEditPath: {
-      type: String,
-      required: false,
-      default: '',
-    },
-    canCurrentUserFork: {
-      type: Boolean,
-      required: true,
-    },
-    canModifyBlob: {
-      type: Boolean,
-      required: false,
-      default: false,
-    },
-  },
-  data() {
-    return { tooltipId: uniqueId('edit_button_tooltip_') };
-  },
-  computed: {
-    tooltipTitle() {
-      if (this.isDisabled) {
-        return __("Can't edit as source branch was deleted");
-      }
-
-      return __('Edit file in...');
-    },
-    isDisabled() {
-      return !this.editPath;
-    },
-  },
-  methods: {
-    handleShow(evt) {
-      // We must hide the tooltip because it is redundant and doesn't close itself
-      // when dropdown opens because we are still "focused".
-      this.$root.$emit('bv::hide::tooltip', this.tooltipId);
-
-      if (this.canCurrentUserFork && !this.canModifyBlob) {
-        evt.preventDefault();
-        this.$emit('showForkMessage');
-      } else {
-        this.$emit('open');
-      }
-    },
-    handleHide() {
-      this.$emit('close');
-    },
-  },
-};
-</script>
-
-<template>
-  <div v-gl-tooltip.top="{ title: tooltipTitle, id: tooltipId }" class="gl-display-flex">
-    <gl-dropdown
-      toggle-class="rounded-0"
-      :disabled="isDisabled"
-      :class="{ 'cursor-not-allowed': isDisabled }"
-      right
-      data-testid="edit_file"
-      @show="handleShow"
-      @hide="handleHide"
-    >
-      <template #button-content>
-        <span class="gl-dropdown-toggle-text"><gl-icon name="pencil"/></span>
-        <gl-icon class="gl-dropdown-caret" name="chevron-down" aria-hidden="true" />
-      </template>
-      <gl-dropdown-item v-if="editPath" :href="editPath">{{
-        __('Edit in single-file editor')
-      }}</gl-dropdown-item>
-      <gl-dropdown-item v-if="ideEditPath" :href="ideEditPath">{{
-        __('Edit in Web IDE')
-      }}</gl-dropdown-item>
-    </gl-dropdown>
-  </div>
-</template>

app/assets/javascripts/diffs/store/getters.js

@@ -46,15 +46,24 @@ export const diffHasAllCollapsedDiscussions = (state, getters) => diff => {
  * @param {Object} diff
  * @returns {Boolean}
  */
-export const diffHasExpandedDiscussions = (state, getters) => diff => {
-  const discussions = getters.getDiffFileDiscussions(diff);
-
-  return (
-    (discussions &&
-      discussions.length &&
-      discussions.find(discussion => discussion.expanded) !== undefined) ||
-    false
-  );
+export const diffHasExpandedDiscussions = state => diff => {
+  const lines = {
+    [INLINE_DIFF_VIEW_TYPE]: diff.highlighted_diff_lines || [],
+    [PARALLEL_DIFF_VIEW_TYPE]: (diff.parallel_diff_lines || []).reduce((acc, line) => {
+      if (line.left) {
+        acc.push(line.left);
+      }
+
+      if (line.right) {
+        acc.push(line.right);
+      }
+
+      return acc;
+    }, []),
+  };
+  return lines[window.gon?.features?.unifiedDiffLines ? 'inline' : state.diffViewType]
+    .filter(l => l.discussions.length >= 1)
+    .some(l => l.discussionsExpanded);
 };
 
 /**
@@ -62,8 +71,25 @@ export const diffHasExpandedDiscussions = (state, getters) => diff => {
  * @param {Boolean} diff
  * @returns {Boolean}
  */
-export const diffHasDiscussions = (state, getters) => diff =>
-  getters.getDiffFileDiscussions(diff).length > 0;
+export const diffHasDiscussions = state => diff => {
+  const lines = {
+    [INLINE_DIFF_VIEW_TYPE]: diff.highlighted_diff_lines || [],
+    [PARALLEL_DIFF_VIEW_TYPE]: (diff.parallel_diff_lines || []).reduce((acc, line) => {
+      if (line.left) {
+        acc.push(line.left);
+      }
+
+      if (line.right) {
+        acc.push(line.right);
+      }
+
+      return acc;
+    }, []),
+  };
+  return lines[window.gon?.features?.unifiedDiffLines ? 'inline' : state.diffViewType].some(
+    l => l.discussions.length >= 1,
+  );
+};
 
 /**
  * Returns an array with the discussions of the given diff

app/assets/stylesheets/framework/files.scss

@@ -50,7 +50,7 @@
       right: 15px;
       margin-left: auto;
 
-      .btn {
+      .btn:not(.btn-icon) {
         padding: 0 10px;
         font-size: 13px;
         line-height: 28px;
@@ -372,7 +372,7 @@ span.idiff {
     color: $gl-text-color;
   }
 
-  .file-actions .btn {
+  .file-actions .btn:not(.btn-icon) {
     padding: 0 10px;
     font-size: 13px;
     line-height: 28px;

app/assets/stylesheets/pages/diff.scss

@@ -13,6 +13,21 @@
     box-shadow: 0 -2px 0 0 var(--white);
     cursor: pointer;
 
+    .dropdown-menu {
+      cursor: auto;
+    }
+
+    @media (max-width: map-get($grid-breakpoints, sm)-1) {
+      .file-header-content {
+        width: 0;
+        flex: 1;
+      }
+
+      .file-actions {
+        margin-left: $gl-spacing-scale-2;
+      }
+    }
+
     @media (min-width: map-get($grid-breakpoints, md)) {
       // The `+11` is to ensure the file header border shows when scrolled -
       // the bottom of the compare-versions header and the top of the file header
@@ -420,6 +435,10 @@
     margin-left: 0;
     border-left: 0;
   }
+
+  .file-actions .dropdown {
+    height: 28px;
+  }
 }
 
 table.code {

changelogs/unreleased/ph-199725-diffHeaderActionDropdown.yml

+---
+title: Move diff header actions into dropdown menu
+merge_request:
+author:
+type: changed

locale/gitlab.pot

@@ -4483,9 +4483,6 @@ msgstr ""
 msgid "Can't create snippet: %{err}"
 msgstr ""
 
-msgid "Can't edit as source branch was deleted"
-msgstr ""
-
 msgid "Can't fetch content for the blob: %{err}"
 msgstr ""
 
@@ -9321,9 +9318,6 @@ msgstr ""
 msgid "Edit environment"
 msgstr ""
 
-msgid "Edit file in..."
-msgstr ""
-
 msgid "Edit files in the editor and commit changes here"
 msgstr ""
 
@@ -12997,6 +12991,9 @@ msgid_plural "Hide charts"
 msgstr[0] ""
 msgstr[1] ""
 
+msgid "Hide comments on this file"
+msgstr ""
+
 msgid "Hide details"
 msgstr ""
 
@@ -15841,9 +15838,6 @@ msgstr ""
 msgid "MergeRequests|Thread will be unresolved"
 msgstr ""
 
-msgid "MergeRequests|Toggle comments for this file"
-msgstr ""
-
 msgid "MergeRequests|View file @ %{commitId}"
 msgstr ""
 
@@ -23420,6 +23414,9 @@ msgstr ""
 msgid "Show comments"
 msgstr ""
 
+msgid "Show comments on this file"
+msgstr ""
+
 msgid "Show comments only"
 msgstr ""
 

spec/features/merge_request/maintainer_edits_fork_spec.rb

@@ -26,10 +26,12 @@ RSpec.describe 'a maintainer edits files on a source-branch of an MR from a fork
     visit project_merge_request_path(target_project, merge_request)
     click_link 'Changes'
     wait_for_requests
-    within first('.js-file-title') do
-      find('[data-testid="edit_file"]').click
-      click_link 'Edit in single-file editor'
+
+    page.within(first('.js-file-title')) do
+      find('.js-diff-more-actions').click
+      find('.js-edit-blob').click
     end
+
     wait_for_requests
   end
 

spec/features/merge_request/user_comments_on_diff_spec.rb

@@ -34,7 +34,8 @@ RSpec.describe 'User comments on a diff', :js do
           page.within('.diff-files-holder > div:nth-child(3)') do
             expect(page).to have_content('Line is wrong')
 
-            find('.js-btn-vue-toggle-comments').click
+            find('.js-diff-more-actions').click
+            click_button 'Hide comments on this file'
 
             expect(page).not_to have_content('Line is wrong')
           end
@@ -67,7 +68,8 @@ RSpec.describe 'User comments on a diff', :js do
 
           # Hide the comment.
           page.within('.diff-files-holder > div:nth-child(3)') do
-            find('.js-btn-vue-toggle-comments').click
+            find('.js-diff-more-actions').click
+            click_button 'Hide comments on this file'
 
             expect(page).not_to have_content('Line is wrong')
           end
@@ -80,7 +82,8 @@ RSpec.describe 'User comments on a diff', :js do
 
           # Show the comment.
           page.within('.diff-files-holder > div:nth-child(3)') do
-            find('.js-btn-vue-toggle-comments').click
+            find('.js-diff-more-actions').click
+            click_button 'Show comments on this file'
           end
 
           # Now both the comments should be shown.

spec/features/merge_request/user_sees_diff_spec.rb

@@ -63,7 +63,7 @@ RSpec.describe 'Merge request > User sees diff', :js do
         visit diffs_project_merge_request_path(project, merge_request)
 
         # Throws `Capybara::Poltergeist::InvalidSelector` if we try to use `#hash` syntax
-        expect(page).to have_selector("[id=\"#{changelog_id}\"] [data-testid='edit_file']")
+        expect(page).to have_selector("[id=\"#{changelog_id}\"] .js-edit-blob", visible: false)
       end
     end
 
@@ -73,7 +73,8 @@ RSpec.describe 'Merge request > User sees diff', :js do
         visit diffs_project_merge_request_path(project, merge_request)
 
         # Throws `Capybara::Poltergeist::InvalidSelector` if we try to use `#hash` syntax
-        find("[id=\"#{changelog_id}\"] [data-testid=\"edit_file\"").click
+        find("[id=\"#{changelog_id}\"] .js-diff-more-actions").click
+        find("[id=\"#{changelog_id}\"] .js-edit-blob").click
 
         expect(page).to have_selector('.js-fork-suggestion-button', count: 1)
         expect(page).to have_selector('.js-cancel-fork-suggestion-button', count: 1)

spec/features/merge_request/user_suggests_changes_on_diff_spec.rb

@@ -119,7 +119,8 @@ RSpec.describe 'User comments on a diff', :js do
     it 'can add and remove suggestions from a batch' do
       files.each_with_index do |file, index|
         page.within("[id='#{file[:hash]}']") do
-          find("button[title='Show full file']").click
+          find('.js-diff-more-actions').click
+          click_button 'Show full file'
           wait_for_requests
 
           click_diff_line(find("[id='#{file[:line_code]}']"))

spec/features/projects/blobs/edit_spec.rb

@@ -20,22 +20,14 @@ RSpec.describe 'Editing file blob', :js do
       sign_in(user)
     end
 
-    def edit_and_commit(commit_changes: true)
+    def edit_and_commit(commit_changes: true, is_diff: false)
       wait_for_requests
-      find('.js-edit-blob').click
 
-      fill_and_commit(commit_changes)
-    end
-
-    def mr_edit_and_commit(commit_changes: true)
-      wait_for_requests
-      find('[data-testid="edit_file"]').click
-      click_link 'Edit in single-file editor'
-
-      fill_and_commit(commit_changes)
-    end
+      if is_diff
+        first('.js-diff-more-actions').click
+      end
 
-    def fill_and_commit(commit_changes)
+      first('.js-edit-blob').click
       fill_editor(content: 'class NextFeature\\nend\\n')
 
       if commit_changes
@@ -51,7 +43,7 @@ RSpec.describe 'Editing file blob', :js do
     context 'from MR diff' do
       before do
         visit diffs_project_merge_request_path(project, merge_request)
-        mr_edit_and_commit
+        edit_and_commit(is_diff: true)
       end
 
       it 'returns me to the mr' do

spec/frontend/diffs/components/diff_file_header_spec.js

 import { shallowMount, createLocalVue } from '@vue/test-utils';
 import Vuex from 'vuex';
-import { GlIcon } from '@gitlab/ui';
 import { cloneDeep } from 'lodash';
 import DiffFileHeader from '~/diffs/components/diff_file_header.vue';
-import EditButton from '~/diffs/components/edit_button.vue';
 import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
 import diffDiscussionsMockData from '../mock_data/diff_discussions';
 import { truncateSha } from '~/lib/utils/text_utility';
@@ -76,16 +74,7 @@ describe('DiffFileHeader component', () => {
   const findReplacedFileButton = () => wrapper.find({ ref: 'replacedFileButton' });
   const findViewFileButton = () => wrapper.find({ ref: 'viewButton' });
   const findCollapseIcon = () => wrapper.find({ ref: 'collapseIcon' });
-  const hasZDropdownMenuClass = () => wrapper.classes('gl-z-dropdown-menu!');
-
-  const findIconByName = iconName => {
-    const icons = wrapper.findAll(GlIcon).filter(w => w.props('name') === iconName);
-    if (icons.length === 0) return icons;
-    if (icons.length > 1) {
-      throw new Error(`Multiple icons found for ${iconName}`);
-    }
-    return icons.at(0);
-  };
+  const findEditButton = () => wrapper.find({ ref: 'editButton' });
 
   const createComponent = props => {
     mockStoreConfig = cloneDeep(defaultMockStoreConfig);
@@ -152,10 +141,6 @@ describe('DiffFileHeader component', () => {
     expect(wrapper.find(ClipboardButton).exists()).toBe(true);
   });
 
-  it('should not have z dropdown menu class', () => {
-    expect(hasZDropdownMenuClass()).toBe(false);
-  });
-
   describe('for submodule', () => {
     const submoduleDiffFile = {
       ...diffFile,
@@ -208,16 +193,6 @@ describe('DiffFileHeader component', () => {
   describe('for any file', () => {
     const otherModes = Object.keys(diffViewerModes).filter(m => m !== 'mode_changed');
 
-    it('when edit button emits showForkMessage event it is re-emitted', () => {
-      createComponent({
-        addMergeRequestButtons: true,
-      });
-      wrapper.find(EditButton).vm.$emit('showForkMessage');
-      return wrapper.vm.$nextTick().then(() => {
-        expect(wrapper.emitted().showForkMessage).toBeDefined();
-      });
-    });
-
     it('for mode_changed file mode displays mode changes', () => {
       createComponent({
         diffFile: {
@@ -276,16 +251,16 @@ describe('DiffFileHeader component', () => {
       });
       it('should not render edit button', () => {
         createComponent({ addMergeRequestButtons: false });
-        expect(wrapper.find(EditButton).exists()).toBe(false);
+        expect(findEditButton().exists()).toBe(false);
       });
     });
 
     describe('when addMergeRequestButtons is true', () => {
       describe('without discussions', () => {
-        it('renders a disabled toggle discussions button', () => {
+        it('does not render a toggle discussions button', () => {
           diffHasDiscussionsResultMock.mockReturnValue(false);
           createComponent({ addMergeRequestButtons: true });
-          expect(findToggleDiscussionsButton().attributes('disabled')).toBe('true');
+          expect(findToggleDiscussionsButton().exists()).toBe(false);
         });
       });
 
@@ -293,7 +268,7 @@ describe('DiffFileHeader component', () => {
         it('dispatches toggleFileDiscussionWrappers when user clicks on toggle discussions button', () => {
           diffHasDiscussionsResultMock.mockReturnValue(true);
           createComponent({ addMergeRequestButtons: true });
-          expect(findToggleDiscussionsButton().attributes('disabled')).toBeFalsy();
+          expect(findToggleDiscussionsButton().exists()).toBe(true);
           findToggleDiscussionsButton().vm.$emit('click');
           expect(
             mockStoreConfig.modules.diffs.actions.toggleFileDiscussionWrappers,
@@ -305,28 +280,7 @@ describe('DiffFileHeader component', () => {
         createComponent({
           addMergeRequestButtons: true,
         });
-        expect(wrapper.find(EditButton).exists()).toBe(true);
-      });
-
-      describe('when edit button opens', () => {
-        beforeEach(async () => {
-          createComponent({ addMergeRequestButtons: true });
-          wrapper.find(EditButton).vm.$emit('open');
-
-          await wrapper.vm.$nextTick();
-        });
-
-        it('should add z dropdown menu class when edit button opens', async () => {
-          expect(hasZDropdownMenuClass()).toBe(true);
-        });
-
-        it('when closes again, should remove class', async () => {
-          wrapper.find(EditButton).vm.$emit('close');
-
-          await wrapper.vm.$nextTick();
-
-          expect(hasZDropdownMenuClass()).toBe(false);
-        });
+        expect(findEditButton().exists()).toBe(true);
       });
 
       describe('view on environment button', () => {
@@ -360,7 +314,7 @@ describe('DiffFileHeader component', () => {
         });
 
         it('should not render edit button', () => {
-          expect(wrapper.find(EditButton).exists()).toBe(false);
+          expect(findEditButton().exists()).toBe(false);
         });
       });
       describe('with file blob', () => {
@@ -371,7 +325,7 @@ describe('DiffFileHeader component', () => {
             addMergeRequestButtons: true,
           });
           expect(findViewFileButton().attributes('href')).toBe(viewPath);
-          expect(findViewFileButton().attributes('title')).toEqual(
+          expect(findViewFileButton().text()).toEqual(
             `View file @ ${diffFile.content_sha.substr(0, 8)}`,
           );
         });
@@ -401,21 +355,6 @@ describe('DiffFileHeader component', () => {
           addMergeRequestButtons: true,
         };
 
-        it.each`
-          iconName         | isShowingFullFile
-          ${'doc-expand'}  | ${false}
-          ${'doc-changes'} | ${true}
-        `(
-          'shows $iconName when isShowingFullFile set to $isShowingFullFile',
-          ({ iconName, isShowingFullFile }) => {
-            createComponent({
-              ...fullyNotExpandedFileProps,
-              diffFile: { ...fullyNotExpandedFileProps.diffFile, isShowingFullFile },
-            });
-            expect(findIconByName(iconName).exists()).toBe(true);
-          },
-        );
-
         it('renders expand to full file button if not showing full file already', () => {
           createComponent(fullyNotExpandedFileProps);
           expect(findExpandButton().exists()).toBe(true);
@@ -481,7 +420,7 @@ describe('DiffFileHeader component', () => {
 
     it('does not show edit button', () => {
       createComponent({ diffFile: { ...diffFile, deleted_file: true } });
-      expect(wrapper.find(EditButton).exists()).toBe(false);
+      expect(findEditButton().exists()).toBe(false);
     });
   });
 

spec/frontend/diffs/components/edit_button_spec.js

-import { shallowMount, mount } from '@vue/test-utils';
-import { GlDeprecatedDropdown, GlDeprecatedDropdownItem, GlIcon } from '@gitlab/ui';
-import { createMockDirective, getBinding } from 'helpers/vue_mock_directive';
-import EditButton from '~/diffs/components/edit_button.vue';
-
-jest.mock('lodash/uniqueId', () => (str = '') => `${str}fake`);
-
-const TOOLTIP_ID = 'edit_button_tooltip_fake';
-const EDIT_ITEM = {
-  href: 'test-path',
-  text: 'Edit in single-file editor',
-};
-const IDE_EDIT_ITEM = {
-  href: 'ide-test-path',
-  text: 'Edit in Web IDE',
-};
-
-describe('EditButton', () => {
-  let wrapper;
-
-  const createComponent = (props = {}, mountFn = shallowMount) => {
-    wrapper = mountFn(EditButton, {
-      propsData: {
-        editPath: EDIT_ITEM.href,
-        ideEditPath: IDE_EDIT_ITEM.href,
-        canCurrentUserFork: false,
-        ...props,
-      },
-      directives: {
-        GlTooltip: createMockDirective(),
-      },
-    });
-  };
-
-  afterEach(() => {
-    wrapper.destroy();
-  });
-
-  const getTooltip = () => getBinding(wrapper.element, 'gl-tooltip').value;
-  const findDropdown = () => wrapper.find(GlDeprecatedDropdown);
-  const parseDropdownItems = () =>
-    wrapper.findAll(GlDeprecatedDropdownItem).wrappers.map(x => ({
-      text: x.text(),
-      href: x.attributes('href'),
-    }));
-  const triggerShow = () => {
-    const event = new Event('');
-    jest.spyOn(event, 'preventDefault');
-
-    findDropdown().vm.$emit('show', event);
-
-    return event;
-  };
-
-  it.each`
-    props                  | expectedItems
-    ${{}}                  | ${[EDIT_ITEM, IDE_EDIT_ITEM]}
-    ${{ editPath: '' }}    | ${[IDE_EDIT_ITEM]}
-    ${{ ideEditPath: '' }} | ${[EDIT_ITEM]}
-  `('should render items with=$props', ({ props, expectedItems }) => {
-    createComponent(props);
-
-    expect(parseDropdownItems()).toEqual(expectedItems);
-  });
-
-  describe('with default', () => {
-    beforeEach(() => {
-      createComponent({}, mount);
-    });
-
-    it('does not have tooltip', () => {
-      expect(getTooltip()).toEqual({ id: TOOLTIP_ID, title: 'Edit file in...' });
-    });
-
-    it('shows pencil dropdown', () => {
-      expect(wrapper.find(GlIcon).props('name')).toBe('pencil');
-      expect(wrapper.find('.gl-dropdown-caret').exists()).toBe(true);
-    });
-
-    describe.each`
-      event     | expectedEmit | expectedRootEmit
-      ${'show'} | ${'open'}    | ${[['bv::hide::tooltip', TOOLTIP_ID]]}
-      ${'hide'} | ${'close'}   | ${[]}
-    `('when dropdown emits $event', ({ event, expectedEmit, expectedRootEmit }) => {
-      let rootEmitSpy;
-
-      beforeEach(() => {
-        rootEmitSpy = jest.spyOn(wrapper.vm.$root, '$emit');
-
-        findDropdown().vm.$emit(event);
-      });
-
-      it(`emits ${expectedEmit}`, () => {
-        expect(wrapper.emitted(expectedEmit)).toEqual([[]]);
-      });
-
-      it(`emits root = ${JSON.stringify(expectedRootEmit)}`, () => {
-        expect(rootEmitSpy.mock.calls).toEqual(expectedRootEmit);
-      });
-    });
-  });
-
-  describe('with cant modify blob and can fork', () => {
-    beforeEach(() => {
-      createComponent({
-        canModifyBlob: false,
-        canCurrentUserFork: true,
-      });
-    });
-
-    it('when try to open, emits showForkMessage', () => {
-      expect(wrapper.emitted('showForkMessage')).toBeUndefined();
-
-      const event = triggerShow();
-
-      expect(wrapper.emitted('showForkMessage')).toEqual([[]]);
-      expect(event.preventDefault).toHaveBeenCalled();
-      expect(wrapper.emitted('open')).toBeUndefined();
-    });
-  });
-
-  describe('with editPath is falsey', () => {
-    beforeEach(() => {
-      createComponent({
-        editPath: '',
-      });
-    });
-
-    it('should disable dropdown', () => {
-      expect(findDropdown().attributes('disabled')).toBe('true');
-    });
-
-    it('should have tooltip', () => {
-      expect(getTooltip()).toEqual({
-        id: TOOLTIP_ID,
-        title: "Can't edit as source branch was deleted",
-      });
-    });
-  });
-});

spec/frontend/diffs/store/getters_spec.js

@@ -139,50 +139,74 @@ describe('Diffs Module Getters', () => {
 
   describe('diffHasExpandedDiscussions', () => {
     it('returns true when one of the discussions is expanded', () => {
-      discussionMock1.expanded = false;
+      const diffFile = {
+        parallel_diff_lines: [],
+        highlighted_diff_lines: [
+          {
+            discussions: [discussionMock, discussionMock],
+            discussionsExpanded: true,
+          },
+        ],
+      };
 
-      expect(
-        getters.diffHasExpandedDiscussions(localState, {
-          getDiffFileDiscussions: () => [discussionMock, discussionMock],
-        })(diffFileMock),
-      ).toEqual(true);
+      expect(getters.diffHasExpandedDiscussions(localState)(diffFile)).toEqual(true);
     });
 
     it('returns false when there are no discussions', () => {
-      expect(
-        getters.diffHasExpandedDiscussions(localState, { getDiffFileDiscussions: () => [] })(
-          diffFileMock,
-        ),
-      ).toEqual(false);
+      const diffFile = {
+        parallel_diff_lines: [],
+        highlighted_diff_lines: [
+          {
+            discussions: [],
+            discussionsExpanded: true,
+          },
+        ],
+      };
+      expect(getters.diffHasExpandedDiscussions(localState)(diffFile)).toEqual(false);
     });
 
     it('returns false when no discussion is expanded', () => {
-      discussionMock.expanded = false;
-      discussionMock1.expanded = false;
+      const diffFile = {
+        parallel_diff_lines: [],
+        highlighted_diff_lines: [
+          {
+            discussions: [discussionMock, discussionMock],
+            discussionsExpanded: false,
+          },
+        ],
+      };
 
-      expect(
-        getters.diffHasExpandedDiscussions(localState, {
-          getDiffFileDiscussions: () => [discussionMock, discussionMock1],
-        })(diffFileMock),
-      ).toEqual(false);
+      expect(getters.diffHasExpandedDiscussions(localState)(diffFile)).toEqual(false);
     });
   });
 
   describe('diffHasDiscussions', () => {
     it('returns true when getDiffFileDiscussions returns discussions', () => {
-      expect(
-        getters.diffHasDiscussions(localState, {
-          getDiffFileDiscussions: () => [discussionMock],
-        })(diffFileMock),
-      ).toEqual(true);
+      const diffFile = {
+        parallel_diff_lines: [],
+        highlighted_diff_lines: [
+          {
+            discussions: [discussionMock, discussionMock],
+            discussionsExpanded: false,
+          },
+        ],
+      };
+
+      expect(getters.diffHasDiscussions(localState)(diffFile)).toEqual(true);
     });
 
     it('returns false when getDiffFileDiscussions returns no discussions', () => {
-      expect(
-        getters.diffHasDiscussions(localState, {
-          getDiffFileDiscussions: () => [],
-        })(diffFileMock),
-      ).toEqual(false);
+      const diffFile = {
+        parallel_diff_lines: [],
+        highlighted_diff_lines: [
+          {
+            discussions: [],
+            discussionsExpanded: false,
+          },
+        ],
+      };
+
+      expect(getters.diffHasDiscussions(localState)(diffFile)).toEqual(false);
     });
   });