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