From acded872266e08a697d455aa4f49a786d2a38146 Mon Sep 17 00:00:00 2001
From: Annabel Dunstone Gray <annabel.dunstone@gmail.com>
Date: Tue, 26 Sep 2023 11:58:32 -0600
Subject: [PATCH] Move notifications and todos to buttons at top of page

Changelog: changed
---
 .../components/sticky_header.vue              | 27 ++++++++++++++-----
 .../notes/components/discussion_counter.vue   |  7 ++++-
 .../sidebar_subscriptions_widget.vue          | 18 ++++++++++++-
 .../todo_toggle/sidebar_todo_widget.vue       | 16 ++++++++++-
 .../components/mr_more_dropdown.vue           | 12 +++++++--
 .../projects/merge_requests_controller.rb     |  1 +
 app/helpers/merge_requests_helper.rb          |  4 +++
 .../projects/merge_requests/_page.html.haml   |  6 ++++-
 .../notifications_todos_buttons.yml           |  8 ++++++
 .../user_manages_subscription_spec.rb         | 17 ++++++++++++
 10 files changed, 104 insertions(+), 12 deletions(-)
 create mode 100644 config/feature_flags/development/notifications_todos_buttons.yml

diff --git a/app/assets/javascripts/merge_requests/components/sticky_header.vue b/app/assets/javascripts/merge_requests/components/sticky_header.vue
index c1e88a901c4caf..24ac67a09643e3 100644
--- a/app/assets/javascripts/merge_requests/components/sticky_header.vue
+++ b/app/assets/javascripts/merge_requests/components/sticky_header.vue
@@ -11,6 +11,7 @@ import StatusBadge from '~/issuable/components/status_badge.vue';
 import { TYPE_MERGE_REQUEST } from '~/issues/constants';
 import DiscussionCounter from '~/notes/components/discussion_counter.vue';
 import TodoWidget from '~/sidebar/components/todo_toggle/sidebar_todo_widget.vue';
+import SubscriptionsWidget from '~/sidebar/components/subscriptions/sidebar_subscriptions_widget.vue';
 import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
 import titleSubscription from '../queries/title.subscription.graphql';
 
@@ -46,6 +47,7 @@ export default {
     DiscussionCounter,
     StatusBadge,
     TodoWidget,
+    SubscriptionsWidget,
     ClipboardButton,
   },
   directives: {
@@ -80,6 +82,9 @@ export default {
     isSignedIn() {
       return isLoggedIn();
     },
+    isNotificationsTodosButtons() {
+      return this.glFeatures.notificationsTodosButtons;
+    },
   },
   watch: {
     discussionTabCounter(val) {
@@ -189,13 +194,23 @@ export default {
           </ul>
           <div class="gl-display-none gl-lg-display-flex gl-align-items-center gl-ml-auto">
             <discussion-counter blocks-merge hide-options />
-            <todo-widget
+            <div
               v-if="isSignedIn"
-              :issuable-id="issuableId"
-              :issuable-iid="issuableIid"
-              :full-path="projectPath"
-              issuable-type="merge_request"
-            />
+              :class="{ 'gl-display-flex gl-gap-3': isNotificationsTodosButtons }"
+            >
+              <todo-widget
+                :issuable-id="issuableId"
+                :issuable-iid="issuableIid"
+                :full-path="projectPath"
+                issuable-type="merge_request"
+              />
+              <subscriptions-widget
+                v-if="isNotificationsTodosButtons"
+                :iid="issuableIid"
+                :full-path="projectPath"
+                issuable-type="merge_request"
+              />
+            </div>
           </div>
         </div>
       </div>
diff --git a/app/assets/javascripts/notes/components/discussion_counter.vue b/app/assets/javascripts/notes/components/discussion_counter.vue
index d8883f90edac56..9404fd04bc914d 100644
--- a/app/assets/javascripts/notes/components/discussion_counter.vue
+++ b/app/assets/javascripts/notes/components/discussion_counter.vue
@@ -71,6 +71,9 @@ export default {
 
       return options;
     },
+    isNotificationsTodosButtons() {
+      return this.glFeatures.notificationsTodosButtons;
+    },
   },
   methods: {
     ...mapActions(['setExpandDiscussions']),
@@ -92,10 +95,12 @@ export default {
     class="gl-display-flex discussions-counter"
   >
     <div
-      class="gl-display-flex gl-align-items-center gl-pl-4 gl-rounded-base gl-mr-3 gl-min-h-7"
+      class="gl-display-flex gl-align-items-center gl-pl-4 gl-rounded-base gl-min-h-7"
       :class="{
         'gl-bg-orange-50': blocksMerge && !allResolved,
         'gl-bg-gray-50': !blocksMerge || allResolved,
+        'gl-mr-3': !isNotificationsTodosButtons,
+        'gl-mr-5': isNotificationsTodosButtons,
       }"
       data-testid="discussions-counter-text"
     >
diff --git a/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions_widget.vue b/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions_widget.vue
index 568962cddc7fe1..54b9ad90b62419 100644
--- a/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions_widget.vue
+++ b/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions_widget.vue
@@ -1,5 +1,6 @@
 <script>
 import {
+  GlButton,
   GlDisclosureDropdownItem,
   GlDropdownForm,
   GlIcon,
@@ -30,6 +31,7 @@ export default {
     GlTooltip: GlTooltipDirective,
   },
   components: {
+    GlButton,
     GlDisclosureDropdownItem,
     GlDropdownForm,
     GlIcon,
@@ -130,6 +132,9 @@ export default {
     canSubscribe() {
       return this.emailsDisabled || !this.isLoggedIn;
     },
+    isNotificationsTodosButtons() {
+      return this.glFeatures.notificationsTodosButtons;
+    },
   },
   methods: {
     setSubscribed(subscribed) {
@@ -207,7 +212,7 @@ export default {
     </div>
   </gl-dropdown-form>
   <gl-disclosure-dropdown-item
-    v-else-if="isMovedMrSidebar"
+    v-else-if="isMovedMrSidebar && !isNotificationsTodosButtons"
     data-testid="notification-toggle"
     @action="toggleSubscribed"
   >
@@ -220,6 +225,17 @@ export default {
       />
     </template>
   </gl-disclosure-dropdown-item>
+  <gl-button
+    v-else-if="isNotificationsTodosButtons"
+    ref="tooltip"
+    v-gl-tooltip.hover.top
+    category="secondary"
+    data-testid="subscribe-button"
+    :title="notificationTooltip"
+    @click="toggleSubscribed"
+  >
+    <gl-icon :name="notificationIcon" :size="16" :class="{ 'gl-fill-blue-500': subscribed }" />
+  </gl-button>
   <sidebar-editable-item
     v-else
     ref="editable"
diff --git a/app/assets/javascripts/sidebar/components/todo_toggle/sidebar_todo_widget.vue b/app/assets/javascripts/sidebar/components/todo_toggle/sidebar_todo_widget.vue
index 1099dcb832f535..31133139bd4f9b 100644
--- a/app/assets/javascripts/sidebar/components/todo_toggle/sidebar_todo_widget.vue
+++ b/app/assets/javascripts/sidebar/components/todo_toggle/sidebar_todo_widget.vue
@@ -114,6 +114,9 @@ export default {
     tootltipTitle() {
       return todoLabel(this.hasTodo);
     },
+    isNotificationsTodosButtons() {
+      return this.glFeatures.notificationsTodosButtons;
+    },
   },
   methods: {
     toggleTodo() {
@@ -183,7 +186,18 @@ export default {
 </script>
 
 <template>
-  <div data-testid="sidebar-todo">
+  <gl-button
+    v-if="isNotificationsTodosButtons && isMergeRequest"
+    v-gl-tooltip.hover.top
+    data-testid="sidebar-todo"
+    :title="tootltipTitle"
+    category="secondary"
+    type="reset"
+    @click.stop.prevent="toggleTodo"
+  >
+    <gl-icon :class="{ 'todo-undone gl-fill-blue-500': hasTodo }" :name="collapsedButtonIcon" />
+  </gl-button>
+  <div v-else data-testid="sidebar-todo">
     <todo-button
       :issuable-type="issuableType"
       :issuable-id="issuableId"
diff --git a/app/assets/javascripts/vue_shared/components/mr_more_dropdown.vue b/app/assets/javascripts/vue_shared/components/mr_more_dropdown.vue
index 7871721f38b705..e2227904daf52f 100644
--- a/app/assets/javascripts/vue_shared/components/mr_more_dropdown.vue
+++ b/app/assets/javascripts/vue_shared/components/mr_more_dropdown.vue
@@ -143,6 +143,9 @@ export default {
     isMovedMrSidebar() {
       return this.glFeatures.movedMrSidebar;
     },
+    isNotificationsTodosButtons() {
+      return this.glFeatures.notificationsTodosButtons;
+    },
     draftLabel() {
       return this.draft ? this.$options.i18n.markAsReady : this.$options.i18n.markAsDraft;
     },
@@ -250,7 +253,9 @@ export default {
           />
         </div>
       </template>
-      <gl-disclosure-dropdown-group v-if="isLoggedIn && isMovedMrSidebar">
+      <gl-disclosure-dropdown-group
+        v-if="isLoggedIn && isMovedMrSidebar && !isNotificationsTodosButtons"
+      >
         <sidebar-subscriptions-widget
           :iid="String(mr.iid)"
           :full-path="fullPath"
@@ -261,7 +266,10 @@ export default {
 
       <gl-disclosure-dropdown-group
         bordered
-        :class="{ 'gl-mt-0! gl-pt-0! gl-border-t-0!': !(isLoggedIn && isMovedMrSidebar) }"
+        :class="{
+          'gl-mt-0! gl-pt-0! gl-border-t-0!':
+            !(isLoggedIn && isMovedMrSidebar) || isNotificationsTodosButtons,
+        }"
       >
         <gl-disclosure-dropdown-item
           v-if="canUpdateMergeRequest"
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 5cd5f8a1dcd52a..e25d78e9a7cb92 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -48,6 +48,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     push_force_frontend_feature_flag(:summarize_my_code_review, summarize_my_code_review_enabled?)
     push_frontend_feature_flag(:ci_job_failures_in_mr, project)
     push_frontend_feature_flag(:mr_pipelines_graphql, project)
+    push_frontend_feature_flag(:notifications_todos_buttons, project)
   end
 
   before_action only: [:edit] do
diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb
index 697796752091d0..12c75d882647f4 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/app/helpers/merge_requests_helper.rb
@@ -175,6 +175,10 @@ def moved_mr_sidebar_enabled?
     Feature.enabled?(:moved_mr_sidebar, @project)
   end
 
+  def notifications_todos_buttons_enabled?
+    Feature.enabled?(:notifications_todos_buttons, @project)
+  end
+
   def diffs_tab_pane_data(project, merge_request, params)
     {
       "is-locked": merge_request.discussion_locked?,
diff --git a/app/views/projects/merge_requests/_page.html.haml b/app/views/projects/merge_requests/_page.html.haml
index f99396e4155d6d..637980bd2f8bd8 100644
--- a/app/views/projects/merge_requests/_page.html.haml
+++ b/app/views/projects/merge_requests/_page.html.haml
@@ -50,7 +50,11 @@
           #js-vue-discussion-counter{ data: { blocks_merge: @project.only_allow_merge_if_all_discussions_are_resolved?.to_s } }
           - if moved_mr_sidebar_enabled?
             - if !!@issuable_sidebar.dig(:current_user, :id)
-              .js-sidebar-todo-widget-root{ data: { project_path: @issuable_sidebar[:project_full_path], iid: @issuable_sidebar[:iid], id: @issuable_sidebar[:id] } }
+              .gl-display-flex.gl-gap-3
+                .js-sidebar-todo-widget-root{ data: { project_path: @issuable_sidebar[:project_full_path], iid: @issuable_sidebar[:iid], id: @issuable_sidebar[:id] } }
+                - if notifications_todos_buttons_enabled?
+                  .js-sidebar-subscriptions-widget-root{ data: { full_path: @issuable_sidebar[:project_full_path], iid: @issuable_sidebar[:iid] } }
+
             .gl-ml-auto.gl-align-items-center.gl-display-none.gl-md-display-flex.gl-ml-3.js-expand-sidebar.gl-absolute.gl-right-5{ class: "gl-lg-display-none!" }
               = render Pajamas::ButtonComponent.new(icon: 'chevron-double-lg-left',
                 button_options: { class: 'js-sidebar-toggle' }) do
diff --git a/config/feature_flags/development/notifications_todos_buttons.yml b/config/feature_flags/development/notifications_todos_buttons.yml
new file mode 100644
index 00000000000000..5dd6a19c49eb98
--- /dev/null
+++ b/config/feature_flags/development/notifications_todos_buttons.yml
@@ -0,0 +1,8 @@
+---
+name: notifications_todos_buttons
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/132678
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/426305
+milestone: '16.5'
+type: development
+group: group::ux paper cuts
+default_enabled: false
diff --git a/spec/features/merge_request/user_manages_subscription_spec.rb b/spec/features/merge_request/user_manages_subscription_spec.rb
index 843879659897ef..cd086ca8178269 100644
--- a/spec/features/merge_request/user_manages_subscription_spec.rb
+++ b/spec/features/merge_request/user_manages_subscription_spec.rb
@@ -9,9 +9,11 @@
   let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
   let(:user) { create(:user) }
   let(:moved_mr_sidebar_enabled) { false }
+  let(:notifications_todos_buttons_enabled) { false }
 
   before do
     stub_feature_flags(moved_mr_sidebar: moved_mr_sidebar_enabled)
+    stub_feature_flags(notifications_todos_buttons: notifications_todos_buttons_enabled)
     set_cookie('new-actions-popover-viewed', 'true')
     project.add_maintainer(user)
     sign_in(user)
@@ -60,4 +62,19 @@
       expect(page).to have_selector('.gl-toggle:not(.is-checked)')
     end
   end
+
+  context 'with notifications_todos_buttons feature flag enabled' do
+    let(:moved_mr_sidebar_enabled) { true }
+    let(:notifications_todos_buttons_enabled) { true }
+
+    it 'toggles subscription' do
+      wait_for_requests
+
+      find('[data-testid="subscribe-button"]').click
+      expect(page).to have_selector('[data-testid="notifications-off-icon"]')
+
+      find('[data-testid="subscribe-button"]').click
+      expect(page).to have_selector('[data-testid="notifications-icon"]')
+    end
+  end
 end
-- 
GitLab