Merge branch...

Merge branch '227257-replace-bootstrap-popover-with-gitlab-ui-popover-in-merge-conflict-widget' into 'master'

Replace Bootstrap popover with GitLab UI popover for merge conflict

See merge request gitlab-org/gitlab!55652

app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_conflicts.vue

 <script>
-import { GlButton, GlModalDirective, GlSkeletonLoader } from '@gitlab/ui';
-import $ from 'jquery';
-import { escape } from 'lodash';
-import { s__, sprintf } from '~/locale';
-import { mouseenter, debouncedMouseleave, togglePopover } from '~/shared/popover';
+import { GlButton, GlModalDirective, GlSkeletonLoader, GlPopover, GlLink } from '@gitlab/ui';
+import { s__ } from '~/locale';
 import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import mergeRequestQueryVariablesMixin from '../../mixins/merge_request_query_variables';
 import userPermissionsQuery from '../../queries/permissions.query.graphql';
@@ -16,6 +13,8 @@ export default {
     GlSkeletonLoader,
     StatusIcon,
     GlButton,
+    GlPopover,
+    GlLink,
   },
   directives: {
     GlModalDirective,
@@ -106,48 +105,11 @@ export default {
       return this.showResolveButton && this.sourceBranchProtected;
     },
   },
-  watch: {
-    showPopover: {
-      handler(newVal) {
-        if (newVal) {
-          this.$nextTick(this.initPopover);
-        }
-      },
-      immediate: true,
-    },
-  },
-  methods: {
-    initPopover() {
-      const $el = $(this.$refs.popover);
-
-      $el
-        .popover({
-          html: true,
-          trigger: 'focus',
-          container: 'body',
-          placement: 'top',
-          template:
-            '<div class="popover" role="tooltip"><div class="arrow"></div><p class="popover-header"></p><div class="popover-body"></div></div>',
-          title: s__(
-            'mrWidget|This feature merges changes from the target branch to the source branch. You cannot use this feature since the source branch is protected.',
-          ),
-          content: sprintf(
-            s__('mrWidget|%{link_start}Learn more about resolving conflicts%{link_end}'),
-            {
-              link_start: `<a href="${escape(
-                this.mr.conflictsDocsPath,
-              )}" target="_blank" rel="noopener noreferrer">`,
-              link_end: '</a>',
-            },
-            false,
-          ),
-        })
-        .on('mouseenter', mouseenter)
-        .on('mouseleave', debouncedMouseleave(300))
-        .on('show.bs.popover', () => {
-          window.addEventListener('scroll', togglePopover.bind($el, false), { once: true });
-        });
-    },
+  i18n: {
+    title: s__(
+      'mrWidget|This feature merges changes from the target branch to the source branch. You cannot use this feature since the source branch is protected.',
+    ),
+    linkText: s__('mrWidget|Learn more about resolving conflicts'),
   },
 };
 </script>
@@ -181,17 +143,35 @@ export default {
         </span>
         <span v-if="showResolveButton" ref="popover">
           <gl-button
-            :href="!sourceBranchProtected && mr.conflictResolutionPath"
+            :href="mr.conflictResolutionPath"
             :disabled="sourceBranchProtected"
-            class="js-resolve-conflicts-button"
+            data-testid="resolve-conflicts-button"
           >
             {{ s__('mrWidget|Resolve conflicts') }}
           </gl-button>
+          <gl-popover
+            v-if="showPopover"
+            :target="() => $refs.popover"
+            placement="top"
+            triggers="hover focus"
+          >
+            <template #title>
+              <div class="gl-font-weight-normal gl-font-base">
+                {{ $options.i18n.title }}
+              </div>
+            </template>
+
+            <div class="gl-text-center">
+              <gl-link :href="mr.conflictsDocsPath" target="_blank" rel="noopener noreferrer">
+                {{ $options.i18n.linkText }}
+              </gl-link>
+            </div>
+          </gl-popover>
         </span>
         <gl-button
           v-if="canMerge"
           v-gl-modal-directive="'modal-merge-info'"
-          class="js-merge-locally-button"
+          data-testid="merge-locally-button"
         >
           {{ s__('mrWidget|Merge locally') }}
         </gl-button>

changelogs/unreleased/227257-replace-bootstrap-popover-with-gitlab-ui-popover-in-merge-conflict.yml

+---
+title: Replace Bootstrap popover with GitLab UI popover for merge conflict
+merge_request: 55652
+author:
+type: other

locale/gitlab.pot

@@ -35629,9 +35629,6 @@ msgstr ""
 msgid "mrWidget| Please restore it or use a different %{missingBranchName} branch"
 msgstr ""
 
-msgid "mrWidget|%{link_start}Learn more about resolving conflicts%{link_end}"
-msgstr ""
-
 msgid "mrWidget|%{mergeError}."
 msgstr ""
 
@@ -35749,6 +35746,9 @@ msgstr ""
 msgid "mrWidget|Jump to first unresolved thread"
 msgstr ""
 
+msgid "mrWidget|Learn more about resolving conflicts"
+msgstr ""
+
 msgid "mrWidget|Loading deployment statistics"
 msgstr ""
 

spec/frontend/vue_mr_widget/components/states/mr_widget_conflicts_spec.js

-import { createLocalVue, shallowMount } from '@vue/test-utils';
-import $ from 'jquery';
+import { GlPopover } from '@gitlab/ui';
+import { shallowMount } from '@vue/test-utils';
 import { TEST_HOST } from 'helpers/test_constants';
 import { removeBreakLine } from 'helpers/text_helper';
+import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 import ConflictsComponent from '~/vue_merge_request_widget/components/states/mr_widget_conflicts.vue';
 
 describe('MRWidgetConflicts', () => {
-  let vm;
+  let wrapper;
   let mergeRequestWidgetGraphql = null;
   const path = '/conflicts';
 
-  function createComponent(propsData = {}) {
-    const localVue = createLocalVue();
+  const findPopover = () => wrapper.find(GlPopover);
+  const findResolveButton = () => wrapper.findByTestId('resolve-conflicts-button');
+  const findMergeLocalButton = () => wrapper.findByTestId('merge-locally-button');
 
-    vm = shallowMount(localVue.extend(ConflictsComponent), {
-      propsData,
-      provide: {
-        glFeatures: {
-          mergeRequestWidgetGraphql,
+  function createComponent(propsData = {}) {
+    wrapper = extendedWrapper(
+      shallowMount(ConflictsComponent, {
+        propsData,
+        provide: {
+          glFeatures: {
+            mergeRequestWidgetGraphql,
+          },
         },
-      },
-      mocks: {
-        $apollo: {
-          queries: {
-            userPermissions: { loading: false },
-            stateData: { loading: false },
+        mocks: {
+          $apollo: {
+            queries: {
+              userPermissions: { loading: false },
+              stateData: { loading: false },
+            },
           },
         },
-      },
-    });
+      }),
+    );
 
     if (mergeRequestWidgetGraphql) {
-      vm.setData({
+      wrapper.setData({
         userPermissions: {
           canMerge: propsData.mr.canMerge,
           pushToSourceBranch: propsData.mr.canPushToSourceBranch,
@@ -42,16 +47,12 @@ describe('MRWidgetConflicts', () => {
       });
     }
 
-    return vm.vm.$nextTick();
+    return wrapper.vm.$nextTick();
   }
 
-  beforeEach(() => {
-    jest.spyOn($.fn, 'popover');
-  });
-
   afterEach(() => {
     mergeRequestWidgetGraphql = null;
-    vm.destroy();
+    wrapper.destroy();
   });
 
   [false, true].forEach((featureEnabled) => {
@@ -82,18 +83,16 @@ describe('MRWidgetConflicts', () => {
         });
 
         it('should tell you about conflicts without bothering other people', () => {
-          expect(vm.text()).toContain('There are merge conflicts');
-          expect(vm.text()).not.toContain('ask someone with write access');
+          expect(wrapper.text()).toContain('There are merge conflicts');
+          expect(wrapper.text()).not.toContain('ask someone with write access');
         });
 
         it('should not allow you to resolve the conflicts', () => {
-          expect(vm.text()).not.toContain('Resolve conflicts');
+          expect(wrapper.text()).not.toContain('Resolve conflicts');
         });
 
         it('should have merge buttons', () => {
-          const mergeLocallyButton = vm.find('.js-merge-locally-button');
-
-          expect(mergeLocallyButton.text()).toContain('Merge locally');
+          expect(findMergeLocalButton().text()).toContain('Merge locally');
         });
       });
 
@@ -110,19 +109,17 @@ describe('MRWidgetConflicts', () => {
         });
 
         it('should tell you about conflicts', () => {
-          expect(vm.text()).toContain('There are merge conflicts');
-          expect(vm.text()).toContain('ask someone with write access');
+          expect(wrapper.text()).toContain('There are merge conflicts');
+          expect(wrapper.text()).toContain('ask someone with write access');
         });
 
         it('should allow you to resolve the conflicts', () => {
-          const resolveButton = vm.find('.js-resolve-conflicts-button');
-
-          expect(resolveButton.text()).toContain('Resolve conflicts');
-          expect(resolveButton.attributes('href')).toEqual(path);
+          expect(findResolveButton().text()).toContain('Resolve conflicts');
+          expect(findResolveButton().attributes('href')).toEqual(path);
         });
 
         it('should not have merge buttons', () => {
-          expect(vm.text()).not.toContain('Merge locally');
+          expect(wrapper.text()).not.toContain('Merge locally');
         });
       });
 
@@ -139,21 +136,17 @@ describe('MRWidgetConflicts', () => {
         });
 
         it('should tell you about conflicts without bothering other people', () => {
-          expect(vm.text()).toContain('There are merge conflicts');
-          expect(vm.text()).not.toContain('ask someone with write access');
+          expect(wrapper.text()).toContain('There are merge conflicts');
+          expect(wrapper.text()).not.toContain('ask someone with write access');
         });
 
         it('should allow you to resolve the conflicts', () => {
-          const resolveButton = vm.find('.js-resolve-conflicts-button');
-
-          expect(resolveButton.text()).toContain('Resolve conflicts');
-          expect(resolveButton.attributes('href')).toEqual(path);
+          expect(findResolveButton().text()).toContain('Resolve conflicts');
+          expect(findResolveButton().attributes('href')).toEqual(path);
         });
 
         it('should have merge buttons', () => {
-          const mergeLocallyButton = vm.find('.js-merge-locally-button');
-
-          expect(mergeLocallyButton.text()).toContain('Merge locally');
+          expect(findMergeLocalButton().text()).toContain('Merge locally');
         });
       });
 
@@ -167,7 +160,7 @@ describe('MRWidgetConflicts', () => {
             },
           });
 
-          expect(vm.text().trim().replace(/\s\s+/g, ' ')).toContain(
+          expect(wrapper.text().trim().replace(/\s\s+/g, ' ')).toContain(
             'ask someone with write access',
           );
         });
@@ -181,8 +174,8 @@ describe('MRWidgetConflicts', () => {
             },
           });
 
-          expect(vm.find('.js-resolve-conflicts-button').exists()).toBe(false);
-          expect(vm.find('.js-merge-locally-button').exists()).toBe(false);
+          expect(findResolveButton().exists()).toBe(false);
+          expect(findMergeLocalButton().exists()).toBe(false);
         });
 
         it('should not have resolve button when no conflict resolution path', async () => {
@@ -194,7 +187,7 @@ describe('MRWidgetConflicts', () => {
             },
           });
 
-          expect(vm.find('.js-resolve-conflicts-button').exists()).toBe(false);
+          expect(findResolveButton().exists()).toBe(false);
         });
       });
 
@@ -207,7 +200,7 @@ describe('MRWidgetConflicts', () => {
             },
           });
 
-          expect(removeBreakLine(vm.text()).trim()).toContain(
+          expect(removeBreakLine(wrapper.text()).trim()).toContain(
             'Fast-forward merge is not possible. To merge this request, first rebase locally.',
           );
         });
@@ -227,11 +220,11 @@ describe('MRWidgetConflicts', () => {
         });
 
         it('sets resolve button as disabled', () => {
-          expect(vm.find('.js-resolve-conflicts-button').attributes('disabled')).toBe('true');
+          expect(findResolveButton().attributes('disabled')).toBe('true');
         });
 
-        it('renders popover', () => {
-          expect($.fn.popover).toHaveBeenCalled();
+        it('shows the popover', () => {
+          expect(findPopover().exists()).toBe(true);
         });
       });
 
@@ -249,11 +242,11 @@ describe('MRWidgetConflicts', () => {
         });
 
         it('sets resolve button as disabled', () => {
-          expect(vm.find('.js-resolve-conflicts-button').attributes('disabled')).toBe(undefined);
+          expect(findResolveButton().attributes('disabled')).toBe(undefined);
         });
 
-        it('renders popover', () => {
-          expect($.fn.popover).not.toHaveBeenCalled();
+        it('does not show the popover', () => {
+          expect(findPopover().exists()).toBe(false);
         });
       });
     });