Download as
Browse Code »

Commit 2d554f42f07a9bebcb6d43ace2cdeedcc54e8de9

Authored by Jacob Carlborg
Committed by Jacob Carlborg
1 parent 138e2a50
Exists in spb-stable and in 3 other branches 6-8-stable, 7-0-stable, 7-0-stable-spb

Add button for toggling inline comments in diff view. 

This is useful when there are many comments and they're becoming a distraction when trying to read the diff.
Showing 7 changed files with 154 additions and 4 deletions   Show diff stats
app/assets/stylesheets/sections/commits.scss
  Diff comments   View file @2d554f4
@@ -60,6 +60,9 @@ @@ -60,6 +60,9 @@
60 } 60 }
61 61
62 table { 62 table {
  63 + &.text-file {
  64 + border-collapse: separate;
  65 + }
63 width: 100%; 66 width: 100%;
64 font-family: $monospace_font; 67 font-family: $monospace_font;
65 border: none; 68 border: none;
app/assets/stylesheets/sections/notes.scss
  Diff comments   View file @2d554f4
@@ -184,13 +184,40 @@ ul.notes { @@ -184,13 +184,40 @@ ul.notes {
184 } 184 }
185 } 185 }
186 } 186 }
  187 +
  188 +.file {
  189 + .discussion-actions {
  190 + margin-right: 5px;
  191 + margin-top: 3px;
  192 +
  193 + .turn-off {
  194 + display: inherit;
  195 + }
  196 + .turn-on {
  197 + display: none;
  198 + }
  199 + }
  200 +
  201 + &.open .discussion-actions {
  202 + .turn-off {
  203 + display: none;
  204 + }
  205 + .turn-on {
  206 + display: inherit;
  207 + }
  208 + }
  209 +
  210 + &[id^="diff"] .content {
  211 + display: block;
  212 + }
  213 +}
  214 +
187 .file .note .note-actions { 215 .file .note .note-actions {
188 right: 0; 216 right: 0;
189 top: 0; 217 top: 0;
190 } 218 }
191 219
192 220
193 -  
194 /** 221 /**
195 * Line note button on the side of diffs 222 * Line note button on the side of diffs
196 */ 223 */
app/views/projects/commits/_diffs.html.haml
  Diff comments   View file @2d554f4
@@ -44,7 +44,7 @@ @@ -44,7 +44,7 @@
44 - file = project.repository.blob_at(@commit.id, diff.new_path) 44 - file = project.repository.blob_at(@commit.id, diff.new_path)
45 - file = project.repository.blob_at(@commit.parent_id, diff.old_path) unless file 45 - file = project.repository.blob_at(@commit.parent_id, diff.old_path) unless file
46 - next unless file 46 - next unless file
47 - .file{id: "diff-#{i}"} 47 + .file.js-details-container.js-toggle-container.open{id: "diff-#{i}"}
48 .header 48 .header
49 - if diff.deleted_file 49 - if diff.deleted_file
50 %span= diff.old_path 50 %span= diff.old_path
@@ -62,6 +62,14 @@ @@ -62,6 +62,14 @@
62 View file @ 62 View file @
63 %span.commit-short-id= @commit.short_id(6) 63 %span.commit-short-id= @commit.short_id(6)
64 64
  65 + .discussion-actions.pull-right
  66 + = link_to "javascript:;", class: "js-details-target js-toggle-button turn-on" do
  67 + %i.icon-eye-close
  68 + Hide inline discussion
  69 + = link_to "javascript:;", class: "js-details-target js-toggle-button turn-off" do
  70 + %i.icon-eye-open
  71 + Show inline discussion
  72 +
65 .content 73 .content
66 -# Skipp all non non-supported blobs 74 -# Skipp all non non-supported blobs
67 - next unless file.respond_to?('text?') 75 - next unless file.respond_to?('text?')
app/views/projects/notes/_diff_notes_with_reply.html.haml
  Diff comments   View file @2d554f4
1 - note = notes.first # example note 1 - note = notes.first # example note
2 -# Check if line want not changed since comment was left 2 -# Check if line want not changed since comment was left
3 - if !defined?(line) || line == note.diff_line 3 - if !defined?(line) || line == note.diff_line
4 - %tr.notes_holder 4 + %tr.notes_holder.js-toggle-content
5 %td.notes_line{ colspan: 2 } 5 %td.notes_line{ colspan: 2 }
6 %span.btn.disabled 6 %span.btn.disabled
7 %i.icon-comment 7 %i.icon-comment
app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml
  Diff comments   View file @2d554f4
1 - note1 = notes1.first # example note 1 - note1 = notes1.first # example note
2 - note2 = notes2.first # example note 2 - note2 = notes2.first # example note
3 -%tr.notes_holder 3 +%tr.notes_holder.js-toggle-content
4 -# Check if line want not changed since comment was left 4 -# Check if line want not changed since comment was left
5 /- if !defined?(line1) || line1 == note1.diff_line 5 /- if !defined?(line1) || line1 == note1.diff_line
6 - if note1 6 - if note1
features/project/merge_requests.feature
  Diff comments   View file @2d554f4
@@ -88,3 +88,47 @@ Feature: Project Merge Requests @@ -88,3 +88,47 @@ Feature: Project Merge Requests
88 Given I visit merge request page "Bug NS-04" 88 Given I visit merge request page "Bug NS-04"
89 And I leave a comment with a header containing "Comment with a header" 89 And I leave a comment with a header containing "Comment with a header"
90 Then The comment with the header should not have an ID 90 Then The comment with the header should not have an ID
  91 +
  92 + # Toggling inline comments
  93 +
  94 + @javascript
  95 + Scenario: I hide comments on a merge request diff with comments in a single file
  96 + Given project "Shop" have "Bug NS-05" open merge request with diffs inside
  97 + And I visit merge request page "Bug NS-05"
  98 + And I switch to the diff tab
  99 + And I leave a comment like "Line is wrong" on line 39 of the second file
  100 + And I click link "Hide inline discussion" of the second file
  101 + Then I should not see a comment like "Line is wrong" in the second file
  102 +
  103 + @javascript
  104 + Scenario: I show comments on a merge request diff with comments in a single file
  105 + Given project "Shop" have "Bug NS-05" open merge request with diffs inside
  106 + And I visit merge request page "Bug NS-05"
  107 + And I switch to the diff tab
  108 + And I leave a comment like "Line is wrong" on line 39 of the second file
  109 + And I click link "Hide inline discussion" of the second file
  110 + And I click link "Show inline discussion" of the second file
  111 + Then I should see a comment like "Line is wrong" in the second file
  112 +
  113 + @javascript
  114 + Scenario: I hide comments on a merge request diff with comments in multiple files
  115 + Given project "Shop" have "Bug NS-05" open merge request with diffs inside
  116 + And I visit merge request page "Bug NS-05"
  117 + And I switch to the diff tab
  118 + And I leave a comment like "Line is correct" on line 12 of the first file
  119 + And I leave a comment like "Line is wrong" on line 39 of the second file
  120 + And I click link "Hide inline discussion" of the second file
  121 + Then I should not see a comment like "Line is wrong" in the second file
  122 + And I should still see a comment like "Line is correct" in the first file
  123 +
  124 + @javascript
  125 + Scenario: I show comments on a merge request diff with comments in multiple files
  126 + Given project "Shop" have "Bug NS-05" open merge request with diffs inside
  127 + And I visit merge request page "Bug NS-05"
  128 + And I switch to the diff tab
  129 + And I leave a comment like "Line is correct" on line 12 of the first file
  130 + And I leave a comment like "Line is wrong" on line 39 of the second file
  131 + And I click link "Hide inline discussion" of the second file
  132 + And I click link "Show inline discussion" of the second file
  133 + Then I should see a comment like "Line is wrong" in the second file
  134 + And I should still see a comment like "Line is correct" in the first file
features/steps/project/project_merge_requests.rb
  Diff comments   View file @2d554f4
@@ -170,6 +170,62 @@ class ProjectMergeRequests < Spinach::FeatureSteps @@ -170,6 +170,62 @@ class ProjectMergeRequests < Spinach::FeatureSteps
170 end 170 end
171 end 171 end
172 172
  173 + step 'I click link "Hide inline discussion" of the second file' do
  174 + within '.files [id^=diff]:nth-child(2)' do
  175 + click_link "Hide inline discussion"
  176 + end
  177 + end
  178 +
  179 + step 'I click link "Show inline discussion" of the second file' do
  180 + within '.files [id^=diff]:nth-child(2)' do
  181 + click_link "Show inline discussion"
  182 + end
  183 + end
  184 +
  185 + step 'I should not see a comment like "Line is wrong" in the second file' do
  186 + within '.files [id^=diff]:nth-child(2)' do
  187 + page.should_not have_visible_content "Line is wrong"
  188 + end
  189 + end
  190 +
  191 + step 'I should see a comment like "Line is wrong" in the second file' do
  192 + within '.files [id^=diff]:nth-child(2) .note-text' do
  193 + page.should have_visible_content "Line is wrong"
  194 + end
  195 + end
  196 +
  197 + step 'I leave a comment like "Line is correct" on line 12 of the first file' do
  198 + init_diff_note_first_file
  199 +
  200 + within(".js-discussion-note-form") do
  201 + fill_in "note_note", with: "Line is correct"
  202 + click_button "Add Comment"
  203 + end
  204 +
  205 + within ".files [id^=diff]:nth-child(1) .note-text" do
  206 + page.should have_content "Line is correct"
  207 + end
  208 + end
  209 +
  210 + step 'I leave a comment like "Line is wrong" on line 39 of the second file' do
  211 + init_diff_note_second_file
  212 +
  213 + within(".js-discussion-note-form") do
  214 + fill_in "note_note", with: "Line is wrong"
  215 + click_button "Add Comment"
  216 + end
  217 +
  218 + within ".files [id^=diff]:nth-child(2) .note-text" do
  219 + page.should have_content "Line is wrong"
  220 + end
  221 + end
  222 +
  223 + step 'I should still see a comment like "Line is correct" in the first file' do
  224 + within '.files [id^=diff]:nth-child(1) .note-text' do
  225 + page.should have_visible_content "Line is correct"
  226 + end
  227 + end
  228 +
173 def project 229 def project
174 @project ||= Project.find_by!(name: "Shop") 230 @project ||= Project.find_by!(name: "Shop")
175 end 231 end
@@ -192,4 +248,16 @@ class ProjectMergeRequests < Spinach::FeatureSteps @@ -192,4 +248,16 @@ class ProjectMergeRequests < Spinach::FeatureSteps
192 page.should have_content message 248 page.should have_content message
193 end 249 end
194 end 250 end
  251 +
  252 + def init_diff_note_first_file
  253 + find('a[data-line-code="a5cc2925ca8258af241be7e5b0381edf30266302_12_12"]').click
  254 + end
  255 +
  256 + def init_diff_note_second_file
  257 + find('a[data-line-code="8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d_28_39"]').click
  258 + end
  259 +
  260 + def have_visible_content (text)
  261 + have_css("*", text: text, visible: true)
  262 + end
195 end 263 end