From cbeb5a9d75a1ad58e13cc14ea0147aa6c988ee7f Mon Sep 17 00:00:00 2001
From: Annabel Dunstone Gray <annabel.dunstone@gmail.com>
Date: Thu, 22 Aug 2024 14:30:20 -0600
Subject: [PATCH 1/2] Refactor draft notes CSS

Changelog: changed
---
 .../batch_comments/components/draft_note.vue  |  4 +-
 .../stylesheets/framework/timeline.scss       |  3 +-
 app/assets/stylesheets/pages/notes.scss       | 25 +------
 .../themes/dark_mode_overrides.scss           |  2 +-
 .../assets/stylesheets/components/_index.scss |  2 +-
 .../components/batch_comments/draft_note.scss | 68 -------------------
 .../batch_comments/draft_note_v2.scss         | 50 ++++++++++++++
 .../merge_request/batch_comments_spec.rb      | 14 ++--
 .../merge_request/user_reviews_image_spec.rb  |  2 +-
 .../components/draft_note_spec.js             |  2 +-
 10 files changed, 66 insertions(+), 106 deletions(-)
 delete mode 100644 ee/app/assets/stylesheets/components/batch_comments/draft_note.scss
 create mode 100644 ee/app/assets/stylesheets/components/batch_comments/draft_note_v2.scss

diff --git a/app/assets/javascripts/batch_comments/components/draft_note.vue b/app/assets/javascripts/batch_comments/components/draft_note.vue
index 3b79dc1de6d483db..1ded43d95af2e867 100644
--- a/app/assets/javascripts/batch_comments/components/draft_note.vue
+++ b/app/assets/javascripts/batch_comments/components/draft_note.vue
@@ -86,7 +86,7 @@ export default {
     :note="draft"
     :line="line"
     :discussion-root="true"
-    class="draft-note-component draft-note !gl-mb-0"
+    class="draft-note-v2 !gl-mb-0"
     @handleEdit="handleEditing"
     @cancelForm="handleNotEditing"
     @updateSuccess="handleNotEditing"
@@ -110,7 +110,7 @@ export default {
       <div
         v-if="draftCommands"
         v-safe-html:[$options.safeHtmlConfig]="draftCommands"
-        class="referenced-commands draft-note-commands"
+        class="gl-text-subtle gl-text-sm gl-mb-2 gl-ml-3 referenced-commands-v2"
       ></div>
     </template>
   </noteable-note>
diff --git a/app/assets/stylesheets/framework/timeline.scss b/app/assets/stylesheets/framework/timeline.scss
index 4a5a27f83dacc278..9e49fe3506cbaf91 100644
--- a/app/assets/stylesheets/framework/timeline.scss
+++ b/app/assets/stylesheets/framework/timeline.scss
@@ -28,8 +28,7 @@
 .timeline-entry {
   color: $gl-text-color;
 
-  &:not(.note-form).internal-note .timeline-content,
-  &:not(.note-form).draft-note .timeline-content {
+  &:not(.note-form).internal-note .timeline-content {
     background-color: $orange-50 !important;
   }
 
diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss
index ae622c0c1b4338fb..d1ba8d8186e283f4 100644
--- a/app/assets/stylesheets/pages/notes.scss
+++ b/app/assets/stylesheets/pages/notes.scss
@@ -40,7 +40,7 @@ $system-note-icon-m-left: $avatar-m-left + $icon-size-diff / $avatar-m-ratio;
     }
   }
 
-  .timeline-entry:not(.draft-note):last-child::before {
+  .timeline-entry:not(.draft-note-v2):last-child::before {
     background: var(--white);
 
     .gl-dark & {
@@ -87,8 +87,7 @@ $system-note-icon-m-left: $avatar-m-left + $icon-size-diff / $avatar-m-ratio;
     }
 
     &.note-comment,
-    &.note-skeleton,
-    .draft-note {
+    &.note-skeleton {
       .timeline-avatar {
         margin-top: 5px;
       }
@@ -104,10 +103,6 @@ $system-note-icon-m-left: $avatar-m-left + $icon-size-diff / $avatar-m-ratio;
         background-color: $white;
       }
 
-      &.draft-note .timeline-content {
-        border: 0;
-      }
-
       .note-header-info {
         min-height: 2rem;
         display: flex;
@@ -147,12 +142,6 @@ $system-note-icon-m-left: $avatar-m-left + $icon-size-diff / $avatar-m-ratio;
             background-color: $white;
           }
 
-          &.draft-note .timeline-content {
-            margin-left: 0;
-            border-top-left-radius: 0;
-            border-top-right-radius: 0;
-          }
-
           &:not(:first-of-type) .timeline-entry-inner {
             margin-left: 2.5rem;
             border-left: 1px solid $border-color;
@@ -1054,16 +1043,6 @@ $system-note-icon-m-left: $avatar-m-left + $icon-size-diff / $avatar-m-ratio;
     }
   }
 
-  .draft-note-component.draft-note.timeline-entry {
-    .timeline-content {
-      padding: $gl-padding-8 $gl-padding-8 $gl-padding-8 $gl-padding;
-    }
-
-    .timeline-avatar {
-      margin: $avatar-m-top 0 0 $avatar-m-left;
-    }
-  }
-
   .diff-comment-form {
     display: block;
   }
diff --git a/app/assets/stylesheets/themes/dark_mode_overrides.scss b/app/assets/stylesheets/themes/dark_mode_overrides.scss
index 55c47ddb5847dfc9..4e5b6cd30d3d9d74 100644
--- a/app/assets/stylesheets/themes/dark_mode_overrides.scss
+++ b/app/assets/stylesheets/themes/dark_mode_overrides.scss
@@ -92,7 +92,7 @@ aside.right-sidebar:not(.right-sidebar-merge-requests) {
 }
 
 .timeline-entry.internal-note:not(.note-form) .timeline-content,
-.timeline-entry.draft-note:not(.note-form) .timeline-content {
+.timeline-entry.draft-note-v2:not(.note-form) .timeline-content {
   // soften on darkmode
   background-color: mix($gray-50, $orange-50, 75%) !important;
 }
diff --git a/ee/app/assets/stylesheets/components/_index.scss b/ee/app/assets/stylesheets/components/_index.scss
index 05b3d2eae20e5908..5bd63e2dfd58db5b 100644
--- a/ee/app/assets/stylesheets/components/_index.scss
+++ b/ee/app/assets/stylesheets/components/_index.scss
@@ -2,7 +2,7 @@
 @import './tanuki_bot';
 @import './audit_logs/logs_table';
 @import './banner';
-@import './batch_comments/draft_note';
+@import './batch_comments/draft_note_v2';
 @import './compliance_dashboard/dashboard';
 @import './generic_vulnerability_report';
 @import './related_items_tree';
diff --git a/ee/app/assets/stylesheets/components/batch_comments/draft_note.scss b/ee/app/assets/stylesheets/components/batch_comments/draft_note.scss
deleted file mode 100644
index 85e8dc05984f007b..0000000000000000
--- a/ee/app/assets/stylesheets/components/batch_comments/draft_note.scss
+++ /dev/null
@@ -1,68 +0,0 @@
-$draft-actions-left-margin: 40px + $gl-padding;
-
-.draft-note-component {
-  margin: 0;
-
-  .referenced-commands.draft-note-commands {
-    color: var(--gl-text-color-subtle);
-    font-size: $label-font-size;
-  }
-
-  .timeline-entry {
-    background-color: transparent;
-  }
-
-  .draft-note-actions {
-    margin: 0;
-  }
-}
-
-.diff-file:not(.discussion-wrapper) .note.draft-note {
-  margin: 0 -1px;
-  border-radius: 3px;
-
-  .timeline-entry-inner {
-    border: 1px solid $orange-400;
-  }
-
-  &.is-editing {
-    margin-bottom: 0;
-  }
-}
-
-.diff-file.discussion-wrapper .note.draft-note {
-  .timeline-entry-inner {
-    border-bottom-left-radius: $gl-border-radius-base;
-    border-bottom-right-radius: $gl-border-radius-base;
-    background-color: $orange-50 !important;
-  }
-}
-
-.discussion-body,
-.diff-file {
-  .notes_holder {
-    .notes-content {
-      .notes {
-        &.draft-notes {
-          &::before {
-            border: 0;
-          }
-
-          background-color: transparent;
-        }
-      }
-    }
-  }
-}
-
-.diffs .draft-note-component .referenced-commands.draft-note-commands {
-  margin: 0;
-  border-radius: 0;
-}
-
-.referenced-commands.draft-note-commands {
-  color: var(--gl-text-color-subtle);
-  background-color: transparent;
-  border: 0;
-  padding: $gl-padding-8 0 0;
-}
diff --git a/ee/app/assets/stylesheets/components/batch_comments/draft_note_v2.scss b/ee/app/assets/stylesheets/components/batch_comments/draft_note_v2.scss
new file mode 100644
index 0000000000000000..2bfd3ddeaf96c3aa
--- /dev/null
+++ b/ee/app/assets/stylesheets/components/batch_comments/draft_note_v2.scss
@@ -0,0 +1,50 @@
+// Draft notes base styles
+
+.draft-note-v2 {
+  .note-header-info {
+    padding: $gl-spacing-scale-2 $gl-spacing-scale-3;
+  }
+
+  .timeline-content {
+    background-color: $orange-50 !important;
+    padding: $gl-spacing-scale-2 $gl-spacing-scale-3;
+    margin-left: $gl-spacing-scale-8;
+    border-radius: $gl-border-radius-base;
+  }
+}
+
+.referenced-commands-v2 p {
+  margin: 0;
+}
+
+
+// Draft notes on diffs on overview tab
+
+.diff-content {
+  .draft-note-v2 .timeline-content {
+    margin-left: 0;
+    border-top-left-radius: 0;
+    border-top-right-radius: 0;
+  }
+}
+
+
+// Draft notes on diffs on changes tab
+
+.files {
+  .draft-note-v2 {
+    margin: 0 -1px;
+
+    .timeline-entry-inner {
+      border: 1px solid $orange-400;
+    }
+
+    .timeline-content {
+      border-radius: 0;
+    }
+
+    .timeline-avatar {
+      margin: $gl-spacing-scale-3 0 0 $gl-spacing-scale-5;
+    }
+  }
+}
diff --git a/spec/features/merge_request/batch_comments_spec.rb b/spec/features/merge_request/batch_comments_spec.rb
index 59a07142b9f753f1..90ce5beaf25a712a 100644
--- a/spec/features/merge_request/batch_comments_spec.rb
+++ b/spec/features/merge_request/batch_comments_spec.rb
@@ -23,7 +23,7 @@
   it 'adds draft note' do
     write_diff_comment
 
-    expect(find('.draft-note-component')).to have_content('Line is wrong')
+    expect(find('.draft-note-v2')).to have_content('Line is wrong')
 
     expect(page).to have_selector('[data-testid="review_bar_component"]')
 
@@ -40,7 +40,7 @@
 
     wait_for_requests
 
-    expect(page).not_to have_selector('.draft-note-component', text: 'Line is wrong')
+    expect(page).not_to have_selector('.draft-note-v2', text: 'Line is wrong')
 
     expect(page).to have_selector('.note:not(.draft-note)', text: 'Line is wrong')
   end
@@ -58,7 +58,7 @@
 
     wait_for_requests
 
-    expect(page).not_to have_selector('.draft-note-component', text: 'Line is wrong')
+    expect(page).not_to have_selector('.draft-note-v2', text: 'Line is wrong')
   end
 
   it 'edits draft note' do
@@ -73,7 +73,7 @@
 
     write_comment(text: 'Testing update', button_text: 'Save comment')
 
-    expect(page).to have_selector('.draft-note-component', text: 'Testing update')
+    expect(page).to have_selector('.draft-note-v2', text: 'Testing update')
   end
 
   context 'multiple times on the same diff line' do
@@ -129,7 +129,7 @@
       it 'can add comment to review' do
         write_comment(selector: '.js-main-target-form', field: 'note-body', text: 'Its a draft comment', button_text: 'Add comment to review')
 
-        expect(page).to have_selector('.draft-note-component', text: 'Its a draft comment')
+        expect(page).to have_selector('.draft-note-v2', text: 'Its a draft comment')
 
         click_button('Pending comments')
 
@@ -158,8 +158,8 @@
       write_parallel_comment('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9')
       write_parallel_comment('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9', button_text: 'Add to review', text: 'Another wrong line')
 
-      expect(find('.new .draft-note-component')).to have_content('Line is wrong')
-      expect(find('.old .draft-note-component')).to have_content('Another wrong line')
+      expect(find('.new .draft-note-v2')).to have_content('Line is wrong')
+      expect(find('.old .draft-note-v2')).to have_content('Another wrong line')
 
       expect(find('.review-bar-content .gl-badge')).to have_content('2')
     end
diff --git a/spec/features/merge_request/user_reviews_image_spec.rb b/spec/features/merge_request/user_reviews_image_spec.rb
index 815b006d029bed74..8dbc6d3a9d47e2d4 100644
--- a/spec/features/merge_request/user_reviews_image_spec.rb
+++ b/spec/features/merge_request/user_reviews_image_spec.rb
@@ -30,7 +30,7 @@
 
     wait_for_requests
 
-    page.within(find('.draft-note-component')) do
+    page.within(find('.draft-note-v2')) do
       expect(page).to have_content('image diff test comment')
     end
   end
diff --git a/spec/frontend/batch_comments/components/draft_note_spec.js b/spec/frontend/batch_comments/components/draft_note_spec.js
index b6042b4aa81ac873..59cf93915b44135c 100644
--- a/spec/frontend/batch_comments/components/draft_note_spec.js
+++ b/spec/frontend/batch_comments/components/draft_note_spec.js
@@ -103,7 +103,7 @@ describe('Batch comments draft note component', () => {
       });
 
       await nextTick();
-      const referencedCommands = wrapper.find('.referenced-commands');
+      const referencedCommands = wrapper.find('.referenced-commands-v2');
 
       expect(referencedCommands.exists()).toBe(true);
       expect(referencedCommands.text()).toContain('test command');
-- 
GitLab


From a43f4509eaf0667ca009ee7a95571eca5f3c74cf Mon Sep 17 00:00:00 2001
From: Annabel Dunstone Gray <annabel.dunstone@gmail.com>
Date: Wed, 28 Aug 2024 05:47:19 -0600
Subject: [PATCH 2/2] Remove -v2 from draft notes

---
 .../batch_comments/components/draft_note.vue       |  4 ++--
 app/assets/stylesheets/pages/notes.scss            |  2 +-
 .../stylesheets/themes/dark_mode_overrides.scss    |  2 +-
 ee/app/assets/stylesheets/components/_index.scss   |  2 +-
 .../{draft_note_v2.scss => draft_note.scss}        |  8 ++++----
 spec/features/merge_request/batch_comments_spec.rb | 14 +++++++-------
 .../merge_request/user_reviews_image_spec.rb       |  2 +-
 .../batch_comments/components/draft_note_spec.js   |  2 +-
 8 files changed, 18 insertions(+), 18 deletions(-)
 rename ee/app/assets/stylesheets/components/batch_comments/{draft_note_v2.scss => draft_note.scss} (88%)

diff --git a/app/assets/javascripts/batch_comments/components/draft_note.vue b/app/assets/javascripts/batch_comments/components/draft_note.vue
index 1ded43d95af2e867..d8fe476facd20fe2 100644
--- a/app/assets/javascripts/batch_comments/components/draft_note.vue
+++ b/app/assets/javascripts/batch_comments/components/draft_note.vue
@@ -86,7 +86,7 @@ export default {
     :note="draft"
     :line="line"
     :discussion-root="true"
-    class="draft-note-v2 !gl-mb-0"
+    class="draft-note !gl-mb-0"
     @handleEdit="handleEditing"
     @cancelForm="handleNotEditing"
     @updateSuccess="handleNotEditing"
@@ -110,7 +110,7 @@ export default {
       <div
         v-if="draftCommands"
         v-safe-html:[$options.safeHtmlConfig]="draftCommands"
-        class="gl-text-subtle gl-text-sm gl-mb-2 gl-ml-3 referenced-commands-v2"
+        class="draft-note-referenced-commands gl-mb-2 gl-ml-3 gl-text-sm gl-text-subtle"
       ></div>
     </template>
   </noteable-note>
diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss
index d1ba8d8186e283f4..265d3305f87b09ca 100644
--- a/app/assets/stylesheets/pages/notes.scss
+++ b/app/assets/stylesheets/pages/notes.scss
@@ -40,7 +40,7 @@ $system-note-icon-m-left: $avatar-m-left + $icon-size-diff / $avatar-m-ratio;
     }
   }
 
-  .timeline-entry:not(.draft-note-v2):last-child::before {
+  .timeline-entry:not(.draft-note):last-child::before {
     background: var(--white);
 
     .gl-dark & {
diff --git a/app/assets/stylesheets/themes/dark_mode_overrides.scss b/app/assets/stylesheets/themes/dark_mode_overrides.scss
index 4e5b6cd30d3d9d74..55c47ddb5847dfc9 100644
--- a/app/assets/stylesheets/themes/dark_mode_overrides.scss
+++ b/app/assets/stylesheets/themes/dark_mode_overrides.scss
@@ -92,7 +92,7 @@ aside.right-sidebar:not(.right-sidebar-merge-requests) {
 }
 
 .timeline-entry.internal-note:not(.note-form) .timeline-content,
-.timeline-entry.draft-note-v2:not(.note-form) .timeline-content {
+.timeline-entry.draft-note:not(.note-form) .timeline-content {
   // soften on darkmode
   background-color: mix($gray-50, $orange-50, 75%) !important;
 }
diff --git a/ee/app/assets/stylesheets/components/_index.scss b/ee/app/assets/stylesheets/components/_index.scss
index 5bd63e2dfd58db5b..05b3d2eae20e5908 100644
--- a/ee/app/assets/stylesheets/components/_index.scss
+++ b/ee/app/assets/stylesheets/components/_index.scss
@@ -2,7 +2,7 @@
 @import './tanuki_bot';
 @import './audit_logs/logs_table';
 @import './banner';
-@import './batch_comments/draft_note_v2';
+@import './batch_comments/draft_note';
 @import './compliance_dashboard/dashboard';
 @import './generic_vulnerability_report';
 @import './related_items_tree';
diff --git a/ee/app/assets/stylesheets/components/batch_comments/draft_note_v2.scss b/ee/app/assets/stylesheets/components/batch_comments/draft_note.scss
similarity index 88%
rename from ee/app/assets/stylesheets/components/batch_comments/draft_note_v2.scss
rename to ee/app/assets/stylesheets/components/batch_comments/draft_note.scss
index 2bfd3ddeaf96c3aa..6caa17d1e58fd69d 100644
--- a/ee/app/assets/stylesheets/components/batch_comments/draft_note_v2.scss
+++ b/ee/app/assets/stylesheets/components/batch_comments/draft_note.scss
@@ -1,6 +1,6 @@
 // Draft notes base styles
 
-.draft-note-v2 {
+.draft-note {
   .note-header-info {
     padding: $gl-spacing-scale-2 $gl-spacing-scale-3;
   }
@@ -13,7 +13,7 @@
   }
 }
 
-.referenced-commands-v2 p {
+.draft-note-referenced-commands p {
   margin: 0;
 }
 
@@ -21,7 +21,7 @@
 // Draft notes on diffs on overview tab
 
 .diff-content {
-  .draft-note-v2 .timeline-content {
+  .draft-note .timeline-content {
     margin-left: 0;
     border-top-left-radius: 0;
     border-top-right-radius: 0;
@@ -32,7 +32,7 @@
 // Draft notes on diffs on changes tab
 
 .files {
-  .draft-note-v2 {
+  .draft-note {
     margin: 0 -1px;
 
     .timeline-entry-inner {
diff --git a/spec/features/merge_request/batch_comments_spec.rb b/spec/features/merge_request/batch_comments_spec.rb
index 90ce5beaf25a712a..a57036c42c7eb6ae 100644
--- a/spec/features/merge_request/batch_comments_spec.rb
+++ b/spec/features/merge_request/batch_comments_spec.rb
@@ -23,7 +23,7 @@
   it 'adds draft note' do
     write_diff_comment
 
-    expect(find('.draft-note-v2')).to have_content('Line is wrong')
+    expect(find('.draft-note')).to have_content('Line is wrong')
 
     expect(page).to have_selector('[data-testid="review_bar_component"]')
 
@@ -40,7 +40,7 @@
 
     wait_for_requests
 
-    expect(page).not_to have_selector('.draft-note-v2', text: 'Line is wrong')
+    expect(page).not_to have_selector('.draft-note', text: 'Line is wrong')
 
     expect(page).to have_selector('.note:not(.draft-note)', text: 'Line is wrong')
   end
@@ -58,7 +58,7 @@
 
     wait_for_requests
 
-    expect(page).not_to have_selector('.draft-note-v2', text: 'Line is wrong')
+    expect(page).not_to have_selector('.draft-note', text: 'Line is wrong')
   end
 
   it 'edits draft note' do
@@ -73,7 +73,7 @@
 
     write_comment(text: 'Testing update', button_text: 'Save comment')
 
-    expect(page).to have_selector('.draft-note-v2', text: 'Testing update')
+    expect(page).to have_selector('.draft-note', text: 'Testing update')
   end
 
   context 'multiple times on the same diff line' do
@@ -129,7 +129,7 @@
       it 'can add comment to review' do
         write_comment(selector: '.js-main-target-form', field: 'note-body', text: 'Its a draft comment', button_text: 'Add comment to review')
 
-        expect(page).to have_selector('.draft-note-v2', text: 'Its a draft comment')
+        expect(page).to have_selector('.draft-note', text: 'Its a draft comment')
 
         click_button('Pending comments')
 
@@ -158,8 +158,8 @@
       write_parallel_comment('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9')
       write_parallel_comment('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9', button_text: 'Add to review', text: 'Another wrong line')
 
-      expect(find('.new .draft-note-v2')).to have_content('Line is wrong')
-      expect(find('.old .draft-note-v2')).to have_content('Another wrong line')
+      expect(find('.new .draft-note')).to have_content('Line is wrong')
+      expect(find('.old .draft-note')).to have_content('Another wrong line')
 
       expect(find('.review-bar-content .gl-badge')).to have_content('2')
     end
diff --git a/spec/features/merge_request/user_reviews_image_spec.rb b/spec/features/merge_request/user_reviews_image_spec.rb
index 8dbc6d3a9d47e2d4..f95926bd92803387 100644
--- a/spec/features/merge_request/user_reviews_image_spec.rb
+++ b/spec/features/merge_request/user_reviews_image_spec.rb
@@ -30,7 +30,7 @@
 
     wait_for_requests
 
-    page.within(find('.draft-note-v2')) do
+    page.within(find('.draft-note')) do
       expect(page).to have_content('image diff test comment')
     end
   end
diff --git a/spec/frontend/batch_comments/components/draft_note_spec.js b/spec/frontend/batch_comments/components/draft_note_spec.js
index 59cf93915b44135c..b8b22aabf3b53a81 100644
--- a/spec/frontend/batch_comments/components/draft_note_spec.js
+++ b/spec/frontend/batch_comments/components/draft_note_spec.js
@@ -103,7 +103,7 @@ describe('Batch comments draft note component', () => {
       });
 
       await nextTick();
-      const referencedCommands = wrapper.find('.referenced-commands-v2');
+      const referencedCommands = wrapper.find('.draft-note-referenced-commands');
 
       expect(referencedCommands.exists()).toBe(true);
       expect(referencedCommands.text()).toContain('test command');
-- 
GitLab