From 85da0f718249dfb6c9642d1d006f1e9cb2dcf9ee Mon Sep 17 00:00:00 2001 From: Chad Lavimoniere Date: Thu, 26 Oct 2023 19:51:57 -0400 Subject: [PATCH 1/4] Change Issue activity to match MRs This changes all actions in an issue exept for closing to use the small dot style used on MRs. It also changes the size of the icons in an issue's activity feed to match those in MRs. This MR also removes some custom styling that was applied in merge_requests.scss and upstreams it into issuables.scss. Changelog: fixed --- .../components/notes/system_note.vue | 31 +++++----- .../stylesheets/page_bundles/issuable.scss | 58 +++++++++++++++++++ .../page_bundles/merge_requests.scss | 57 ------------------ 3 files changed, 73 insertions(+), 73 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 81cbbf951ad01d..da47a5e5ba1903 100644 --- a/app/assets/javascripts/vue_shared/components/notes/system_note.vue +++ b/app/assets/javascripts/vue_shared/components/notes/system_note.vue @@ -29,13 +29,16 @@ import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { renderGFM } from '~/behaviors/markdown/render_gfm'; import TimelineEntryItem from './timeline_entry_item.vue'; +// Currently, only icons for issue closure, merge request approval, +// merge request closure, and merge request merge should be shown; +// for other activity types, a small dot is used. +const ALLOWED_ICONS = ['check', 'merge', 'merge-request-close', 'issue-close']; + const MAX_VISIBLE_COMMIT_LIST_COUNT = 3; -const MR_ICON_COLORS = { +const ICON_COLORS = { 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', }; @@ -76,6 +79,9 @@ export default { noteAnchorId() { return `note_${this.note.id}`; }, + isAllowedIcon() { + return ALLOWED_ICONS.includes(this.note.system_note_icon_name); + }, isTargetNote() { return this.targetNoteHash === this.noteAnchorId; }, @@ -95,15 +101,8 @@ export default { isMergeRequest() { return this.getNoteableData.noteableType === 'MergeRequest'; }, - hasIconColors() { - if (!this.isMergeRequest) return true; - - return this.isMergeRequest && MR_ICON_COLORS[this.note.system_note_icon_name]; - }, iconBgClass() { - 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'; + return ICON_COLORS[this.note.system_note_icon_name] || 'gl-bg-gray-50 gl-text-gray-600'; }, }, mounted() { @@ -140,17 +139,17 @@ export default { :class="[ iconBgClass, { - 'mr-system-note-empty gl-bg-gray-900!': !hasIconColors, - 'gl-w-6 gl-h-6 gl-mt-n1 gl-ml-2': !isMergeRequest, - 'mr-system-note-icon': isMergeRequest, + // 'gl-w-5 gl-h-5 gl-mt-1 gl-ml-3': !isMergeRequest && isAllowedIcon, + 'system-note-icon': isAllowedIcon, + 'system-note-tiny-dot gl-bg-gray-900!': !isAllowedIcon, }, ]" class="gl-float-left gl--flex-center gl-rounded-full gl-relative timeline-icon" > diff --git a/app/assets/stylesheets/page_bundles/issuable.scss b/app/assets/stylesheets/page_bundles/issuable.scss index 07614c5271a651..93aa156d96ab29 100644 --- a/app/assets/stylesheets/page_bundles/issuable.scss +++ b/app/assets/stylesheets/page_bundles/issuable.scss @@ -156,3 +156,61 @@ @include gl-font-weight-normal; } } + +.system-note-tiny-dot { + width: 8px; + height: 8px; + margin-top: 6px; + margin-left: 12px; + margin-right: 8px; + border: 2px solid var(--gray-50, $gray-50); +} + + +.system-note-icon { + width: 20px; + height: 20px; + margin-left: 6px; + + &.gl-bg-green-100 { + --bg-color: var(--green-100, #{$green-100}); + } + + &.gl-bg-red-100 { + --bg-color: var(--red-100, #{$red-100}); + } + + &.gl-bg-blue-100 { + --bg-color: var(--blue-100, #{$blue-100}); + } +} + +.system-note-icon:not(.mr-system-note-empty)::before { + content: ''; + display: block; + position: absolute; + left: calc(50% - 1px); + bottom: 100%; + width: 2px; + height: 20px; + background: linear-gradient(to bottom, transparent, var(--bg-color)); + + .system-note:first-child & { + display: none; + } +} + +.system-note-icon:not(.mr-system-note-empty)::after { + content: ''; + display: block; + position: absolute; + left: calc(50% - 1px); + top: 100%; + width: 2px; + height: 20px; + background: linear-gradient(to bottom, var(--bg-color), transparent); + + .system-note:last-child & { + display: none; + } +} diff --git a/app/assets/stylesheets/page_bundles/merge_requests.scss b/app/assets/stylesheets/page_bundles/merge_requests.scss index 3b97ab2cc4fab9..8027ffb1968141 100644 --- a/app/assets/stylesheets/page_bundles/merge_requests.scss +++ b/app/assets/stylesheets/page_bundles/merge_requests.scss @@ -1194,63 +1194,6 @@ $tabs-holder-z-index: 250; } } -.mr-system-note-icon { - width: 20px; - height: 20px; - margin-left: 6px; - - &.gl-bg-green-100 { - --bg-color: var(--green-100, #{$green-100}); - } - - &.gl-bg-red-100 { - --bg-color: var(--red-100, #{$red-100}); - } - - &.gl-bg-blue-100 { - --bg-color: var(--blue-100, #{$blue-100}); - } -} - -.mr-system-note-icon:not(.mr-system-note-empty)::before { - content: ''; - display: block; - position: absolute; - left: calc(50% - 1px); - bottom: 100%; - width: 2px; - height: 20px; - background: linear-gradient(to bottom, transparent, var(--bg-color)); - - .system-note:first-child & { - display: none; - } -} - -.mr-system-note-icon:not(.mr-system-note-empty)::after { - content: ''; - display: block; - position: absolute; - left: calc(50% - 1px); - top: 100%; - width: 2px; - height: 20px; - background: linear-gradient(to bottom, var(--bg-color), transparent); - - .system-note:last-child & { - display: none; - } -} - -.mr-system-note-empty { - width: 8px; - height: 8px; - margin-top: 6px; - margin-left: 12px; - margin-right: 8px; - border: 2px solid var(--gray-50, $gray-50); -} - .diff-file-discussions-wrapper { @include gl-w-full; -- GitLab From a3ac2036e2a0f465fc14128d717f10326cebf9ef Mon Sep 17 00:00:00 2001 From: Chad Lavimoniere Date: Thu, 26 Oct 2023 22:59:20 -0400 Subject: [PATCH 2/4] Fix jest test --- .../vue_shared/components/notes/system_note_spec.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/spec/frontend/vue_shared/components/notes/system_note_spec.js b/spec/frontend/vue_shared/components/notes/system_note_spec.js index 7f3912dcadb683..ade97290004508 100644 --- a/spec/frontend/vue_shared/components/notes/system_note_spec.js +++ b/spec/frontend/vue_shared/components/notes/system_note_spec.js @@ -61,10 +61,16 @@ describe('system note component', () => { expect(vm.classes()).toContain('target'); }); - it('should render svg icon', () => { + it('should render svg icon only for certain icons', () => { + const ALLOWED_ICONS = ['check', 'merge', 'merge-request-close', 'issue-close']; createComponent(props); - expect(vm.find('[data-testid="timeline-icon"]').exists()).toBe(true); + expect(vm.find('[data-testid="timeline-icon"]').exists()).toBe(false); + + ALLOWED_ICONS.forEach((icon) => { + createComponent({ note: { ...props.note, system_note_icon_name: icon } }); + expect(vm.find('[data-testid="timeline-icon"]').exists()).toBe(true); + }); }); // Redcarpet Markdown renderer wraps text in `

` tags -- GitLab From 3463221d58e715e6b8c625fa8b050987416da518 Mon Sep 17 00:00:00 2001 From: Chad Lavimoniere Date: Mon, 30 Oct 2023 19:44:29 +0000 Subject: [PATCH 3/4] remove commented out code --- .../javascripts/vue_shared/components/notes/system_note.vue | 1 - 1 file changed, 1 deletion(-) 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 da47a5e5ba1903..41532715522f9c 100644 --- a/app/assets/javascripts/vue_shared/components/notes/system_note.vue +++ b/app/assets/javascripts/vue_shared/components/notes/system_note.vue @@ -139,7 +139,6 @@ export default { :class="[ iconBgClass, { - // 'gl-w-5 gl-h-5 gl-mt-1 gl-ml-3': !isMergeRequest && isAllowedIcon, 'system-note-icon': isAllowedIcon, 'system-note-tiny-dot gl-bg-gray-900!': !isAllowedIcon, }, -- GitLab From 4d5c36762bc3009a7c9938916ee42dbfe2115592 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Tue, 31 Oct 2023 12:20:25 -0600 Subject: [PATCH 4/4] Update isAllowedIcon() --- .../vue_shared/components/notes/system_note.vue | 7 +------ 1 file changed, 1 insertion(+), 6 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 41532715522f9c..abebdb84c1784d 100644 --- a/app/assets/javascripts/vue_shared/components/notes/system_note.vue +++ b/app/assets/javascripts/vue_shared/components/notes/system_note.vue @@ -29,11 +29,6 @@ import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { renderGFM } from '~/behaviors/markdown/render_gfm'; import TimelineEntryItem from './timeline_entry_item.vue'; -// Currently, only icons for issue closure, merge request approval, -// merge request closure, and merge request merge should be shown; -// for other activity types, a small dot is used. -const ALLOWED_ICONS = ['check', 'merge', 'merge-request-close', 'issue-close']; - const MAX_VISIBLE_COMMIT_LIST_COUNT = 3; const ICON_COLORS = { check: 'gl-bg-green-100 gl-text-green-700', @@ -80,7 +75,7 @@ export default { return `note_${this.note.id}`; }, isAllowedIcon() { - return ALLOWED_ICONS.includes(this.note.system_note_icon_name); + return Object.keys(ICON_COLORS).includes(this.note.system_note_icon_name); }, isTargetNote() { return this.targetNoteHash === this.noteAnchorId; -- GitLab