Improve discussions
* check for outdated discussions by comparing diff * improve discussion UI Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
This commit is contained in:
parent
69baa3afb2
commit
e93f6b030a
11 changed files with 119 additions and 52 deletions
|
@ -43,38 +43,14 @@ ul.notes {
|
|||
}
|
||||
|
||||
.discussion {
|
||||
padding: 8px 0;
|
||||
padding: 10px 0;
|
||||
overflow: hidden;
|
||||
display: block;
|
||||
position:relative;
|
||||
border-bottom: 1px solid #EEE;
|
||||
|
||||
.discussion-body {
|
||||
margin-left: 50px;
|
||||
|
||||
.diff-file,
|
||||
.discussion-hidden,
|
||||
.notes {
|
||||
background-color: #F9F9F9;
|
||||
}
|
||||
.diff-file .notes {
|
||||
/* reset */
|
||||
background: inherit;
|
||||
border: none;
|
||||
@include box-shadow(none);
|
||||
|
||||
}
|
||||
.discussion-hidden .note {
|
||||
@extend .cgray;
|
||||
padding: 8px;
|
||||
text-align: center;
|
||||
}
|
||||
.notes .note {
|
||||
border-color: #ddd;
|
||||
padding: 8px;
|
||||
}
|
||||
.reply-btn {
|
||||
margin-top: 8px;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -137,10 +113,6 @@ ul.notes {
|
|||
vertical-align: top;
|
||||
}
|
||||
}
|
||||
|
||||
.reply-btn {
|
||||
margin: 5px;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -376,3 +348,17 @@ ul.notes {
|
|||
margin-top: 5px;
|
||||
margin-bottom: 5px;
|
||||
}
|
||||
|
||||
.discussion-body,
|
||||
.diff-file {
|
||||
.notes .note {
|
||||
border-color: #ddd;
|
||||
padding: 10px 15px;
|
||||
}
|
||||
|
||||
.discussion-reply-holder {
|
||||
background: #f9f9f9;
|
||||
padding: 10px 15px;
|
||||
border-top: 1px solid #DDD;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -15,12 +15,6 @@ module NotesHelper
|
|||
end
|
||||
end
|
||||
|
||||
def link_to_merge_request_diff_line_note(note)
|
||||
if note.for_merge_request_diff_line? and note.diff
|
||||
link_to "#{note.diff_file_name}:L#{note.diff_new_line}", diffs_project_merge_request_path(note.project, note.noteable, anchor: note.line_code)
|
||||
end
|
||||
end
|
||||
|
||||
def note_timestamp(note)
|
||||
# Shows the created at time and the updated at time if different
|
||||
ts = "#{time_ago_with_tooltip(note.created_at, 'bottom', 'note_created_ago')}"
|
||||
|
|
|
@ -179,10 +179,26 @@ class Note < ActiveRecord::Base
|
|||
@diff ||= Gitlab::Git::Diff.new(st_diff) if st_diff.respond_to?(:map)
|
||||
end
|
||||
|
||||
# Check if such line of code exists in merge request diff
|
||||
# If exists - its active discussion
|
||||
# If not - its outdated diff
|
||||
def active?
|
||||
# TODO: determine if discussion is outdated
|
||||
# according to recent MR diff or not
|
||||
true
|
||||
noteable.diffs.each do |mr_diff|
|
||||
next unless mr_diff.new_path == self.diff.new_path
|
||||
|
||||
Gitlab::DiffParser.new(mr_diff.diff.lines.to_a, mr_diff.new_path).
|
||||
each do |full_line, type, line_code, line_new, line_old|
|
||||
if full_line == diff_line
|
||||
return true
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
false
|
||||
end
|
||||
|
||||
def outdated?
|
||||
!active?
|
||||
end
|
||||
|
||||
def diff_file_index
|
||||
|
|
0
app/views/projects/notes/_commit_discussion.html.haml
Normal file
0
app/views/projects/notes/_commit_discussion.html.haml
Normal file
|
@ -1,7 +1,7 @@
|
|||
- note = notes.first # example note
|
||||
-# Check if line want not changed since comment was left
|
||||
- if !defined?(line) || line == note.diff_line
|
||||
%tr.notes_holder.js-toggle-content
|
||||
%tr.notes_holder
|
||||
%td.notes_line{ colspan: 2 }
|
||||
%span.btn.disabled
|
||||
%i.icon-comment
|
||||
|
@ -9,4 +9,5 @@
|
|||
%td.notes_content
|
||||
%ul.notes{ rel: note.discussion_id }
|
||||
= render notes
|
||||
= link_to_reply_diff(note)
|
||||
.discussion-reply-holder
|
||||
= link_to_reply_diff(note)
|
||||
|
|
|
@ -2,7 +2,7 @@
|
|||
- note2 = notes2.first # example note
|
||||
-# Check if line want not changed since comment was left
|
||||
/- if !defined?(line) || line == note.diff_line
|
||||
%tr.notes_holder.js-toggle-content
|
||||
%tr.notes_holder
|
||||
- if note1
|
||||
%td.notes_line
|
||||
%span.btn.disabled
|
||||
|
|
|
@ -1,5 +1,13 @@
|
|||
- note = discussion_notes.first
|
||||
.discussion.js-toggle-container{ class: note.discussion_id }
|
||||
- if note.for_merge_request?
|
||||
- if note.outdated?
|
||||
= render "projects/notes/discussions/outdated", discussion_notes: discussion_notes
|
||||
- else
|
||||
= render "projects/notes/discussions/active", discussion_notes: discussion_notes
|
||||
- else
|
||||
= render "projects/notes/discussions/commit", discussion_notes: discussion_notes
|
||||
|
||||
-#.discussion.js-toggle-container{ class: note.discussion_id }
|
||||
.discussion-header
|
||||
.discussion-actions
|
||||
= link_to "#", class: "js-toggle-button" do
|
||||
|
@ -10,7 +18,7 @@
|
|||
= link_to_member(@project, note.author, avatar: false)
|
||||
- if note.for_merge_request?
|
||||
- if note.diff
|
||||
started a discussion on this merge request diff
|
||||
started a discussion on the diff
|
||||
= link_to_merge_request_diff_line_note(note)
|
||||
- else
|
||||
started
|
||||
|
@ -32,14 +40,7 @@
|
|||
#{time_ago_with_tooltip(last_note.updated_at, 'bottom', 'discussion_updated_ago')}
|
||||
.discussion-body.js-toggle-content
|
||||
- if note.for_diff_line?
|
||||
- if note.active?
|
||||
= render "projects/notes/discussion_diff", discussion_notes: discussion_notes, note: note
|
||||
- else
|
||||
= link_to 'show outdated discussion', '#', class: 'js-show-outdated-discussion'
|
||||
%div.hide.outdated-discussion
|
||||
.notes{ rel: discussion_notes.first.discussion_id }
|
||||
= render discussion_notes
|
||||
|
||||
= render "projects/notes/discussion_diff", discussion_notes: discussion_notes, note: note
|
||||
- else
|
||||
.notes{ rel: discussion_notes.first.discussion_id }
|
||||
= render discussion_notes
|
||||
|
|
0
app/views/projects/notes/_outdated_discussion.html.haml
Normal file
0
app/views/projects/notes/_outdated_discussion.html.haml
Normal file
21
app/views/projects/notes/discussions/_active.html.haml
Normal file
21
app/views/projects/notes/discussions/_active.html.haml
Normal file
|
@ -0,0 +1,21 @@
|
|||
- note = discussion_notes.first
|
||||
.discussion.js-toggle-container{ class: note.discussion_id }
|
||||
.discussion-header
|
||||
.discussion-actions
|
||||
= link_to "#", class: "js-toggle-button" do
|
||||
%i.icon-chevron-up
|
||||
Show/hide discussion
|
||||
= image_tag avatar_icon(note.author_email), class: "avatar s32"
|
||||
%div
|
||||
= link_to_member(@project, note.author, avatar: false)
|
||||
started a discussion
|
||||
= link_to diffs_project_merge_request_path(note.project, note.noteable, anchor: note.line_code) do
|
||||
%strong on the diff
|
||||
.last-update.hide.js-toggle-content
|
||||
- last_note = discussion_notes.last
|
||||
last updated by
|
||||
= link_to_member(@project, last_note.author, avatar: false)
|
||||
%span.discussion-last-update
|
||||
#{time_ago_with_tooltip(last_note.updated_at, 'bottom', 'discussion_updated_ago')}
|
||||
.discussion-body.js-toggle-content
|
||||
= render "projects/notes/discussion_diff", discussion_notes: discussion_notes, note: note
|
28
app/views/projects/notes/discussions/_commit.html.haml
Normal file
28
app/views/projects/notes/discussions/_commit.html.haml
Normal file
|
@ -0,0 +1,28 @@
|
|||
- note = discussion_notes.first
|
||||
.discussion.js-toggle-container{ class: note.discussion_id }
|
||||
.discussion-header
|
||||
.discussion-actions
|
||||
= link_to "#", class: "js-toggle-button" do
|
||||
%i.icon-chevron-up
|
||||
Show/hide discussion
|
||||
= image_tag avatar_icon(note.author_email), class: "avatar s32"
|
||||
%div
|
||||
= link_to_member(@project, note.author, avatar: false)
|
||||
started a discussion on commit
|
||||
= link_to(note.noteable.short_id, project_commit_path(note.project, note.noteable), class: 'monospace')
|
||||
.last-update.hide.js-toggle-content
|
||||
- last_note = discussion_notes.last
|
||||
last updated by
|
||||
= link_to_member(@project, last_note.author, avatar: false)
|
||||
%span.discussion-last-update
|
||||
#{time_ago_with_tooltip(last_note.updated_at, 'bottom', 'discussion_updated_ago')}
|
||||
.discussion-body.js-toggle-content
|
||||
- if note.for_diff_line?
|
||||
= render "projects/notes/discussion_diff", discussion_notes: discussion_notes, note: note
|
||||
- else
|
||||
.panel.panel-default
|
||||
.notes{ rel: discussion_notes.first.discussion_id }
|
||||
= render discussion_notes
|
||||
.discussion-reply-holder
|
||||
= link_to_reply_diff(discussion_notes.first)
|
||||
|
20
app/views/projects/notes/discussions/_outdated.html.haml
Normal file
20
app/views/projects/notes/discussions/_outdated.html.haml
Normal file
|
@ -0,0 +1,20 @@
|
|||
- note = discussion_notes.first
|
||||
.discussion.js-toggle-container{ class: note.discussion_id }
|
||||
.discussion-header
|
||||
.discussion-actions
|
||||
= link_to "#", class: "js-toggle-button" do
|
||||
%i.icon-chevron-down
|
||||
Show/hide discussion
|
||||
= image_tag avatar_icon(note.author_email), class: "avatar s32"
|
||||
%div
|
||||
= link_to_member(@project, note.author, avatar: false)
|
||||
started a discussion on the
|
||||
%strong outdated diff
|
||||
%div
|
||||
- last_note = discussion_notes.last
|
||||
last updated by
|
||||
= link_to_member(@project, last_note.author, avatar: false)
|
||||
%span.discussion-last-update
|
||||
#{time_ago_with_tooltip(last_note.updated_at, 'bottom', 'discussion_updated_ago')}
|
||||
.discussion-body.js-toggle-content.hide
|
||||
= render "projects/notes/discussion_diff", discussion_notes: discussion_notes, note: note
|
Loading…
Reference in a new issue