Merge branch '11133-fix-icon-misalignment' into 'master'

Geo - Clean Up Geo Node Actions

Closes #219881 and #11133

See merge request gitlab-org/gitlab!36867

ee/app/assets/javascripts/geo_nodes/components/geo_node_actions.vue

 <script>
-import { GlDeprecatedButton, GlTooltipDirective } from '@gitlab/ui';
+import { GlIcon, GlButton, GlTooltipDirective } from '@gitlab/ui';
 import { __, s__ } from '~/locale';
 import eventHub from '../event_hub';
 import { NODE_ACTIONS } from '../constants';
-import Icon from '~/vue_shared/components/icon.vue';
 
 export default {
   components: {
-    Icon,
-    GlDeprecatedButton,
+    GlIcon,
+    GlButton,
   },
   directives: {
     GlTooltip: GlTooltipDirective,
@@ -87,48 +86,53 @@ export default {
 </script>
 
 <template>
-  <div class="d-flex align-items-center justify-content-end geo-node-actions">
-    <a
+  <div
+    data-testid="nodeActions"
+    class="geo-node-actions gl-display-flex gl-align-items-center gl-justify-content-end gl-flex-direction-column gl-sm-flex-direction-row"
+  >
+    <gl-button
       v-if="isSecondaryNode"
       :href="node.geoProjectsUrl"
-      class="btn btn-sm mx-1 sm-column-spacing"
+      class="gl-mx-2 gl-mt-5 gl-sm-mt-0"
       target="_blank"
     >
-      <icon v-if="!node.current" name="external-link" /> {{ __('Open projects') }}
-    </a>
+      <span class="gl-display-flex gl-align-items-center">
+        <gl-icon v-if="!node.current" name="external-link" class="gl-mr-2" />
+        {{ __('Open projects') }}
+      </span>
+    </gl-button>
     <template v-if="nodeActionsAllowed">
-      <gl-deprecated-button
-        v-if="nodeMissingOauth"
-        class="btn btn-sm btn-default mx-1 sm-column-spacing"
-        @click="onRepairNode"
-      >
+      <gl-button v-if="nodeMissingOauth" class="gl-mx-2 gl-mt-5 gl-sm-mt-0" @click="onRepairNode">
         {{ s__('Repair authentication') }}
-      </gl-deprecated-button>
-      <a v-if="nodeEditAllowed" :href="node.editPath" class="btn btn-sm mx-1 sm-column-spacing">
+      </gl-button>
+      <gl-button v-if="nodeEditAllowed" :href="node.editPath" class="gl-mx-2 gl-mt-5 gl-sm-mt-0">
         {{ __('Edit') }}
-      </a>
-      <gl-deprecated-button
+      </gl-button>
+      <gl-button
         v-if="isSecondaryNode"
-        class="btn btn-sm btn-danger mx-1 sm-column-spacing"
+        data-testid="removeButton"
+        variant="danger"
+        class="gl-mx-2 gl-mt-5 gl-sm-mt-0"
         :disabled="!nodeRemovalAllowed"
         @click="onRemoveSecondaryNode"
       >
         {{ __('Remove') }}
-      </gl-deprecated-button>
+      </gl-button>
       <div
         v-gl-tooltip.hover
         name="disabledRemovalTooltip"
-        class="mx-1 sm-column-spacing"
+        class="gl-mx-2 gl-mt-5 gl-sm-mt-0"
         :title="disabledRemovalTooltip"
       >
-        <gl-deprecated-button
+        <gl-button
           v-if="!isSecondaryNode"
-          class="btn btn-sm btn-danger w-100"
+          variant="danger"
+          class="gl-w-full"
           :disabled="!nodeRemovalAllowed"
           @click="onRemovePrimaryNode"
         >
           {{ __('Remove') }}
-        </gl-deprecated-button>
+        </gl-button>
       </div>
     </template>
   </div>

ee/app/assets/javascripts/geo_nodes/components/node_detail_sections/node_details_section_main.vue

@@ -74,7 +74,7 @@ export default {
 <template>
   <div class="row-fluid clearfix py-3 primary-section">
     <div class="col-md-12">
-      <div class="d-flex geo-node-actions-container">
+      <div class="gl-display-flex gl-flex-wrap gl-flex-direction-column gl-sm-flex-direction-row">
         <div data-testid="nodeUrl" class="d-flex flex-column">
           <span class="gl-text-gray-700">{{ s__('GeoNodes|Node URL') }}</span>
           <gl-link

ee/app/assets/stylesheets/pages/geo.scss

-@media (max-width: map-get($grid-breakpoints, sm)) {
+@media (max-width: $breakpoint-sm) {
   .geo-node-actions {
-    flex-direction: column;
     margin: 0 1rem;
 
-    .sm-column-spacing {
+    > * {
       width: 100%;
-      margin-top: 1rem;
     }
   }
-
-  .geo-node-actions-container {
-    flex-direction: column;
-  }
 }
 
 .project-card-errors {

ee/changelogs/unreleased/11133-fix-icon-misalignment.yml

+---
+title: Geo - Fix Button Icon Alignment
+merge_request: 36867
+author:
+type: fixed

ee/spec/frontend/geo_nodes/components/geo_node_actions_spec.js

-import Vue from 'vue';
-
-import geoNodeActionsComponent from 'ee/geo_nodes/components/geo_node_actions.vue';
-import mountComponent from 'helpers/vue_mount_component_helper';
+import { shallowMount } from '@vue/test-utils';
+import GeoNodeActionsComponent from 'ee/geo_nodes/components/geo_node_actions.vue';
+import { GlButton } from '@gitlab/ui';
 import eventHub from 'ee/geo_nodes/event_hub';
 import { NODE_ACTIONS } from 'ee/geo_nodes/constants';
 import { mockNodes } from '../mock_data';
 
 jest.mock('ee/geo_nodes/event_hub');
 
-const createComponent = (
-  node = mockNodes[0],
-  nodeEditAllowed = true,
-  nodeActionsAllowed = true,
-  nodeRemovalAllowed = true,
-  nodeMissingOauth = false,
-) => {
-  const Component = Vue.extend(geoNodeActionsComponent);
-
-  return mountComponent(Component, {
-    node,
-    nodeEditAllowed,
-    nodeActionsAllowed,
-    nodeRemovalAllowed,
-    nodeMissingOauth,
-  });
-};
-
 describe('GeoNodeActionsComponent', () => {
-  let vm;
-
-  beforeEach(() => {
-    vm = createComponent();
-  });
+  let wrapper;
+
+  const defaultProps = {
+    node: mockNodes[0],
+    nodeEditAllowed: true,
+    nodeActionsAllowed: true,
+    nodeRemovalAllowed: true,
+    nodeMissingOauth: false,
+  };
+
+  const createComponent = (props = {}) => {
+    wrapper = shallowMount(GeoNodeActionsComponent, {
+      propsData: {
+        ...defaultProps,
+        ...props,
+      },
+    });
+  };
 
   afterEach(() => {
-    vm.$destroy();
+    wrapper.destroy();
+    wrapper = null;
   });
 
+  const findGeoNodeActionsComponent = () => wrapper.find('[data-testid="nodeActions"]');
+  const findNodeActions = () => wrapper.findAll(GlButton);
+  const findRemoveButton = () => wrapper.find('[data-testid="removeButton"]');
+
   describe('computed', () => {
     describe('disabledRemovalTooltip', () => {
       describe.each`
@@ -45,11 +44,11 @@ describe('GeoNodeActionsComponent', () => {
         ${false}           | ${'Cannot remove a primary node if there is a secondary node'}
       `('when nodeRemovalAllowed is $nodeRemovalAllowed', ({ nodeRemovalAllowed, tooltip }) => {
         beforeEach(() => {
-          vm = createComponent(mockNodes[0], true, true, nodeRemovalAllowed, false);
+          createComponent({ nodeRemovalAllowed });
         });
 
         it('renders the correct tooltip', () => {
-          const tip = vm.$el.querySelector('div[name=disabledRemovalTooltip]');
+          const tip = wrapper.vm.$el.querySelector('div[name=disabledRemovalTooltip]');
           expect(tip.title).toBe(tooltip);
         });
       });
@@ -57,13 +56,17 @@ describe('GeoNodeActionsComponent', () => {
   });
 
   describe('methods', () => {
+    beforeEach(() => {
+      createComponent();
+    });
+
     describe('onRemovePrimaryNode', () => {
       it('emits showNodeActionModal with actionType `remove`, node reference, modalKind, modalMessage, modalActionLabel, and modalTitle', () => {
-        vm.onRemovePrimaryNode();
+        wrapper.vm.onRemovePrimaryNode();
 
         expect(eventHub.$emit).toHaveBeenCalledWith('showNodeActionModal', {
           actionType: NODE_ACTIONS.REMOVE,
-          node: vm.node,
+          node: wrapper.vm.node,
           modalKind: 'danger',
           modalMessage:
             'Removing a Geo primary node stops the synchronization to all nodes. Are you sure?',
@@ -75,11 +78,11 @@ describe('GeoNodeActionsComponent', () => {
 
     describe('onRemoveSecondaryNode', () => {
       it('emits showNodeActionModal with actionType `remove`, node reference, modalKind, modalMessage, modalActionLabel, and modalTitle', () => {
-        vm.onRemoveSecondaryNode();
+        wrapper.vm.onRemoveSecondaryNode();
 
         expect(eventHub.$emit).toHaveBeenCalledWith('showNodeActionModal', {
           actionType: NODE_ACTIONS.REMOVE,
-          node: vm.node,
+          node: wrapper.vm.node,
           modalKind: 'danger',
           modalMessage:
             'Removing a Geo secondary node stops the synchronization to that node. Are you sure?',
@@ -91,41 +94,45 @@ describe('GeoNodeActionsComponent', () => {
 
     describe('onRepairNode', () => {
       it('emits `repairNode` event with node reference', () => {
-        vm.onRepairNode();
+        wrapper.vm.onRepairNode();
 
-        expect(eventHub.$emit).toHaveBeenCalledWith('repairNode', vm.node);
+        expect(eventHub.$emit).toHaveBeenCalledWith('repairNode', wrapper.vm.node);
       });
     });
   });
 
   describe('template', () => {
+    beforeEach(() => {
+      createComponent();
+    });
+
     it('renders container elements correctly', () => {
-      expect(vm.$el.classList.contains('geo-node-actions')).toBe(true);
-      expect(vm.$el.querySelectorAll('.btn-sm').length).not.toBe(0);
+      expect(findGeoNodeActionsComponent().exists()).toBeTruthy();
+      expect(findNodeActions()).not.toHaveLength(0);
     });
 
     describe.each`
       nodeRemovalAllowed | buttonDisabled
-      ${false}           | ${true}
-      ${true}            | ${false}
-    `(
-      `when nodeRemovalAllowed is $nodeRemovalAllowed`,
-      ({ nodeRemovalAllowed, buttonDisabled }) => {
-        let removeButton;
-
-        beforeEach(() => {
-          vm = createComponent(mockNodes[0], true, true, nodeRemovalAllowed, false);
-          removeButton = vm.$el.querySelector('.btn-danger');
-        });
+      ${false}           | ${'true'}
+      ${true}            | ${undefined}
+    `(`Remove Button`, ({ nodeRemovalAllowed, buttonDisabled }) => {
+      beforeEach(() => {
+        createComponent({ node: mockNodes[1], nodeRemovalAllowed });
+      });
 
+      describe(`when nodeRemovalAllowed is ${nodeRemovalAllowed}`, () => {
         it('has the correct button text', () => {
-          expect(removeButton.innerText.trim()).toBe('Remove');
+          expect(
+            findRemoveButton()
+              .text()
+              .trim(),
+          ).toBe('Remove');
         });
 
         it(`the button's disabled attribute should be ${buttonDisabled}`, () => {
-          expect(removeButton.disabled).toBe(buttonDisabled);
+          expect(findRemoveButton().attributes('disabled')).toBe(buttonDisabled);
         });
-      },
-    );
+      });
+    });
   });
 });