diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index 2a38ac28172..d83c41fae9d 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -125,6 +125,11 @@ // Close any open tooltips $('.has-tooltip, [data-toggle="tooltip"]').tooltip('destroy'); }; + + gl.utils.isMetaKey = function(e) { + return e.metaKey || e.ctrlKey || e.altKey || e.shiftKey; + }; + })(window); }).call(this); diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 44079bc3ca3..6cb87f9ba81 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -12,7 +12,7 @@ var bind = function(fn, me){ return function(){ return fn.apply(me, arguments); }; }; this.Notes = (function() { - var isMetaKey; + const MAX_VISIBLE_COMMIT_LIST_COUNT = 3; Notes.interval = null; @@ -33,6 +33,7 @@ this.resetMainTargetForm = bind(this.resetMainTargetForm, this); this.refresh = bind(this.refresh, this); this.keydownNoteText = bind(this.keydownNoteText, this); + this.toggleCommitList = bind(this.toggleCommitList, this); this.notes_url = notes_url; this.note_ids = note_ids; this.last_fetched_at = last_fetched_at; @@ -46,6 +47,7 @@ this.setPollingInterval(); this.setupMainTargetNoteForm(); this.initTaskList(); + this.collapseLongCommitList(); } Notes.prototype.addBinding = function() { @@ -81,10 +83,13 @@ $(document).on("click", ".js-add-diff-note-button", this.addDiffNote); // hide diff note form $(document).on("click", ".js-close-discussion-note-form", this.cancelDiscussionForm); + // toggle commit list + $(document).on("click", '.system-note-commit-list-toggler', this.toggleCommitList); // fetch notes when tab becomes visible $(document).on("visibilitychange", this.visibilityChange); // when issue status changes, we need to refresh data $(document).on("issuable:change", this.refresh); + // when a key is clicked on the notes return $(document).on("keydown", ".js-note-text", this.keydownNoteText); }; @@ -114,9 +119,10 @@ Notes.prototype.keydownNoteText = function(e) { var $textarea, discussionNoteForm, editNote, myLastNote, myLastNoteEditBtn, newText, originalText; - if (isMetaKey(e)) { + if (gl.utils.isMetaKey(e)) { return; } + $textarea = $(e.target); // Edit previous note when UP arrow is hit switch (e.which) { @@ -156,10 +162,6 @@ } }; - isMetaKey = function(e) { - return e.metaKey || e.ctrlKey || e.altKey || e.shiftKey; - }; - Notes.prototype.initRefresh = function() { clearInterval(Notes.interval); return Notes.interval = setInterval((function(_this) { @@ -263,6 +265,7 @@ $notesList.append(note.html).syntaxHighlight(); // Update datetime format on the recent note gl.utils.localTimeAgo($notesList.find("#note_" + note.id + " .js-timeago"), false); + this.collapseLongCommitList(); this.initTaskList(); this.refresh(); return this.updateNotesCount(1); @@ -433,9 +436,9 @@ var $form = $(xhr.target); if ($form.attr('data-resolve-all') != null) { - var projectPath = $form.data('project-path') - discussionId = $form.data('discussion-id'), - mergeRequestId = $form.data('noteable-iid'); + var projectPath = $form.data('project-path'); + var discussionId = $form.data('discussion-id'); + var mergeRequestId = $form.data('noteable-iid'); if (ResolveService != null) { ResolveService.toggleResolveForDiscussion(projectPath, mergeRequestId, discussionId); @@ -844,9 +847,9 @@ return this.notesCountBadge.text(parseInt(this.notesCountBadge.text()) + updateCount); }; - Notes.prototype.resolveDiscussion = function () { - var $this = $(this), - discussionId = $this.attr('data-discussion-id'); + Notes.prototype.resolveDiscussion = function() { + var $this = $(this); + var discussionId = $this.attr('data-discussion-id'); $this .closest('form') @@ -855,6 +858,36 @@ .attr('data-project-path', $this.attr('data-project-path')); }; + Notes.prototype.toggleCommitList = function(e) { + const $element = $(e.target); + const $closestSystemCommitList = $element.siblings('.system-note-commit-list'); + + $closestSystemCommitList.toggleClass('hide-shade'); + }; + + /** + Scans system notes with `ul` elements in system note body + then collapse long commit list pushed by user to make it less + intrusive. + */ + Notes.prototype.collapseLongCommitList = function() { + const systemNotes = $('#notes-list').find('li.system-note').has('ul'); + + $.each(systemNotes, function(index, systemNote) { + const $systemNote = $(systemNote); + const headerMessage = $systemNote.find('.note-text').find('p:first').text().replace(':', ''); + + $systemNote.find('.note-header .system-note-message').html(headerMessage); + + if ($systemNote.find('li').length > MAX_VISIBLE_COMMIT_LIST_COUNT) { + $systemNote.find('.note-text').addClass('system-note-commit-list'); + $systemNote.find('.system-note-commit-list-toggler').show(); + } else { + $systemNote.find('.note-text').addClass('system-note-commit-list hide-shade'); + } + }); + }; + return Notes; })(); diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 9bfa1c96a5d..cc16d83c210 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -35,11 +35,83 @@ ul.notes { .system-note { font-size: 14px; - padding-top: 10px; - padding-bottom: 10px; - background: #fdfdfd; + padding: 0; + clear: both; + + &.timeline-entry::after { + clear: none; + } + + .system-note-message { + text-transform: lowercase; + + a { + color: $gl-link-color; + text-decoration: none; + } + } + + .timeline-content { + padding: 14px 10px; + } + + .note-body { + overflow: hidden; + + .system-note-commit-list-toggler { + display: none; + padding: 10px 0 0; + cursor: pointer; + } + + .note-text { + & p:first-child { + display: none; + } + + &.system-note-commit-list { + max-height: 63px; + overflow: hidden; + display: block; + + ul { + margin: 3px 0 3px 15px !important; + + li { + font-family: $monospace_font; + font-size: 12px; + } + } + + p:first-child { + display: none; + } + + &::after { + content: ''; + width: 100%; + height: 20px; + position: absolute; + left: 0; + bottom: 50px; + background: linear-gradient(rgba($gray-light, .3) 0, $white-light); + } + + &.hide-shade { + max-height: 100%; + overflow: auto; + + &::after { + background: transparent; + } + } + } + } + } .timeline-icon { + display: none; + .avatar { visibility: hidden; @@ -65,6 +137,12 @@ ul.notes { position: relative; border-bottom: 1px solid $table-border-gray; + &.note-discussion { + &.timeline-entry { + padding: 14px 10px; + } + } + &.is-editting { .note-header, .note-text, @@ -88,10 +166,8 @@ ul.notes { overflow: auto; word-wrap: break-word; @include md-typography; - // Reset ul style types since we're nested inside a ul already @include bulleted-list; - ul.task-list { ul:not(.task-list) { padding-left: 1.3em; @@ -111,6 +187,11 @@ ul.notes { padding-bottom: 3px; padding-right: 20px; + p { + display: inline; + margin: 0; + } + @media (min-width: $screen-sm-min) { padding-right: 0; } @@ -238,6 +319,10 @@ ul.notes { } } +.discussion-header { + font-size: 14px; +} + .note-headline-light, .discussion-headline-light { color: $notes-light-color; diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml index 077e8e64e5f..e4b4ea675d2 100644 --- a/app/views/discussions/_discussion.html.haml +++ b/app/views/discussions/_discussion.html.haml @@ -1,9 +1,6 @@ - expanded = discussion.expanded? %li.note.note-discussion.timeline-entry .timeline-entry-inner - .timeline-icon - = link_to user_path(discussion.author) do - = image_tag avatar_icon(discussion.author), class: "avatar s40" .timeline-content .discussion.js-toggle-container{ class: discussion.id, data: { discussion_id: discussion.id } } .discussion-header @@ -13,9 +10,7 @@ = icon("chevron-up") - else = icon("chevron-down") - Toggle discussion - = link_to_member(@project, discussion.author, avatar: false) .inline.discussion-headline-light @@ -38,8 +33,6 @@ = time_ago_with_tooltip(discussion.created_at, placement: "bottom", html_class: "note-created-ago") - = render "discussions/headline", discussion: discussion - .discussion-body.js-toggle-content{ class: ("hide" unless expanded) } - if discussion.diff_discussion? && discussion.diff_file = render "discussions/diff_with_notes", discussion: discussion diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index afff15228c1..89ae64554c0 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -14,6 +14,9 @@ = note.author.to_reference - unless note.system commented + - if note.system + %span{class: 'system-note-message'} + = h(note.note_html.downcase.html_safe) %a{ href: "##{dom_id(note)}" } = time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago') - unless note.system? @@ -67,7 +70,9 @@ = render 'projects/notes/edit_form', note: note .note-awards = render 'award_emoji/awards_block', awardable: note, inline: false - + - if note.system + .system-note-commit-list-toggler + Toggle commit list - if note.attachment.url .note-attachment - if note.attachment.image? diff --git a/changelogs/unreleased/less-intrusive-system-note.yml b/changelogs/unreleased/less-intrusive-system-note.yml new file mode 100644 index 00000000000..87a5a3be246 --- /dev/null +++ b/changelogs/unreleased/less-intrusive-system-note.yml @@ -0,0 +1,4 @@ +--- +title: Make system notes less intrusive +merge_request: 6755 +author: diff --git a/spec/features/merge_requests/diff_notes_resolve_spec.rb b/spec/features/merge_requests/diff_notes_resolve_spec.rb index d5e3d8e7eff..eab64bd4b54 100644 --- a/spec/features/merge_requests/diff_notes_resolve_spec.rb +++ b/spec/features/merge_requests/diff_notes_resolve_spec.rb @@ -201,7 +201,7 @@ feature 'Diff notes resolve', feature: true, js: true do expect(first('.line-resolve-btn')['data-original-title']).to eq("Resolved by #{user.name}") end - expect(page).to have_content('Last updated') + expect(page).not_to have_content('Last updated') page.within '.line-resolve-all-container' do expect(page).to have_content('0/1 discussion resolved') diff --git a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb index c3c3ab33872..8eceaad2457 100644 --- a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb +++ b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb @@ -73,7 +73,7 @@ feature 'Merge When Build Succeeds', feature: true, js: true do expect(page).to have_button "Merge When Build Succeeds" visit_merge_request(merge_request) # refresh the page - expect(page).to have_content "Canceled the automatic merge" + expect(page).to have_content "canceled the automatic merge" end it "allows the user to remove the source branch" do @@ -101,7 +101,7 @@ feature 'Merge When Build Succeeds', feature: true, js: true do expect(page).not_to have_link "Merge When Build Succeeds" end end - + def visit_merge_request(merge_request) visit namespace_project_merge_request_path(merge_request.project.namespace, merge_request.project, merge_request) end