From e490a54105b6c820e330514aeb91d004997b031c Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 14 Apr 2016 14:50:48 +0100 Subject: [PATCH 1/3] Discussion notes update At the moment discussion notes are rendered in a list but arent actually a list item and that can cause overflow issues Moved the dates to be in the header like other notes Fixed overlapping issue on mobile --- .../stylesheets/framework/timeline.scss | 3 +-- app/assets/stylesheets/pages/notes.scss | 6 +++++ .../projects/notes/_discussion.html.haml | 23 +++++++++---------- app/views/projects/notes/_notes.html.haml | 3 ++- .../notes/discussions/_active.html.haml | 6 +---- .../notes/discussions/_commit.html.haml | 9 +++----- .../notes/discussions/_outdated.html.haml | 6 +---- 7 files changed, 25 insertions(+), 31 deletions(-) diff --git a/app/assets/stylesheets/framework/timeline.scss b/app/assets/stylesheets/framework/timeline.scss index b91f2f6f898..f0ec250de2b 100644 --- a/app/assets/stylesheets/framework/timeline.scss +++ b/app/assets/stylesheets/framework/timeline.scss @@ -39,8 +39,7 @@ .diff-file { border: 1px solid $border-color; border-bottom: none; - margin-left: 0; - margin-right: 0; + margin: 0; } } diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index e421a31549a..7489d5de5f0 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -198,6 +198,12 @@ ul.notes { color: $notes-light-color; } +.discussion-headline-light { + a { + color: $gl-link-color; + } +} + /** * Actions for Discussions/Notes */ diff --git a/app/views/projects/notes/_discussion.html.haml b/app/views/projects/notes/_discussion.html.haml index b8068835b3a..b66fead09b7 100644 --- a/app/views/projects/notes/_discussion.html.haml +++ b/app/views/projects/notes/_discussion.html.haml @@ -1,13 +1,12 @@ - note = discussion_notes.first -.timeline-entry - .timeline-entry-inner - .timeline-icon - = link_to user_path(note.author) do - = image_tag avatar_icon(note.author_email), class: "avatar s40" - .timeline-content - - if note.for_merge_request? - - (active_notes, outdated_notes) = discussion_notes.partition(&:active?) - = render "projects/notes/discussions/active", discussion_notes: active_notes if active_notes.length > 0 - = render "projects/notes/discussions/outdated", discussion_notes: outdated_notes if outdated_notes.length > 0 - - else - = render "projects/notes/discussions/commit", discussion_notes: discussion_notes +.timeline-entry-inner + .timeline-icon + = link_to user_path(note.author) do + = image_tag avatar_icon(note.author_email), class: "avatar s40" + .timeline-content + - if note.for_merge_request? + - (active_notes, outdated_notes) = discussion_notes.partition(&:active?) + = render "projects/notes/discussions/active", discussion_notes: active_notes if active_notes.length > 0 + = render "projects/notes/discussions/outdated", discussion_notes: outdated_notes if outdated_notes.length > 0 + - else + = render "projects/notes/discussions/commit", discussion_notes: discussion_notes diff --git a/app/views/projects/notes/_notes.html.haml b/app/views/projects/notes/_notes.html.haml index 62db86fb181..4eeaf70e987 100644 --- a/app/views/projects/notes/_notes.html.haml +++ b/app/views/projects/notes/_notes.html.haml @@ -6,7 +6,8 @@ = render discussion_notes - else - = render 'projects/notes/discussion', discussion_notes: discussion_notes + %li.note.note-discussion.timeline-entry + = render 'projects/notes/discussion', discussion_notes: discussion_notes - else - @notes.each do |note| - next unless note.author diff --git a/app/views/projects/notes/discussions/_active.html.haml b/app/views/projects/notes/discussions/_active.html.haml index cd8a5f0bd02..0ea8862a684 100644 --- a/app/views/projects/notes/discussions/_active.html.haml +++ b/app/views/projects/notes/discussions/_active.html.haml @@ -6,15 +6,11 @@ = "#{note.author.to_reference} started a discussion" = link_to diffs_namespace_project_merge_request_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code) do on the diff + = time_ago_with_tooltip(note.created_at, placement: "bottom", html_class: "discussion_updated_ago") .discussion-actions = link_to "#", class: "discussion-action-button discussion-toggle-button js-toggle-button" do %i.fa.fa-chevron-up Show/hide discussion - .last-update.hide.js-toggle-content - - last_note = discussion_notes.last - last updated by - = link_to_member(@project, last_note.author, avatar: false) - #{time_ago_with_tooltip(last_note.updated_at, placement: 'bottom', html_class: 'discussion_updated_ago')} .discussion-body.js-toggle-content = render "projects/notes/discussions/diff", discussion_notes: discussion_notes, note: note diff --git a/app/views/projects/notes/discussions/_commit.html.haml b/app/views/projects/notes/discussions/_commit.html.haml index 46f2ba4bbcf..2a2ead58eeb 100644 --- a/app/views/projects/notes/discussions/_commit.html.haml +++ b/app/views/projects/notes/discussions/_commit.html.haml @@ -8,21 +8,18 @@ = "#{note.author.to_reference} started a discussion on #{commit_description}" - if commit = link_to(commit.short_id, namespace_project_commit_path(note.project.namespace, note.project, note.noteable), class: 'monospace') + = time_ago_with_tooltip(note.created_at, placement: "bottom", html_class: "discussion_updated_ago") .discussion-actions = link_to "#", class: "note-action-button discussion-toggle-button js-toggle-button" do %i.fa.fa-chevron-up Show/hide discussion - .last-update.hide.js-toggle-content - - last_note = discussion_notes.last - last updated by - = link_to_member(@project, last_note.author, avatar: false) - #{time_ago_with_tooltip(last_note.updated_at, placement: 'bottom', html_class: 'discussion_updated_ago')} .discussion-body.js-toggle-content - if note.for_diff_line? = render "projects/notes/discussions/diff", discussion_notes: discussion_notes, note: note - else .panel.panel-default .notes{ data: { discussion_id: discussion_notes.first.discussion_id } } - = render discussion_notes + %ul.notes.timeline + = render discussion_notes .discussion-reply-holder = link_to_reply_diff(discussion_notes.first) diff --git a/app/views/projects/notes/discussions/_outdated.html.haml b/app/views/projects/notes/discussions/_outdated.html.haml index f8e000b424f..45141bcd1df 100644 --- a/app/views/projects/notes/discussions/_outdated.html.haml +++ b/app/views/projects/notes/discussions/_outdated.html.haml @@ -5,14 +5,10 @@ .inline.discussion-headline-light = "#{note.author.to_reference} started a discussion" on the outdated diff + = time_ago_with_tooltip(note.created_at, placement: "bottom", html_class: "discussion_updated_ago") .discussion-actions = link_to "#", class: "note-action-button discussion-toggle-button js-toggle-button" do %i.fa.fa-chevron-down Show/hide discussion - .last-update.hide.js-toggle-content - - last_note = discussion_notes.last - last updated by - = link_to_member(@project, last_note.author, avatar: false) - #{time_ago_with_tooltip(last_note.updated_at, placement: 'bottom', html_class: 'discussion_updated_ago')} .discussion-body.js-toggle-content.hide = render "projects/notes/discussions/diff", discussion_notes: discussion_notes, note: note From 44f3f42bac143f87d756ed38fa7142f98c7be90d Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 14 Apr 2016 14:56:05 +0100 Subject: [PATCH 2/3] Dicussion action mobile fix --- app/assets/stylesheets/pages/notes.scss | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 7489d5de5f0..e4f96c11aae 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -215,6 +215,17 @@ ul.notes { color: $notes-action-color; } +.discussion-actions { + @media (max-width: $screen-sm-max) { + float: none; + margin-left: 0; + + .note-action-button { + margin-left: 0; + } + } +} + .note-action-button, .discussion-action-button { display: inline-block; From 038e623042af614fb739f49a9302ec42cab9812a Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 14 Apr 2016 17:26:32 +0100 Subject: [PATCH 3/3] Fixed tests --- .../projects/notes/_discussion.html.haml | 23 ++++++++++--------- app/views/projects/notes/_notes.html.haml | 3 +-- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/views/projects/notes/_discussion.html.haml b/app/views/projects/notes/_discussion.html.haml index b66fead09b7..572b00a38c7 100644 --- a/app/views/projects/notes/_discussion.html.haml +++ b/app/views/projects/notes/_discussion.html.haml @@ -1,12 +1,13 @@ - note = discussion_notes.first -.timeline-entry-inner - .timeline-icon - = link_to user_path(note.author) do - = image_tag avatar_icon(note.author_email), class: "avatar s40" - .timeline-content - - if note.for_merge_request? - - (active_notes, outdated_notes) = discussion_notes.partition(&:active?) - = render "projects/notes/discussions/active", discussion_notes: active_notes if active_notes.length > 0 - = render "projects/notes/discussions/outdated", discussion_notes: outdated_notes if outdated_notes.length > 0 - - else - = render "projects/notes/discussions/commit", discussion_notes: discussion_notes +%li.note.note-discussion.timeline-entry + .timeline-entry-inner + .timeline-icon + = link_to user_path(note.author) do + = image_tag avatar_icon(note.author_email), class: "avatar s40" + .timeline-content + - if note.for_merge_request? + - (active_notes, outdated_notes) = discussion_notes.partition(&:active?) + = render "projects/notes/discussions/active", discussion_notes: active_notes if active_notes.length > 0 + = render "projects/notes/discussions/outdated", discussion_notes: outdated_notes if outdated_notes.length > 0 + - else + = render "projects/notes/discussions/commit", discussion_notes: discussion_notes diff --git a/app/views/projects/notes/_notes.html.haml b/app/views/projects/notes/_notes.html.haml index 4eeaf70e987..62db86fb181 100644 --- a/app/views/projects/notes/_notes.html.haml +++ b/app/views/projects/notes/_notes.html.haml @@ -6,8 +6,7 @@ = render discussion_notes - else - %li.note.note-discussion.timeline-entry - = render 'projects/notes/discussion', discussion_notes: discussion_notes + = render 'projects/notes/discussion', discussion_notes: discussion_notes - else - @notes.each do |note| - next unless note.author