diff --git a/app/assets/javascripts/single_diff.js.coffee b/app/assets/javascripts/single_diff.js.coffee
index 4d1c28c082b74022bf3663c19389646f293711ee..76db716317b8f48866a6a546061377eacdff26d7 100644
--- a/app/assets/javascripts/single_diff.js.coffee
+++ b/app/assets/javascripts/single_diff.js.coffee
@@ -2,13 +2,15 @@ class @SingleDiff
LOADING_HTML = '<i class="fa fa-spinner fa-spin"></i>'
ERROR_HTML = '<div class="nothing-here-block"><i class="fa fa-warning"></i> Could not load diff</div>'
+ COLLAPSED_HTML = '<div class="nothing-here-block diff-collapsed">This diff is collapsed. Click to expand it.</div>'
constructor: (@file) ->
@content = $('.diff-content', @file)
- @diffForPath = @content.data 'diff-for-path'
+ @diffForPath = @content.find('[data-diff-for-path]').data 'diff-for-path'
@setOpenState()
$('.file-title > a', @file).on 'click', @toggleDiff
+ @enableToggleOnContent()
setOpenState: ->
if @diffForPath
@@ -18,11 +20,15 @@ class @SingleDiff
@contentHTML = @content.html()
return
+ enableToggleOnContent: ->
+ @content.find('.nothing-here-block.diff-collapsed').on 'click', @toggleDiff
+
toggleDiff: (e) =>
e.preventDefault()
@isOpen = !@isOpen
if not @isOpen and not @hasError
- @content.empty()
+ @content.html COLLAPSED_HTML
+ @enableToggleOnContent
return
if @contentHTML
@setContentHTML()
diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss
index 71e4b50f2af57daede41477b34bb8301731b7101..02480689f09a1902952b585114bb5fe9be2ac771 100644
--- a/app/assets/stylesheets/framework/files.scss
+++ b/app/assets/stylesheets/framework/files.scss
@@ -189,3 +189,7 @@ span.idiff {
border-bottom-right-radius: 2px;
}
}
+
+.nothing-here-block.diff-collapsed {
+ cursor: pointer;
+}
diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb
index 9a5920edfa2f7f6e0d0208935e077fac0086086c..93f12198a58be37d8246eb73be0f9c7ab36d5dce 100644
--- a/app/helpers/diff_helper.rb
+++ b/app/helpers/diff_helper.rb
@@ -15,6 +15,7 @@ module DiffHelper
diff_commit = commit_for_diff(diff_file)
blob = project.repository.blob_for_diff(diff_commit, diff_file)
+ @expand_all = true
locals = {
diff_file: diff_file,
diff --git a/app/views/projects/diffs/_content.html.haml b/app/views/projects/diffs/_content.html.haml
index 832aa0c5a148fcd3655219af6966f4d690ed2c97..9c6a17c0a8c21f6af227571eb9ae1baf4f0679a2 100644
--- a/app/views/projects/diffs/_content.html.haml
+++ b/app/views/projects/diffs/_content.html.haml
@@ -9,7 +9,11 @@
- if !project.repository.diffable?(blob)
.nothing-here-block This diff was suppressed by a .gitattributes entry.
- elsif diff_file.diff_lines.length > 0
- - if diff_view == 'parallel'
+ - if diff_file.collapsed_by_default? && !@expand_all
+ - url = url_for(params.merge(action: :diff_for_path, path: diff_file.file_path, format: nil))
+ .nothing-here-block.diff-collapsed{data: { diff_for_path: url } }
+ This diff is collapsed. Click to expand it.
+ - elsif diff_view == 'parallel'
= render "projects/diffs/parallel_view", diff_file: diff_file, project: project, blob: blob
- else
= render "projects/diffs/text_file", diff_file: diff_file
diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml
index c83ed55efe120d8b73b1703c98bd25418043c3ee..c306909fb1ae6e23b37cbb2e6ddc6c9d243ce9ca 100644
--- a/app/views/projects/diffs/_file.html.haml
+++ b/app/views/projects/diffs/_file.html.haml
@@ -16,9 +16,4 @@
= view_file_btn(diff_commit.id, diff_file, project)
- - if diff_file.collapsed_by_default?
- - url = url_for(params.merge(action: :diff_for_path, path: diff_file.file_path, format: nil))
- .diff-content.diff-wrap-lines{data: { diff_for_path: url }}
- .nothing-here-block File hidden by default; content for this element available at #{url}
- - else
- = render 'projects/diffs/content', diff_file: diff_file, diff_commit: diff_commit, diff_refs: diff_refs, blob: blob, project: project
+ = render 'projects/diffs/content', diff_file: diff_file, diff_commit: diff_commit, diff_refs: diff_refs, blob: blob, project: project
diff --git a/spec/features/merge_requests/expand_collapse_diffs_spec.rb b/spec/features/merge_requests/expand_collapse_diffs_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..173ea3720da59ca8632aaf79384f4d561c983332
--- /dev/null
+++ b/spec/features/merge_requests/expand_collapse_diffs_spec.rb
@@ -0,0 +1,138 @@
+require 'spec_helper'
+
+feature 'Expand and collapse diffs', js: true, feature: true do
+ include WaitForAjax
+
+ before do
+ login_as :admin
+ merge_request = create(:merge_request, source_branch: 'expand-collapse-diffs', target_branch: 'master')
+ project = merge_request.source_project
+
+ # Ensure that undiffable.md is in .gitattributes
+ project.repository.copy_gitattributes('expand-collapse-diffs')
+ visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request)
+ execute_script('window.ajaxUris = []; $(document).ajaxSend(function(event, xhr, settings) { ajaxUris.push(settings.url) });')
+ end
+
+ def file_container(filename)
+ find("[data-blob-diff-path*='#{filename}']")
+ end
+
+ # Use define_method instead of let (which is memoized) so that this just works across a
+ # reload.
+ #
+ ['small_diff.md', 'large_diff.md', 'undiffable.md', 'too_large.md', 'too_large_image.jpg'].each do |file|
+ define_method(file.split('.').first) { file_container(file) }
+ end
+
+ context 'visiting an existing merge request' do
+ it 'shows small diffs immediately' do
+ expect(small_diff).to have_selector('.code')
+ expect(small_diff).not_to have_selector('.nothing-here-block')
+ end
+
+ it 'collapses larges diffs by default' do
+ expect(large_diff).not_to have_selector('.code')
+ expect(large_diff).to have_selector('.nothing-here-block')
+ end
+
+ it 'shows non-renderable diffs as such immediately, regardless of their size' do
+ expect(undiffable).not_to have_selector('.code')
+ expect(undiffable).to have_selector('.nothing-here-block')
+ expect(undiffable).to have_content('gitattributes')
+ end
+
+ it 'does not allow diffs that are larger than the maximum size to be expanded' do
+ expect(too_large).not_to have_selector('.code')
+ expect(too_large).to have_selector('.nothing-here-block')
+ expect(too_large).to have_content('too large')
+ end
+
+ it 'shows image diffs immediately, regardless of their size' do
+ expect(too_large_image).not_to have_selector('.nothing-here-block')
+ expect(too_large_image).to have_selector('.image')
+ end
+
+ context 'expanding a large diff' do
+ before do
+ click_link('large_diff.md')
+ wait_for_ajax
+ end
+
+ it 'makes a request to get the content' do
+ ajax_uris = evaluate_script('ajaxUris')
+
+ expect(ajax_uris).not_to be_empty
+ expect(ajax_uris.first).to include('large_diff.md')
+ end
+
+ it 'shows the diff content' do
+ expect(large_diff).to have_selector('.code')
+ expect(large_diff).not_to have_selector('.nothing-here-block')
+ end
+
+ context 'adding a comment to the expanded diff' do
+ let(:comment_text) { 'A comment' }
+
+ before do
+ large_diff.find('.line_holder', match: :prefer_exact).hover
+ large_diff.find('.add-diff-note').click
+ large_diff.find('.note-textarea').send_keys comment_text
+ large_diff.find_button('Comment').click
+ wait_for_ajax
+ end
+
+ it 'adds the comment' do
+ expect(large_diff.find('.notes')).to have_content comment_text
+ end
+
+ context 'reloading the page' do
+ before { refresh }
+
+ it 'collapses the large diff by default' do
+ expect(large_diff).not_to have_selector('.code')
+ expect(large_diff).to have_selector('.nothing-here-block')
+ end
+
+ context 'expanding the diff' do
+ before do
+ click_link('large_diff.md')
+ wait_for_ajax
+ end
+
+ it 'shows the diff content' do
+ expect(large_diff).to have_selector('.code')
+ expect(large_diff).not_to have_selector('.nothing-here-block')
+ end
+
+ it 'shows the diff comment' do
+ expect(large_diff.find('.notes')).to have_content comment_text
+ end
+ end
+ end
+ end
+ end
+
+ context 'collapsing an expanded diff' do
+ before { click_link('small_diff.md') }
+
+ it 'hides the diff content' do
+ expect(small_diff).not_to have_selector('.code')
+ expect(small_diff).to have_selector('.nothing-here-block')
+ end
+
+ context 're-expanding the same diff' do
+ before { click_link('small_diff.md') }
+
+ it 'shows the diff content' do
+ expect(small_diff).to have_selector('.code')
+ expect(small_diff).not_to have_selector('.nothing-here-block')
+ end
+
+ it 'does not make a new HTTP request' do
+ expect(evaluate_script('ajaxUris')).to be_empty
+ end
+ end
+ end
+ end
+end
diff --git a/spec/features/projects/labels/update_prioritization_spec.rb b/spec/features/projects/labels/update_prioritization_spec.rb
index 6a39c302f550b90ef16a1bb38e79777109ae8abc..98ba93b4036b3ed0d45780f904deb99868ae41d7 100644
--- a/spec/features/projects/labels/update_prioritization_spec.rb
+++ b/spec/features/projects/labels/update_prioritization_spec.rb
@@ -76,7 +76,7 @@ feature 'Prioritize labels', feature: true do
expect(page.all('li').last).to have_content('bug')
end
- visit current_url
+ refresh
wait_for_ajax
page.within('.prioritized-labels') do
diff --git a/spec/support/capybara_helpers.rb b/spec/support/capybara_helpers.rb
index 9b5c3065eed423753949cc6dad4864ee54427ed9..b57a3493aff1b01411dc5f5a8a7549030966cbaf 100644
--- a/spec/support/capybara_helpers.rb
+++ b/spec/support/capybara_helpers.rb
@@ -27,6 +27,14 @@ module CapybaraHelpers
end
end
end
+
+ # Refresh the page. Calling `visit current_url` doesn't seem to work consistently.
+ #
+ def refresh
+ url = current_url
+ visit 'about:blank'
+ visit url
+ end
end
RSpec.configure do |config|