From 7a1a3731706fdf1456f3524a3c4e7ad1cc6d6564 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 23 Mar 2023 20:44:36 +0000 Subject: [PATCH 1/2] Updated merge request activity system note icons https://gitlab.com/gitlab-org/gitlab/-/issues/387070 --- .../components/notes/system_note.vue | 33 +++++++++++++++---- .../page_bundles/merge_requests.scss | 16 +++++++++ app/controllers/concerns/issuable_actions.rb | 2 +- app/helpers/system_note_helper.rb | 12 ++++--- 4 files changed, 52 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/vue_shared/components/notes/system_note.vue b/app/assets/javascripts/vue_shared/components/notes/system_note.vue index 7c1ac389632d43..1dcefb331db595 100644 --- a/app/assets/javascripts/vue_shared/components/notes/system_note.vue +++ b/app/assets/javascripts/vue_shared/components/notes/system_note.vue @@ -30,9 +30,9 @@ import TimelineEntryItem from './timeline_entry_item.vue'; const MAX_VISIBLE_COMMIT_LIST_COUNT = 3; const MR_ICON_COLORS = { - approval: 'gl-bg-green-100 gl-text-green-700', - 'issue-close': 'gl-bg-red-100 gl-text-red-700', - 'git-merge': 'gl-bg-blue-100 gl-text-blue-700', + check: 'gl-bg-green-100 gl-text-green-700', + 'merge-request-close': 'gl-bg-red-100 gl-text-red-700', + merge: 'gl-bg-blue-100 gl-text-blue-700', }; const ICON_COLORS = { 'issue-close': 'gl-bg-blue-100 gl-text-blue-700', @@ -91,9 +91,16 @@ export default { descriptionVersion() { return this.descriptionVersions[this.note.description_version_id]; }, + isMergeRequest() { + return this.getNoteableData.noteableType; + }, + hasIconColors() { + if (!this.isMergeRequest) return true; + + return this.isMergeRequest && MR_ICON_COLORS[this.note.system_note_icon_name]; + }, iconBgClass() { - const colors = - this.getNoteableData.noteableType === 'MergeRequest' ? MR_ICON_COLORS : ICON_COLORS; + const colors = this.isMergeRequest === 'MergeRequest' ? MR_ICON_COLORS : ICON_COLORS; return colors[this.note.system_note_icon_name] || 'gl-bg-gray-50 gl-text-gray-600'; }, @@ -129,12 +136,26 @@ export default { class="note system-note note-wrapper" >
>>>>>> f36118ffd5e6 (Updated merge request activity system note icons) >
diff --git a/app/assets/stylesheets/page_bundles/merge_requests.scss b/app/assets/stylesheets/page_bundles/merge_requests.scss index b01ff498ad3844..1ca7a21e464cf5 100644 --- a/app/assets/stylesheets/page_bundles/merge_requests.scss +++ b/app/assets/stylesheets/page_bundles/merge_requests.scss @@ -1249,3 +1249,19 @@ $tabs-holder-z-index: 250; vertical-align: middle; } } + +.mr-system-note-empty { + top: -2px; +} + +.mr-system-note-icon::before { + content: ''; + display: block; + width: calc(100% + 8px); + height: calc(100% + 8px); + border-radius: 100%; + border: 4px solid var(--gray-10, var(--white)); + position: absolute; + left: -4px; + top: -4px; +} diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index b8d47586a155b5..a86a8a0415ab16 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -190,7 +190,7 @@ def notes_filter_updated? end def discussion_cache_context - [current_user&.cache_key, project.team.human_max_access(current_user&.id)].join(':') + [current_user&.cache_key, project.team.human_max_access(current_user&.id), 'v2'].join(':') end def discussion_serializer diff --git a/app/helpers/system_note_helper.rb b/app/helpers/system_note_helper.rb index a1b6e8964759e5..3d31d697452fb3 100644 --- a/app/helpers/system_note_helper.rb +++ b/app/helpers/system_note_helper.rb @@ -2,13 +2,13 @@ module SystemNoteHelper ICON_NAMES_BY_ACTION = { - 'approved' => 'approval', + 'approved' => 'check', 'unapproved' => 'unapproval', 'cherry_pick' => 'cherry-pick-commit', 'commit' => 'commit', 'description' => 'pencil', - 'merge' => 'git-merge', - 'merged' => 'git-merge', + 'merge' => 'merge', + 'merged' => 'merge', 'opened' => 'issues', 'closed' => 'issue-close', 'time_tracking' => 'timer', @@ -51,7 +51,11 @@ module SystemNoteHelper }.freeze def system_note_icon_name(note) - ICON_NAMES_BY_ACTION[note.system_note_metadata&.action] + if note.system_note_metadata&.action == 'closed' && note.for_merge_request? + 'merge-request-close' + else + ICON_NAMES_BY_ACTION[note.system_note_metadata&.action] + end end def icon_for_system_note(note) -- GitLab From 1c176cc7eb695153c1f8e2fa7e6977da4db6069b Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 31 Mar 2023 16:29:45 +0100 Subject: [PATCH 2/2] Added faded line above and below system note icon --- .../components/notes/system_note.vue | 16 ++--- .../page_bundles/merge_requests.scss | 59 ++++++++++++++++--- app/assets/stylesheets/pages/notes.scss | 56 ++++++++++++------ .../alert_management/alert/notes_spec.rb | 2 +- 4 files changed, 93 insertions(+), 40 deletions(-) diff --git a/app/assets/javascripts/vue_shared/components/notes/system_note.vue b/app/assets/javascripts/vue_shared/components/notes/system_note.vue index 1dcefb331db595..06ca90fa8c633a 100644 --- a/app/assets/javascripts/vue_shared/components/notes/system_note.vue +++ b/app/assets/javascripts/vue_shared/components/notes/system_note.vue @@ -92,7 +92,7 @@ export default { return this.descriptionVersions[this.note.description_version_id]; }, isMergeRequest() { - return this.getNoteableData.noteableType; + return this.getNoteableData.noteableType === 'MergeRequest'; }, hasIconColors() { if (!this.isMergeRequest) return true; @@ -100,7 +100,7 @@ export default { return this.isMergeRequest && MR_ICON_COLORS[this.note.system_note_icon_name]; }, iconBgClass() { - const colors = this.isMergeRequest === 'MergeRequest' ? MR_ICON_COLORS : ICON_COLORS; + const colors = this.isMergeRequest ? MR_ICON_COLORS : ICON_COLORS; return colors[this.note.system_note_icon_name] || 'gl-bg-gray-50 gl-text-gray-600'; }, @@ -136,21 +136,15 @@ export default { class="note system-note note-wrapper" >
>>>>>> f36118ffd5e6 (Updated merge request activity system note icons) + class="gl-float-left gl--flex-center gl-rounded-full gl-relative timeline-icon" > li.timeline-entry:not(:last-of-type) { - @include vertical-line(1rem); + .main-notes-list::before, + .timeline-entry:last-child::before { + content: ''; + position: absolute; + width: 2px; + left: 15px; + top: 15px; + height: calc(100% - 15px); + } + + .main-notes-list::before { + background: var(--gray-50, $gray-50); + } + + .timeline-entry:last-child::before { + background: var(--white); + + .gl-dark & { + background: var(--gray-10); + } + + &.note-comment { + top: 30px; + } } } @@ -63,6 +85,10 @@ $system-note-icon-m-left: $avatar-m-left + $icon-size-diff / $avatar-m-ratio; height: 2rem; } + .gl-avatar { + border-color: var(--gray-50, $gray-50); + } + &.note-comment, &.note-skeleton, .draft-note { @@ -265,7 +291,10 @@ $system-note-icon-m-left: $avatar-m-left + $icon-size-diff / $avatar-m-ratio; &.being-posted { pointer-events: none; - opacity: 0.5; + + .timeline-entry-inner { + opacity: 0.5; + } .dummy-avatar { background-color: $gray-100; @@ -370,9 +399,7 @@ $system-note-icon-m-left: $avatar-m-left + $icon-size-diff / $avatar-m-ratio; } .timeline-content { - @include notes-media('min', map-get($grid-breakpoints, sm)) { - margin-left: 30px; - } + margin-left: 30px; } .note-header { @@ -460,7 +487,7 @@ $system-note-icon-m-left: $avatar-m-left + $icon-size-diff / $avatar-m-ratio; } .timeline-icon { - margin: 12px 0 0 20px; + margin: 20px 0 0 28px; } } @@ -570,15 +597,6 @@ $system-note-icon-m-left: $avatar-m-left + $icon-size-diff / $avatar-m-ratio; .system-note { background-color: transparent; padding: 0; - - .timeline-icon { - margin-top: -2px; - } - - .timeline-entry-inner .timeline-icon { - margin-top: $system-note-icon-m-top; - margin-left: $system-note-icon-m-left; - } } } diff --git a/spec/requests/api/graphql/project/alert_management/alert/notes_spec.rb b/spec/requests/api/graphql/project/alert_management/alert/notes_spec.rb index 16dd0dfcfcba8c..c1ac0367853d65 100644 --- a/spec/requests/api/graphql/project/alert_management/alert/notes_spec.rb +++ b/spec/requests/api/graphql/project/alert_management/alert/notes_spec.rb @@ -51,7 +51,7 @@ expect(first_notes_result.first).to include( 'id' => first_system_note.to_global_id.to_s, - 'systemNoteIconName' => 'git-merge', + 'systemNoteIconName' => 'merge', 'body' => first_system_note.note ) end -- GitLab