From 641b0d1f03fd3a39c9cbb7566402ecd93065d1e3 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Tue, 24 Oct 2023 14:07:18 -0600 Subject: [PATCH 1/2] Remove avatars from user profile activity - Remove repeated usernames, names, and avatars from overview and activity tabs - Update styling - Add vertical line to match MR/issue activity - Add missing approval icon - Keep icons visible on mobile Changelog: changed --- app/assets/stylesheets/pages/notes.scss | 42 +++++++++++++++--------- app/helpers/events_helper.rb | 33 ++++++++++++------- app/views/events/_event.html.haml | 2 +- app/views/events/event/_common.html.haml | 4 +-- app/views/events/event/_design.html.haml | 2 +- app/views/events/event/_note.html.haml | 2 +- app/views/events/event/_wiki.html.haml | 2 +- app/views/users/_overview.html.haml | 2 +- app/views/users/show.html.haml | 16 +++++---- spec/helpers/events_helper_spec.rb | 41 +++++++++++++++++++++-- 10 files changed, 102 insertions(+), 44 deletions(-) diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 2722893d04c32b..8be53060fcf833 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -10,16 +10,13 @@ $icon-size-diff: $avatar-icon-size - $system-note-icon-size; $system-note-icon-m-top: $avatar-m-top + $icon-size-diff - 1.3rem; $system-note-icon-m-left: $avatar-m-left + $icon-size-diff / $avatar-m-ratio; -@mixin vertical-line($left) { - &::before { - content: ''; - border-left: 2px solid var(--gray-50, $gray-50); - position: absolute; - top: 16px; - bottom: 0; - left: calc(#{$left} - 1px); - height: calc(100% + 20px); - } +@mixin vertical-line($top, $left) { + content: ''; + position: absolute; + width: 2px; + left: $left; + top: $top; + height: calc(100% - #{$top}); } @mixin outline-comment() { @@ -32,12 +29,7 @@ $system-note-icon-m-left: $avatar-m-left + $icon-size-diff / $avatar-m-ratio; .limited-width-notes { .main-notes-list::before, .timeline-entry:last-child::before { - content: ''; - position: absolute; - width: 2px; - left: 15px; - top: 15px; - height: calc(100% - 15px); + @include vertical-line(15px, 15px); } .main-notes-list::before { @@ -1143,6 +1135,24 @@ $system-note-icon-m-left: $avatar-m-left + $icon-size-diff / $avatar-m-ratio; } } +.user-activity-content { + &::before { + @include vertical-line(80px, 25px); + background: var(--gray-50, $gray-50); + } + + .system-note-image { + @include gl-display-block; + top: 15px; + width: 22px; + height: 22px; + + svg { + fill: $gray-600 !important; + } + } +} + //This needs to be deleted when Snippet/Commit comments are convered to Vue // See https://gitlab.com/gitlab-org/gitlab-foss/issues/53918#note_117038785 .unstyled-comments { diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb index 795d35ec81fc15..30af9aec5dfa31 100644 --- a/app/helpers/events_helper.rb +++ b/app/helpers/events_helper.rb @@ -13,7 +13,10 @@ module EventsHelper 'deleted' => 'remove', 'destroyed' => 'remove', 'imported' => 'import', - 'joined' => 'users' + 'joined' => 'users', + 'approved' => 'check', + 'added' => 'upload', + 'removed' => 'remove' }.freeze def localized_action_name_map @@ -242,7 +245,7 @@ def event_note_target_url(event) def event_wiki_title_html(event) capture do - concat content_tag(:span, _('wiki page'), class: "event-target-type gl-mr-2") + concat content_tag(:span, _('wiki page'), class: "event-target-type gl-mr-2 #{user_profile_activity_classes}") concat link_to( event.target_title, event_wiki_page_target_url(event), @@ -254,7 +257,7 @@ def event_wiki_title_html(event) def event_design_title_html(event) capture do - concat content_tag(:span, _('design'), class: "event-target-type gl-mr-2") + concat content_tag(:span, _('design'), class: "event-target-type gl-mr-2 #{user_profile_activity_classes}") concat link_to( event.design.reference_link_text, design_url(event.design), @@ -303,9 +306,9 @@ def design_event_icon(action, size: 24) end def icon_for_profile_event(event) - if current_path?('users#show') - content_tag :div, class: "system-note-image #{event.action_name.parameterize}-icon" do - icon_for_event(event.action_name) + if current_path?('users#activity') + content_tag :div, class: "system-note-image #{event.action_name.parameterize}-icon gl-p-2 gl-rounded-full gl-bg-gray-50 gl-line-height-0" do + icon_for_event(event.action_name, size: 14) end else content_tag :div, class: 'system-note-image user-avatar' do @@ -315,7 +318,7 @@ def icon_for_profile_event(event) end def inline_event_icon(event) - unless current_path?('users#show') + unless current_path?('users#activity') content_tag :span, class: "system-note-image-inline d-none d-sm-flex gl-mr-2 #{event.action_name.parameterize}-icon align-self-center" do next design_event_icon(event.action, size: 14) if event.design? @@ -325,13 +328,21 @@ def inline_event_icon(event) end def event_user_info(event) - content_tag(:div, class: "event-user-info") do - concat content_tag(:span, link_to_author(event), class: "author-name") - concat " ".html_safe - concat content_tag(:span, event.author.to_reference, class: "username") + unless current_path?('users#activity') + content_tag(:div, class: "event-user-info") do + concat content_tag(:span, link_to_author(event), class: "author-name") + concat " ".html_safe + concat content_tag(:span, event.author.to_reference, class: "username") + end end end + def user_profile_activity_classes + return '' unless current_path?('users#activity') + + ' gl-font-weight-bold gl-text-black-normal' + end + private def design_url(design, opts = {}) diff --git a/app/views/events/_event.html.haml b/app/views/events/_event.html.haml index a3be188854d4ff..698192802b2e6d 100644 --- a/app/views/events/_event.html.haml +++ b/app/views/events/_event.html.haml @@ -1,7 +1,7 @@ - event = event.present - if event.visible_to_user?(current_user) - .event-item + .event-item{ class: current_path?('users#activity') ? 'user-profile-activity gl-border-bottom-0 gl-pl-7! gl-pb-6' : '' } .event-item-timestamp.gl-font-sm #{time_ago_with_tooltip(event.created_at)} diff --git a/app/views/events/event/_common.html.haml b/app/views/events/event/_common.html.haml index 7ef3461a7fbf78..78ce24c429a2cf 100644 --- a/app/views/events/event/_common.html.haml +++ b/app/views/events/event/_common.html.haml @@ -5,9 +5,9 @@ .event-title.d-flex.flex-wrap = inline_event_icon(event) - if event.target - %span.event-type.d-inline-block.gl-mr-2{ class: event.action_name } + %span.event-type.d-inline-block.gl-mr-2{ class: event.action_name + user_profile_activity_classes } = localized_action_name(event) - %span.event-target-type.gl-mr-2= event.target_type_name + %span.event-target-type.gl-mr-2{ class: user_profile_activity_classes }= event.target_type_name = link_to event_target_path(event), class: 'has-tooltip event-target-link gl-mr-2', title: event.target_title do = event.target.reference_link_text - unless event.milestone? diff --git a/app/views/events/event/_design.html.haml b/app/views/events/event/_design.html.haml index c1fa1aaca501bd..945c7465ea8ef2 100644 --- a/app/views/events/event/_design.html.haml +++ b/app/views/events/event/_design.html.haml @@ -4,7 +4,7 @@ .event-title.d-flex.flex-wrap = inline_event_icon(event) - %span.event-type.d-inline-block.gl-mr-2{ class: event.action_name } + %span.event-type.d-inline-block.gl-mr-2{ class: event.action_name + user_profile_activity_classes } = event.action_name = event_design_title_html(event) = render "events/event_scope", event: event diff --git a/app/views/events/event/_note.html.haml b/app/views/events/event/_note.html.haml index 53c59474d833c0..5bbece84e406b4 100644 --- a/app/views/events/event/_note.html.haml +++ b/app/views/events/event/_note.html.haml @@ -6,7 +6,7 @@ .event-title.d-flex.flex-wrap = inline_event_icon(event) - %span.event-type.d-inline-block.gl-mr-2{ class: event.action_name } + %span.event-type.d-inline-block.gl-mr-2{ class: event.action_name + user_profile_activity_classes } = event.action_name = event_note_title_html(event) - title = note_target_title(event.target) diff --git a/app/views/events/event/_wiki.html.haml b/app/views/events/event/_wiki.html.haml index cbd5ebcae1224f..a48c34f80d832f 100644 --- a/app/views/events/event/_wiki.html.haml +++ b/app/views/events/event/_wiki.html.haml @@ -4,7 +4,7 @@ .event-title.d-flex.flex-wrap = inline_event_icon(event) - %span.event-type.d-inline-block.gl-mr-2{ class: event.action_name } + %span.event-type.d-inline-block.gl-mr-2{ class: event.action_name + user_profile_activity_classes } = event.action_name = event_wiki_title_html(event) = render "events/event_scope", event: event diff --git a/app/views/users/_overview.html.haml b/app/views/users/_overview.html.haml index 3649f72c9566b0..a4f0cec2c63f1b 100644 --- a/app/views/users/_overview.html.haml +++ b/app/views/users/_overview.html.haml @@ -33,7 +33,7 @@ %h4.gl-flex-grow-1 = Feature.enabled?(:security_auto_fix) && @user.bot? ? s_('UserProfile|Bot activity') : s_('UserProfile|Activity') = link_to s_('UserProfile|View all'), user_activity_path, class: "hide js-view-all" - .overview-content-list{ data: { href: user_activity_path, testid: 'user-activity-content' } } + .overview-content-list.user-activity-content{ data: { href: user_activity_path, testid: 'user-activity-content' } } = gl_loading_icon(size: 'md', css_class: 'loading') - unless Feature.enabled?(:security_auto_fix) && @user.bot? diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml index 16b69a4298b415..c4a55a7d7ed928 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -171,13 +171,15 @@ - if profile_tab?(:activity) #activity.tab-pane - .flash-container - - if can?(current_user, :read_cross_project) - %h4.prepend-top-20 - = s_('UserProfile|Most Recent Activity') - .content_list{ data: { href: user_activity_path } } - .loading - = gl_loading_icon(size: 'md') + .row + .col-12 + .flash-container + - if can?(current_user, :read_cross_project) + %h4.prepend-top-20 + = s_('UserProfile|Most Recent Activity') + .content_list.user-activity-content{ data: { href: user_activity_path } } + .loading + = gl_loading_icon(size: 'md') - unless @user.bot? - if profile_tab?(:groups) #groups.tab-pane diff --git a/spec/helpers/events_helper_spec.rb b/spec/helpers/events_helper_spec.rb index 6ffca87636120c..26e771312e8068 100644 --- a/spec/helpers/events_helper_spec.rb +++ b/spec/helpers/events_helper_spec.rb @@ -268,12 +268,25 @@ describe '#event_wiki_title_html' do let(:event) { create(:wiki_page_event) } + let(:url) { helper.event_wiki_page_target_url(event) } + let(:title) { event.target_title } it 'produces a suitable title chunk' do - url = helper.event_wiki_page_target_url(event) - title = event.target_title html = [ - "wiki page", + "wiki page", + "", + title, + "" + ].join + + expect(helper.event_wiki_title_html(event)).to eq(html) + end + + it 'produces a suitable title chunk on the user profile' do + allow(helper).to receive(:user_profile_activity_classes).and_return('gl-font-weight-bold gl-text-black-normal') + + html = [ + "wiki page", "", title, "" @@ -441,5 +454,27 @@ def can_read_design_activity(object, ability) end end end + + describe '#user_profile_activity_classes' do + context 'when on the user activity page' do + before do + allow(helper).to receive(:current_path?).with('users#activity').and_return(true) + end + + it 'returns the expected class names' do + expect(helper.user_profile_activity_classes).to eq(' gl-font-weight-bold gl-text-black-normal') + end + end + + context 'when not on the user activity page' do + before do + allow(helper).to receive(:current_path?).with('users#activity').and_return(false) + end + + it 'returns an empty string' do + expect(helper.user_profile_activity_classes).to eq('') + end + end + end end # rubocop:enable RSpec/FactoryBot/AvoidCreate -- GitLab From b8e65cf1181e98ffcd83b0ac5def4d21b4dd17b5 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Fri, 27 Oct 2023 10:19:06 -0600 Subject: [PATCH 2/2] Create and update specs --- .../javascripts/pages/users/user_tabs.js | 2 +- app/assets/stylesheets/pages/notes.scss | 4 +- app/helpers/events_helper.rb | 33 +++---- app/views/events/_event.html.haml | 2 +- app/views/users/_overview.html.haml | 2 +- spec/features/users/overview_spec.rb | 8 +- spec/helpers/events_helper_spec.rb | 85 ++++++++++++++++--- 7 files changed, 98 insertions(+), 38 deletions(-) diff --git a/app/assets/javascripts/pages/users/user_tabs.js b/app/assets/javascripts/pages/users/user_tabs.js index 430022f9a9b3f5..d368c76b6ccd6b 100644 --- a/app/assets/javascripts/pages/users/user_tabs.js +++ b/app/assets/javascripts/pages/users/user_tabs.js @@ -194,7 +194,7 @@ export default class UserTabs { this.loadActivityCalendar(); UserTabs.renderMostRecentBlocks('#js-overview .activities-block', { - requestParams: { limit: 10 }, + requestParams: { limit: 15 }, }); UserTabs.renderMostRecentBlocks('#js-overview .projects-block', { requestParams: { limit: 10, skip_pagination: true, skip_namespace: true, compact_mode: true }, diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 8be53060fcf833..8e0fab04ab29d5 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -1142,8 +1142,8 @@ $system-note-icon-m-left: $avatar-m-left + $icon-size-diff / $avatar-m-ratio; } .system-note-image { - @include gl-display-block; - top: 15px; + @include gl--flex-center; + top: 14px; width: 22px; height: 22px; diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb index 30af9aec5dfa31..5fb36119fbfdf3 100644 --- a/app/helpers/events_helper.rb +++ b/app/helpers/events_helper.rb @@ -274,7 +274,7 @@ def event_wiki_page_target_url(event) def event_note_title_html(event) if event.note_target capture do - concat content_tag(:span, event.note_target_type_name, class: "event-target-type gl-mr-2") + concat content_tag(:span, event.note_target_type_name, class: "event-target-type gl-mr-2 #{user_profile_activity_classes}") concat link_to(event.note_target_reference, event_note_target_url(event), title: event.target_title, class: 'has-tooltip event-target-link gl-mr-2') end else @@ -306,15 +306,12 @@ def design_event_icon(action, size: 24) end def icon_for_profile_event(event) - if current_path?('users#activity') - content_tag :div, class: "system-note-image #{event.action_name.parameterize}-icon gl-p-2 gl-rounded-full gl-bg-gray-50 gl-line-height-0" do - icon_for_event(event.action_name, size: 14) - end - else - content_tag :div, class: 'system-note-image user-avatar' do - author_avatar(event, size: 32) - end - end + base_class = 'system-note-image' + + classes = current_path?('users#activity') ? "#{event.action_name.parameterize}-icon gl-rounded-full gl-bg-gray-50 gl-line-height-0" : "user-avatar" + content = current_path?('users#activity') ? icon_for_event(event.action_name, size: 14) : author_avatar(event, size: 32) + + tag.div(class: "#{base_class} #{classes}") { content } end def inline_event_icon(event) @@ -328,19 +325,17 @@ def inline_event_icon(event) end def event_user_info(event) - unless current_path?('users#activity') - content_tag(:div, class: "event-user-info") do - concat content_tag(:span, link_to_author(event), class: "author-name") - concat " ".html_safe - concat content_tag(:span, event.author.to_reference, class: "username") - end + return if current_path?('users#activity') + + tag.div(class: 'event-user-info') do + concat tag.span(link_to_author(event), class: 'author-name') + concat ' '.html_safe + concat tag.span(event.author.to_reference, class: 'username') end end def user_profile_activity_classes - return '' unless current_path?('users#activity') - - ' gl-font-weight-bold gl-text-black-normal' + current_path?('users#activity') ? ' gl-font-weight-semibold gl-text-black-normal' : '' end private diff --git a/app/views/events/_event.html.haml b/app/views/events/_event.html.haml index 698192802b2e6d..c28fe7c8330152 100644 --- a/app/views/events/_event.html.haml +++ b/app/views/events/_event.html.haml @@ -1,7 +1,7 @@ - event = event.present - if event.visible_to_user?(current_user) - .event-item{ class: current_path?('users#activity') ? 'user-profile-activity gl-border-bottom-0 gl-pl-7! gl-pb-6' : '' } + .event-item{ class: current_path?('users#activity') ? 'user-profile-activity gl-border-bottom-0 gl-pl-7! gl-pb-3' : '' } .event-item-timestamp.gl-font-sm #{time_ago_with_tooltip(event.created_at)} diff --git a/app/views/users/_overview.html.haml b/app/views/users/_overview.html.haml index a4f0cec2c63f1b..597e7c37388590 100644 --- a/app/views/users/_overview.html.haml +++ b/app/views/users/_overview.html.haml @@ -1,4 +1,4 @@ -- activity_pane_class = Feature.enabled?(:security_auto_fix) && @user.bot? ? "col-12" : "col-md-12 col-lg-6" +- activity_pane_class = Feature.enabled?(:security_auto_fix) && @user.bot? ? "col-12" : "col-md-12 col-lg-6 gl-align-self-start" .row.d-none.d-sm-flex .col-12.calendar-block.gl-my-3 diff --git a/spec/features/users/overview_spec.rb b/spec/features/users/overview_spec.rb index d1ff60b6069dd0..528c50b8e54887 100644 --- a/spec/features/users/overview_spec.rb +++ b/spec/features/users/overview_spec.rb @@ -61,15 +61,15 @@ def push_code_contribution end end - describe 'user has 11 activities' do + describe 'user has 15 activities' do before do - 11.times { push_code_contribution } + 16.times { push_code_contribution } end include_context 'visit overview tab' - it 'displays 10 entries in the list of activities' do - expect(find('#js-overview')).to have_selector('.event-item', count: 10) + it 'displays 15 entries in the list of activities' do + expect(find('#js-overview')).to have_selector('.event-item', count: 15) end it 'shows a link to the activity list' do diff --git a/spec/helpers/events_helper_spec.rb b/spec/helpers/events_helper_spec.rb index 26e771312e8068..2b677b0978184c 100644 --- a/spec/helpers/events_helper_spec.rb +++ b/spec/helpers/events_helper_spec.rb @@ -39,6 +39,69 @@ end end + describe '#icon_for_profile_event' do + let(:event) { build(:event, :joined) } + let(:users_activity_page?) { true } + + before do + allow(helper).to receive(:current_path?).and_call_original + allow(helper).to receive(:current_path?).with('users#activity').and_return(users_activity_page?) + end + + context 'when on users activity page' do + it 'gives an icon with specialized classes' do + result = helper.icon_for_profile_event(event) + + expect(result).to include('joined-icon') + expect(result).to include('wiki page", + "wiki page", "", title, "" @@ -456,20 +520,21 @@ def can_read_design_activity(object, ability) end describe '#user_profile_activity_classes' do - context 'when on the user activity page' do - before do - allow(helper).to receive(:current_path?).with('users#activity').and_return(true) - end + let(:users_activity_page?) { true } + before do + allow(helper).to receive(:current_path?).and_call_original + allow(helper).to receive(:current_path?).with('users#activity').and_return(users_activity_page?) + end + + context 'when on the user activity page' do it 'returns the expected class names' do - expect(helper.user_profile_activity_classes).to eq(' gl-font-weight-bold gl-text-black-normal') + expect(helper.user_profile_activity_classes).to eq(' gl-font-weight-semibold gl-text-black-normal') end end context 'when not on the user activity page' do - before do - allow(helper).to receive(:current_path?).with('users#activity').and_return(false) - end + let(:users_activity_page?) { false } it 'returns an empty string' do expect(helper.user_profile_activity_classes).to eq('') -- GitLab