Merge branch 'health-status-dropdown' into 'master'

Use dropdown to change health status

See merge request gitlab-org/gitlab!28547

app/assets/stylesheets/framework/sidebar.scss

@@ -210,3 +210,15 @@
     }
   }
 }
+
+.health-status {
+  .dropdown-body {
+    .health-divider {
+      border-top-color: $gray-200;
+    }
+
+    .dropdown-item:not(.health-dropdown-item) {
+      padding: 0;
+    }
+  }
+}

doc/user/project/issues/index.md

@@ -177,7 +177,7 @@ that's progressing as planned or needs attention to keep on schedule:
 - **Needs attention** (amber)
 - **At risk** (red)
 
-!["On track" health status on an issue](img/issue_health_status_v12_10.png)
+!["On track" health status on an issue](img/issue_health_status_dropdown_v12_10.png)
 
 You can then see issue statuses on the
 [Epic tree](../../group/epics/index.md#issue-health-status-in-epic-tree-ultimate).

ee/app/assets/javascripts/sidebar/components/status/sidebar_status.vue

@@ -17,7 +17,7 @@ export default {
     },
   },
   methods: {
-    handleFormSubmission(status) {
+    handleDropdownClick(status) {
       this.mediator.updateStatus(status).catch(() => {
         Flash(__('Error occurred while updating the issue status'));
       });
@@ -31,6 +31,6 @@ export default {
     :is-editable="mediator.store.editable"
     :is-fetching="mediator.store.isFetching.status"
     :status="mediator.store.status"
-    @onStatusChange="handleFormSubmission"
+    @onDropdownClick="handleDropdownClick"
   />
 </template>

ee/app/assets/javascripts/sidebar/components/status/status.vue

 <script>
+import Tracking from '~/tracking';
 import {
-  GlDeprecatedButton,
-  GlFormGroup,
-  GlFormRadioGroup,
   GlIcon,
+  GlNewButton,
   GlLoadingIcon,
   GlTooltip,
+  GlDropdownItem,
+  GlDropdown,
+  GlDropdownDivider,
 } from '@gitlab/ui';
 import { s__ } from '~/locale';
-import { healthStatusColorMap, healthStatusTextMap } from '../../constants';
+import { healthStatusTextMap } from '../../constants';
 
 export default {
   components: {
-    GlDeprecatedButton,
     GlIcon,
+    GlNewButton,
     GlLoadingIcon,
-    GlFormGroup,
-    GlFormRadioGroup,
     GlTooltip,
+    GlDropdown,
+    GlDropdownItem,
+    GlDropdownDivider,
   },
+  mixins: [Tracking.mixin()],
   props: {
     isEditable: {
       type: Boolean,
@@ -38,11 +42,11 @@ export default {
   },
   data() {
     return {
-      isFormShowing: false,
+      isDropdownShowing: false,
       selectedStatus: this.status,
       statusOptions: Object.keys(healthStatusTextMap).map(key => ({
-        value: key,
-        text: healthStatusTextMap[key],
+        key,
+        value: healthStatusTextMap[key],
       })),
     };
   },
@@ -53,11 +57,11 @@ export default {
     statusText() {
       return this.status ? healthStatusTextMap[this.status] : s__('Sidebar|None');
     },
-    statusColor() {
-      return healthStatusColorMap[this.status];
+    dropdownText() {
+      return this.status ? healthStatusTextMap[this.status] : s__('Select health status');
     },
     tooltipText() {
-      let tooltipText = s__('Sidebar|Status');
+      let tooltipText = s__('Sidebar|Health status');
 
       if (this.status) {
         tooltipText += `: ${this.statusText}`;
@@ -72,19 +76,33 @@ export default {
     },
   },
   methods: {
-    handleFormSubmission() {
-      this.$emit('onStatusChange', this.selectedStatus);
-      this.hideForm();
+    handleDropdownClick(status) {
+      this.selectedStatus = status;
+      this.$emit('onDropdownClick', status);
+      this.track('change_health_status', { property: status });
+      this.hideDropdown();
     },
-    hideForm() {
-      this.isFormShowing = false;
-      this.$refs.editButton.focus();
+    hideDropdown() {
+      this.isDropdownShowing = false;
     },
     toggleFormDropdown() {
-      this.isFormShowing = !this.isFormShowing;
+      this.isDropdownShowing = !this.isDropdownShowing;
+
+      /**
+       * We need to programmatically open the dropdown to make the
+       * outside click on document close the dropdown.
+       */
+      const { dropdown } = this.$refs.dropdown.$refs;
+
+      if (dropdown && this.isDropdownShowing) {
+        dropdown.show();
+      }
     },
     removeStatus() {
-      this.$emit('onStatusChange', null);
+      this.handleDropdownClick(null);
+    },
+    isSelected(status) {
+      return this.status === status;
     },
   },
 };
@@ -104,62 +122,75 @@ export default {
 
     <div class="hide-collapsed">
       <p class="title d-flex justify-content-between">
-        {{ s__('Sidebar|Status') }}
+        {{ s__('Sidebar|Health status') }}
         <a
           v-if="isEditable"
           ref="editButton"
           class="btn-link"
           href="#"
           @click="toggleFormDropdown"
-          @keydown.esc="hideForm"
+          @keydown.esc="hideDropdown"
         >
           {{ __('Edit') }}
         </a>
       </p>
 
-      <div v-if="isFormShowing" class="dropdown show">
-        <form class="dropdown-menu p-3" @submit.prevent="handleFormSubmission">
-          <p>
-            {{
-              __('Choose which status most accurately reflects the current state of this issue:')
-            }}
-          </p>
-          <gl-form-group>
-            <gl-form-radio-group
-              v-model="selectedStatus"
-              :checked="selectedStatus"
-              :options="statusOptions"
-              stacked
-              @keydown.esc.native="hideForm"
+      <div
+        class="dropdown dropdown-menu-selectable"
+        :class="{ show: isDropdownShowing, 'd-none': !isDropdownShowing }"
+      >
+        <gl-dropdown
+          ref="dropdown"
+          class="w-100"
+          :text="dropdownText"
+          @keydown.esc.native="hideDropdown"
+          @hide="hideDropdown"
+        >
+          <div class="dropdown-title">
+            <span class="health-title">{{ s__('Sidebar|Assign health status') }}</span>
+            <gl-new-button
+              :aria-label="__('Close')"
+              variant="link"
+              class="dropdown-title-button dropdown-menu-close"
+              icon="close"
+              @click="hideDropdown"
             />
-          </gl-form-group>
-          <gl-form-group class="mb-0">
-            <gl-deprecated-button type="button" class="append-right-10" @click="hideForm">
-              {{ __('Cancel') }}
-            </gl-deprecated-button>
-            <gl-deprecated-button type="submit" variant="success">
-              {{ __('Save') }}
-            </gl-deprecated-button>
-          </gl-form-group>
-        </form>
+          </div>
+
+          <div class="dropdown-content dropdown-body">
+            <gl-dropdown-item @click="handleDropdownClick(null)">
+              <gl-new-button
+                variant="link"
+                class="dropdown-item health-dropdown-item"
+                :class="{ 'is-active': isSelected(null) }"
+              >
+                {{ s__('Sidebar|No status') }}
+              </gl-new-button>
+            </gl-dropdown-item>
+
+            <gl-dropdown-divider class="divider health-divider" />
+
+            <gl-dropdown-item
+              v-for="option in statusOptions"
+              :key="option.key"
+              @click="handleDropdownClick(option.key)"
+            >
+              <gl-new-button
+                variant="link"
+                class="dropdown-item health-dropdown-item"
+                :class="{ 'is-active': isSelected(option.key) }"
+              >
+                {{ option.value }}
+              </gl-new-button>
+            </gl-dropdown-item>
+          </div>
+        </gl-dropdown>
       </div>
 
       <gl-loading-icon v-if="isFetching" :inline="true" />
-      <p v-else class="value d-flex align-items-center m-0" :class="{ 'no-value': !status }">
-        <gl-icon
-          v-if="status"
-          name="severity-low"
-          :size="14"
-          class="align-bottom append-right-10"
-          :class="statusColor"
-        />
-        {{ statusText }}
-        <template v-if="canRemoveStatus">
-          <span class="text-secondary mx-1" aria-hidden="true">-</span>
-          <gl-deprecated-button variant="link" class="text-secondary" @click="removeStatus">
-            {{ __('remove status') }}
-          </gl-deprecated-button>
-        </template>
+      <p v-else-if="!isDropdownShowing" class="value m-0" :class="{ 'no-value': !status }">
+        <span v-if="status" class="text-plain bold">{{ statusText }}</span>
+        <span v-else>{{ __('None') }}</span>
       </p>
     </div>
   </div>

ee/app/assets/javascripts/sidebar/constants.js

@@ -6,12 +6,6 @@ export const healthStatus = {
   AT_RISK: 'atRisk',
 };
 
-export const healthStatusColorMap = {
-  [healthStatus.ON_TRACK]: 'text-success',
-  [healthStatus.NEEDS_ATTENTION]: 'text-warning',
-  [healthStatus.AT_RISK]: 'text-danger',
-};
-
 export const healthStatusTextMap = {
   [healthStatus.ON_TRACK]: __('On track'),
   [healthStatus.NEEDS_ATTENTION]: __('Needs attention'),

ee/changelogs/unreleased/health-status-dropdown.yml

+---
+title: Use dropdown to change health status
+merge_request: 28547
+author:
+type: changed

ee/spec/frontend/sidebar/components/status/sidebar_status_spec.js

-import { mount, shallowMount } from '@vue/test-utils';
-import Vue from 'vue';
+import { shallowMount } from '@vue/test-utils';
 import SidebarStatus from 'ee/sidebar/components/status/sidebar_status.vue';
 import Status from 'ee/sidebar/components/status/status.vue';
 
-const getStatusText = wrapper => wrapper.find('.value').text();
-
 describe('SidebarStatus', () => {
   let wrapper;
+  let handleDropdownClickMock;
+
+  beforeEach(() => {
+    const mediator = {
+      store: {
+        isFetching: {
+          status: true,
+        },
+        status: '',
+      },
+    };
+
+    handleDropdownClickMock = jest.fn();
+
+    wrapper = shallowMount(SidebarStatus, {
+      propsData: {
+        mediator,
+      },
+      methods: {
+        handleDropdownClick: handleDropdownClickMock,
+      },
+    });
+  });
 
   afterEach(() => {
     wrapper.destroy();
@@ -14,73 +34,22 @@ describe('SidebarStatus', () => {
   });
 
   describe('Status child component', () => {
-    let handleFormSubmissionMock;
-
-    beforeEach(() => {
-      const mediator = {
-        store: {
-          isFetching: {
-            status: true,
-          },
-          status: '',
-        },
-      };
-
-      handleFormSubmissionMock = jest.fn();
-
-      wrapper = shallowMount(SidebarStatus, {
-        propsData: {
-          mediator,
-        },
-        methods: {
-          handleFormSubmission: handleFormSubmissionMock,
-        },
-      });
-    });
+    beforeEach(() => {});
 
     it('renders Status component', () => {
       expect(wrapper.contains(Status)).toBe(true);
     });
 
-    it('calls handleFormSubmission when receiving an onStatusChange event from Status component', () => {
-      wrapper.find(Status).vm.$emit('onStatusChange', 'onTrack');
+    it('calls handleFormSubmission when receiving an onDropdownClick event from Status component', () => {
+      wrapper.find(Status).vm.$emit('onDropdownClick', 'onTrack');
 
-      expect(handleFormSubmissionMock).toHaveBeenCalledWith('onTrack');
+      expect(handleDropdownClickMock).toHaveBeenCalledWith('onTrack');
     });
   });
 
-  it('removes status when user clicks on "remove status"', () => {
-    const mediator = {
-      store: {
-        editable: true,
-        isFetching: {
-          status: false,
-        },
-        status: 'onTrack',
-      },
-      updateStatus(status) {
-        this.store.status = status;
-        wrapper.setProps({
-          mediator: {
-            ...this,
-          },
-        });
-        return Promise.resolve();
-      },
-    };
+  it('calls handleFormSubmission when receiving an onFormSubmit event from Status component', () => {
+    wrapper.find(Status).vm.$emit('onDropdownClick', 'onTrack');
 
-    wrapper = mount(SidebarStatus, {
-      propsData: {
-        mediator,
-      },
-    });
-
-    expect(getStatusText(wrapper)).toContain('On track');
-
-    wrapper.find('button.btn-link').trigger('click');
-
-    return Vue.nextTick().then(() => {
-      expect(getStatusText(wrapper)).toBe('None');
-    });
+    expect(handleDropdownClickMock).toHaveBeenCalledWith('onTrack');
   });
 });

ee/spec/frontend/sidebar/components/status/status_spec.js

-import { GlDeprecatedButton, GlFormRadioGroup, GlLoadingIcon, GlTooltip } from '@gitlab/ui';
+import { GlDropdown, GlDropdownItem, GlLoadingIcon, GlTooltip } from '@gitlab/ui';
 import { mount, shallowMount } from '@vue/test-utils';
 import Vue from 'vue';
 import Status from 'ee/sidebar/components/status/status.vue';
-import { healthStatus, healthStatusColorMap, healthStatusTextMap } from 'ee/sidebar/constants';
+import { healthStatus, healthStatusTextMap } from 'ee/sidebar/constants';
 
-const getStatusText = wrapper => wrapper.find('.value').text();
+const getStatusText = wrapper => wrapper.find('.value .text-plain').text();
 
 const getTooltipText = wrapper => wrapper.find(GlTooltip).text();
 
-const getStatusIconCssClasses = wrapper => wrapper.find('[name="severity-low"]').classes();
-
 const getEditButton = wrapper => wrapper.find({ ref: 'editButton' });
 
-const getRemoveStatusButton = wrapper => wrapper.find(GlDeprecatedButton);
-
-const getEditForm = wrapper => wrapper.find('form');
-
-const getRadioInputs = wrapper => wrapper.findAll('input[type="radio"]');
+const getDropdownElement = wrapper => wrapper.find(GlDropdown);
 
-const getRadioComponent = wrapper => wrapper.find(GlFormRadioGroup);
+const getRemoveStatusItem = wrapper => wrapper.find(GlDropdownItem);
 
 describe('Status', () => {
   let wrapper;
@@ -41,8 +35,7 @@ describe('Status', () => {
 
   it('shows the text "Status"', () => {
     shallowMountStatus();
-
-    expect(wrapper.find('.title').text()).toBe('Status');
+    expect(wrapper.find('.title').text()).toBe('Health status');
   });
 
   describe('loading icon', () => {
@@ -89,18 +82,7 @@ describe('Status', () => {
     });
   });
 
-  describe('remove status button', () => {
-    it('is hidden when there is no status', () => {
-      const props = {
-        isEditable: true,
-        status: '',
-      };
-
-      shallowMountStatus(props);
-
-      expect(getRemoveStatusButton(wrapper).exists()).toBe(false);
-    });
-
+  describe('remove status dropdown item', () => {
     it('is displayed when there is a status', () => {
       const props = {
         isEditable: true,
@@ -109,10 +91,14 @@ describe('Status', () => {
 
       shallowMountStatus(props);
 
-      expect(getRemoveStatusButton(wrapper).exists()).toBe(true);
+      wrapper.vm.isDropdownShowing = true;
+
+      wrapper.vm.$nextTick(() => {
+        expect(getRemoveStatusItem(wrapper).exists()).toBe(true);
+      });
     });
 
-    it('emits an onStatusChange event with argument null when clicked', () => {
+    it('emits an onDropdownClick event with argument null when clicked', () => {
       const props = {
         isEditable: true,
         status: healthStatus.AT_RISK,
@@ -120,9 +106,13 @@ describe('Status', () => {
 
       shallowMountStatus(props);
 
-      getRemoveStatusButton(wrapper).vm.$emit('click');
+      wrapper.vm.isDropdownShowing = true;
+
+      wrapper.vm.$nextTick(() => {
+        getRemoveStatusItem(wrapper).vm.$emit('click', { preventDefault: () => null });
 
-      expect(wrapper.emitted().onStatusChange[0]).toEqual([null]);
+        expect(wrapper.emitted().onDropdownClick[0]).toEqual([null]);
+      });
     });
   });
 
@@ -137,11 +127,11 @@ describe('Status', () => {
       });
 
       it('shows "None"', () => {
-        expect(getStatusText(wrapper)).toBe('None');
+        expect(wrapper.find('.no-value').text()).toBe('None');
       });
 
       it('shows "Status" in the tooltip', () => {
-        expect(getTooltipText(wrapper)).toBe('Status');
+        expect(getTooltipText(wrapper)).toBe('Health status');
       });
     });
 
@@ -159,24 +149,22 @@ describe('Status', () => {
       });
 
       it(`shows "Status: ${healthStatusTextMap[statusValue]}" in the tooltip`, () => {
-        expect(getTooltipText(wrapper)).toBe(`Status: ${healthStatusTextMap[statusValue]}`);
-      });
-
-      it(`uses ${healthStatusColorMap[statusValue]} color for the status icon`, () => {
-        expect(getStatusIconCssClasses(wrapper)).toContain(healthStatusColorMap[statusValue]);
+        expect(getTooltipText(wrapper)).toBe(`Health status: ${healthStatusTextMap[statusValue]}`);
       });
     });
   });
 
-  describe('status edit form', () => {
+  describe('status dropdown', () => {
     it('is hidden by default', () => {
       const props = {
         isEditable: true,
       };
 
-      shallowMountStatus(props);
+      mountStatus(props);
+
+      const dropdown = wrapper.find('.dropdown');
 
-      expect(getEditForm(wrapper).exists()).toBe(false);
+      expect(dropdown.classes()).toContain('d-none');
     });
 
     describe('when hidden', () => {
@@ -185,14 +173,14 @@ describe('Status', () => {
           isEditable: true,
         };
 
-        shallowMountStatus(props);
+        mountStatus(props);
       });
 
-      it('shows the form when the Edit button is clicked', () => {
+      it('shows the dropdown when the Edit button is clicked', () => {
         getEditButton(wrapper).trigger('click');
 
         return Vue.nextTick().then(() => {
-          expect(getEditForm(wrapper).exists()).toBe(true);
+          expect(wrapper.find('.dropdown').classes()).toContain('show');
         });
       });
     });
@@ -205,47 +193,43 @@ describe('Status', () => {
 
         shallowMountStatus(props);
 
-        wrapper.setData({ isFormShowing: true });
+        wrapper.setData({ isDropdownShowing: true });
       });
 
       it('shows text to ask the user to pick an option', () => {
-        const message =
-          'Choose which status most accurately reflects the current state of this issue:';
+        const message = 'Assign health status';
+
         expect(
-          getEditForm(wrapper)
-            .find('p')
+          getDropdownElement(wrapper)
+            .find('.health-title')
             .text(),
         ).toContain(message);
       });
 
-      it('hides form when the Edit button is clicked', () => {
+      it('hides form when the `edit` button is clicked', () => {
         getEditButton(wrapper).trigger('click');
 
         return Vue.nextTick().then(() => {
-          expect(getEditForm(wrapper).exists()).toBe(false);
+          expect(wrapper.find('.dropdown').classes()).toContain('d-none');
         });
       });
 
-      it('hides form when the Cancel button is clicked', () => {
-        const button = getEditForm(wrapper).find('[type="button"]');
+      it('hides form when a dropdown item is clicked', () => {
+        const dropdownItem = wrapper.findAll(GlDropdownItem).at(1);
 
-        button.vm.$emit('click');
+        dropdownItem.vm.$emit('click');
 
-        return Vue.nextTick().then(() => {
-          expect(getEditForm(wrapper).exists()).toBe(false);
-        });
-      });
-
-      it('hides form when the form is submitted', () => {
-        getEditForm(wrapper).trigger('submit');
-
-        return Vue.nextTick().then(() => {
-          expect(getEditForm(wrapper).exists()).toBe(false);
+        return wrapper.vm.$nextTick().then(() => {
+          expect(wrapper.find('.dropdown').classes()).toContain('d-none');
         });
       });
     });
 
-    describe('radio buttons', () => {
+    describe('dropdown', () => {
+      const getIterableArray = arr => {
+        return arr.map((value, index) => [value, index]);
+      };
+
       beforeEach(() => {
         const props = {
           isEditable: true,
@@ -253,29 +237,37 @@ describe('Status', () => {
 
         mountStatus(props);
 
-        wrapper.setData({ isFormShowing: true });
+        wrapper.setData({ isDropdownShowing: true });
       });
 
-      it('shows 3 radio buttons', () => {
-        expect(getRadioInputs(wrapper).length).toBe(3);
+      it('shows 4 dropdown items', () => {
+        expect(wrapper.findAll(GlDropdownItem).length).toBe(4);
       });
 
       // Test that "On track", "Needs attention", and "At risk" are displayed
-      it.each(Object.values(healthStatusTextMap))('shows "%s" text', statusText => {
-        expect(getRadioComponent(wrapper).text()).toContain(statusText);
-      });
+      it.each(getIterableArray(Object.values(healthStatusTextMap)))(
+        'shows "%s" text',
+        (statusText, index) => {
+          expect(
+            wrapper
+              .findAll(GlDropdownItem)
+              .at(index + 1) // +1 in index to account for 1st item as `No status`
+              .text(),
+          ).toContain(statusText);
+        },
+      );
 
       // Test that "onTrack", "needsAttention", and "atRisk" values are emitted when form is submitted
-      it.each(Object.values(healthStatus))(
-        'emits onStatusChange event with argument "%s" when user selects the option and submits form',
-        status => {
-          getEditForm(wrapper)
-            .find(`input[value="${status}"]`)
-            .trigger('click');
+      it.each(getIterableArray(Object.values(healthStatus)))(
+        'emits onFormSubmit event with argument "%s" when user selects the option and submits form',
+        (status, index) => {
+          wrapper
+            .findAll(GlDropdownItem)
+            .at(index + 1)
+            .vm.$emit('click', { preventDefault: () => null });
 
           return Vue.nextTick().then(() => {
-            getEditForm(wrapper).trigger('submit');
-            expect(wrapper.emitted().onStatusChange[0]).toEqual([status]);
+            expect(wrapper.emitted().onDropdownClick[0]).toEqual([status]);
           });
         },
       );

locale/gitlab.pot

@@ -3850,9 +3850,6 @@ msgstr ""
 msgid "Choose which shards you wish to synchronize to this secondary node"
 msgstr ""
 
-msgid "Choose which status most accurately reflects the current state of this issue:"
-msgstr ""
-
 msgid "Choose your framework"
 msgstr ""
 
@@ -18040,6 +18037,9 @@ msgstr ""
 msgid "Select groups to replicate"
 msgstr ""
 
+msgid "Select health status"
+msgstr ""
+
 msgid "Select labels"
 msgstr ""
 
@@ -18576,16 +18576,22 @@ msgstr ""
 msgid "Side-by-side"
 msgstr ""
 
+msgid "Sidebar|Assign health status"
+msgstr ""
+
 msgid "Sidebar|Change weight"
 msgstr ""
 
-msgid "Sidebar|None"
+msgid "Sidebar|Health status"
 msgstr ""
 
-msgid "Sidebar|Only numeral characters allowed"
+msgid "Sidebar|No status"
 msgstr ""
 
-msgid "Sidebar|Status"
+msgid "Sidebar|None"
+msgstr ""
+
+msgid "Sidebar|Only numeral characters allowed"
 msgstr ""
 
 msgid "Sidebar|Weight"
@@ -24957,9 +24963,6 @@ msgstr ""
 msgid "remove due date"
 msgstr ""
 
-msgid "remove status"
-msgstr ""
-
 msgid "remove weight"
 msgstr ""