diff options
author | Jacob Schatz <jschatz1@gmail.com> | 2016-06-18 11:19:57 -0400 |
---|---|---|
committer | Paco Guzman <pacoguzmanp@gmail.com> | 2016-07-06 08:29:57 +0200 |
commit | e065f4848b257256141752e6498398cd68fa7786 (patch) | |
tree | 5b2775a3ad3e7f02d668faf850b0d31e5c76f4a2 | |
parent | cfd5870b62e9d76e564ffc64db1d1281b4a363bb (diff) | |
download | gitlab-ce-e065f4848b257256141752e6498398cd68fa7786.tar.gz |
Diffs will create button/diff form on demand no on server side
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/assets/javascripts/diff.js.coffee | 4 | ||||
-rw-r--r-- | app/assets/javascripts/files_comment_button.js.coffee | 82 | ||||
-rw-r--r-- | app/assets/javascripts/merge_request_tabs.js.coffee | 1 | ||||
-rw-r--r-- | app/controllers/projects/compare_controller.rb | 1 | ||||
-rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 1 | ||||
-rw-r--r-- | app/helpers/notes_helper.rb | 20 | ||||
-rw-r--r-- | app/mailers/emails/projects.rb | 1 | ||||
-rw-r--r-- | app/views/projects/diffs/_diffs.html.haml | 2 | ||||
-rw-r--r-- | app/views/projects/diffs/_line.html.haml | 15 | ||||
-rw-r--r-- | app/views/projects/diffs/_match_line_parallel.html.haml | 4 | ||||
-rw-r--r-- | app/views/projects/diffs/_parallel_view.html.haml | 26 | ||||
-rw-r--r-- | app/views/projects/show.html.haml | 2 | ||||
-rw-r--r-- | features/project/commits/diff_comments.feature | 4 | ||||
-rw-r--r-- | features/steps/shared/diff_note.rb | 14 | ||||
-rw-r--r-- | spec/features/notes_on_merge_requests_spec.rb | 3 |
16 files changed, 121 insertions, 60 deletions
diff --git a/CHANGELOG b/CHANGELOG index 8ef934bf80d..15344fc6886 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -24,6 +24,7 @@ v 8.10.0 (unreleased) - Allow importing from Github using Personal Access Tokens. (Eric K Idema) - API: Todos !3188 (Robert Schilling) - Add "Enabled Git access protocols" to Application Settings + - Diffs will create button/diff form on demand no on server side - Fix user creation with stronger minimum password requirements !4054 (nathan-pmt) - PipelinesFinder uses git cache data - Check for conflicts with existing Project's wiki path when creating a new project. diff --git a/app/assets/javascripts/diff.js.coffee b/app/assets/javascripts/diff.js.coffee index 6d9b364cb8d..ddb43f02dec 100644 --- a/app/assets/javascripts/diff.js.coffee +++ b/app/assets/javascripts/diff.js.coffee @@ -1,6 +1,8 @@ class @Diff UNFOLD_COUNT = 20 constructor: -> + @filesCommentButton = new FilesCommentButton($('.files')) + $(document).off('click', '.js-unfold') $(document).on('click', '.js-unfold', (event) => target = $(event.target) @@ -36,7 +38,7 @@ class @Diff # see https://gitlab.com/gitlab-org/gitlab-ce/issues/707 indent: 1 - $.get(link, params, (response) => + $.get(link, params, (response) -> target.parent().replaceWith(response) ) ) diff --git a/app/assets/javascripts/files_comment_button.js.coffee b/app/assets/javascripts/files_comment_button.js.coffee new file mode 100644 index 00000000000..56768991dd8 --- /dev/null +++ b/app/assets/javascripts/files_comment_button.js.coffee @@ -0,0 +1,82 @@ +class @FilesCommentButton + constructor: (@filesContainerElement) -> + return if not @filesContainerElement and not @filesContainerElement.data 'can-create-note' + + @COMMENT_BUTTON_CLASS = '.add-diff-note' + @COMMENT_BUTTON_TEMPLATE = _.template("<button name='button' type='submit' class='btn <%- COMMENT_BUTTON_CLASS %> js-add-diff-note-button' title='Add a comment to this line'><i class='fa fa-comment-o'></i></button>") + + @LINE_HOLDER_CLASS = '.line_holder' + @LINE_NUMBER_CLASS = 'diff-line-num' + @LINE_CONTENT_CLASS = 'line_content' + @LINE_COLUMN_CLASSES = ".#{@LINE_NUMBER_CLASS}, .line_content" + + @DEBOUNCE_TIMEOUT_DURATION = 150 + + $(document) + .on 'mouseover', @LINE_COLUMN_CLASSES, @debounceRender + .on 'mouseleave', @LINE_COLUMN_CLASSES, @destroy + + debounceRender: (e) => + clearTimeout @debounceTimeout if @debounceTimeout + @debounceTimeout = setTimeout => + @render e + , @DEBOUNCE_TIMEOUT_DURATION + return + + render: (e) -> + lineHolderElement = @getLineHolder($(e.currentTarget)) + lineContentElement = @getLineContent($(e.currentTarget)) + lineNumElement = @getLineNum($(e.currentTarget)) + buttonParentElement = lineNumElement + + return if not @shouldRender e, buttonParentElement + + buttonParentElement.append @buildButton + id: + noteable: lineHolderElement.attr 'data-noteable-id' + commit: lineHolderElement.attr 'data-commit-id' + discussion: lineContentElement.attr('data-discussion-id') || lineHolderElement.attr('data-discussion-id') + type: + noteable: lineHolderElement.attr 'data-noteable-type' + note: lineHolderElement.attr 'data-note-type' + line: lineContentElement.attr 'data-line-type' + code: + line: lineContentElement.attr('data-line-code') || lineHolderElement.attr('id') + return + + destroy: (e) => + return if @isMovingToSameType e + $(@COMMENT_BUTTON_CLASS, @getLineNum $(e.currentTarget)).remove() + return + + buildButton: (buttonAttributes) -> + $(@COMMENT_BUTTON_TEMPLATE COMMENT_BUTTON_CLASS: @COMMENT_BUTTON_CLASS.substr 1).attr + 'data-noteable-id': buttonAttributes.id.noteable + 'data-commit-id': buttonAttributes.id.commit + 'data-discussion-id': buttonAttributes.id.discussion + 'data-noteable-type': buttonAttributes.type.noteable + 'data-line-type': buttonAttributes.type.line + 'data-note-type': buttonAttributes.type.note + 'data-line-code': buttonAttributes.code.line + + getLineHolder: (hoveredElement) -> + return hoveredElement if hoveredElement.hasClass @LINE_HOLDER_CLASS + $(hoveredElement.parent()) + + getLineNum: (hoveredElement) -> + return hoveredElement if hoveredElement.hasClass @LINE_NUMBER_CLASS + + $(hoveredElement).prev('.' + @LINE_NUMBER_CLASS) + + getLineContent: (hoveredElement) -> + return hoveredElement if hoveredElement.hasClass @LINE_CONTENT_CLASS + + $(hoveredElement).next('.' + @LINE_CONTENT_CLASS) + + isMovingToSameType: (e) -> + newLineNum = @getLineNum($(e.toElement)) + return false unless newLineNum + (newLineNum).is @getLineNum($(e.currentTarget)) + + shouldRender: (e, buttonParentElement) -> + (!buttonParentElement.hasClass('empty-cell') and $(@COMMENT_BUTTON_CLASS, buttonParentElement).length is 0) diff --git a/app/assets/javascripts/merge_request_tabs.js.coffee b/app/assets/javascripts/merge_request_tabs.js.coffee index 894f80586f1..7b0ebfb4490 100644 --- a/app/assets/javascripts/merge_request_tabs.js.coffee +++ b/app/assets/javascripts/merge_request_tabs.js.coffee @@ -153,7 +153,6 @@ class @MergeRequestTabs loadDiff: (source) -> return if @diffsLoaded - @_get url: "#{source}.json" + @_location.search success: (data) => diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index af0b69a2442..57725944d68 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -24,6 +24,7 @@ class Projects::CompareController < Projects::ApplicationController @diff_refs = [@base_commit, @commit] @diff_notes_disabled = true @grouped_diff_notes = {} + @comments_target = {} end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index dd86b940a08..99630bc1099 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -138,6 +138,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @base_commit = @merge_request.diff_base_commit @diffs = @merge_request.compare.diffs(diff_options) if @merge_request.compare @diff_notes_disabled = true + @comments_target = {} @pipeline = @merge_request.pipeline @statuses = @pipeline.statuses if @pipeline diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index e85ba76887d..42419aa908e 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -24,28 +24,12 @@ module NotesHelper }.to_json end - def link_to_new_diff_note(line_code, line_type = nil) - discussion_id = LegacyDiffNote.build_discussion_id( + def discussion_id(line_code) + LegacyDiffNote.build_discussion_id( @comments_target[:noteable_type], @comments_target[:noteable_id] || @comments_target[:commit_id], line_code ) - - data = { - noteable_type: @comments_target[:noteable_type], - noteable_id: @comments_target[:noteable_id], - commit_id: @comments_target[:commit_id], - line_type: line_type, - line_code: line_code, - note_type: LegacyDiffNote.name, - discussion_id: discussion_id - } - - button_tag(class: 'btn add-diff-note js-add-diff-note-button', - data: data, - title: 'Add a comment to this line') do - icon('comment-o') - end end def link_to_reply_discussion(note, line_type = nil) diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb index e0af7081411..1a6900b3293 100644 --- a/app/mailers/emails/projects.rb +++ b/app/mailers/emails/projects.rb @@ -30,6 +30,7 @@ module Emails @target_url = @message.target_url @project = Project.find(project_id) @diff_notes_disabled = true + @comments_target = {} add_project_headers headers['X-GitLab-Author'] = @message.author_username diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml index f18bc8c41b3..fea294628fe 100644 --- a/app/views/projects/diffs/_diffs.html.haml +++ b/app/views/projects/diffs/_diffs.html.haml @@ -21,7 +21,7 @@ - if diff_files.overflow? = render 'projects/diffs/warning', diff_files: diff_files -.files +.files{data: {can_create_note: (!@diff_notes_disabled && can?(current_user, :create_note, @project)).to_s}} - diff_files.each_with_index do |diff_file, index| - diff_commit = commit_for_diff(diff_file) - blob = project.repository.blob_for_diff(diff_commit, diff_file) diff --git a/app/views/projects/diffs/_line.html.haml b/app/views/projects/diffs/_line.html.haml index f1577e8a47b..31e8378c9dc 100644 --- a/app/views/projects/diffs/_line.html.haml +++ b/app/views/projects/diffs/_line.html.haml @@ -1,5 +1,6 @@ - type = line.type -%tr.line_holder{ id: line_code, class: type } +- holder_data = @comments_target.any? ? { data: { noteable_id: @comments_target[:noteable_id], noteable_type: @comments_target[:noteable_type], commit_id: @comments_target[:commit_id], discussion_id: discussion_id(line_code), note_type: LegacyDiffNote.name } } : {} +%tr.line_holder{ holder_data, id: line_code, class: type } - case type - when 'match' = render "projects/diffs/match_line", { line: line.text, @@ -10,17 +11,15 @@ %td.line_content.match= line.text - else %td.old_line.diff-line-num{ class: type, data: { linenumber: line.new_pos } } - - link_text = type == "new" ? " ".html_safe : line.old_pos + - link_text = type == "new" ? " " : line.old_pos - if defined?(plain) && plain = link_text - else - = link_to "", "##{line_code}", id: line_code, data: { linenumber: link_text } - - if !@diff_notes_disabled && can?(current_user, :create_note, @project) - = link_to_new_diff_note(line_code) + %a{href: "##{line_code}", data: { linenumber: link_text }}= " " %td.new_line.diff-line-num{ class: type, data: { linenumber: line.new_pos } } - - link_text = type == "old" ? " ".html_safe : line.new_pos + - link_text = type == "old" ? " " : line.new_pos - if defined?(plain) && plain = link_text - else - = link_to "", "##{line_code}", id: line_code, data: { linenumber: link_text } - %td.line_content{ class: ['noteable_line', type, line_code], data: { line_code: line_code } }= diff_line_content(line.text, type) + %a{href: "##{line_code}", data: { linenumber: link_text }}= "" + %td.line_content{ class: ['noteable_line', type], data: { line_code: line_code, line_type: type } }= diff_line_content(line.text, type) diff --git a/app/views/projects/diffs/_match_line_parallel.html.haml b/app/views/projects/diffs/_match_line_parallel.html.haml index 0cd888876e0..b9c0d9dcdfd 100644 --- a/app/views/projects/diffs/_match_line_parallel.html.haml +++ b/app/views/projects/diffs/_match_line_parallel.html.haml @@ -1,4 +1,4 @@ -%td.old_line.diff-line-num +%td.old_line.diff-line-num.empty-cell %td.line_content.parallel.match= line -%td.new_line.diff-line-num +%td.new_line.diff-line-num.empty-cell %td.line_content.parallel.match= line diff --git a/app/views/projects/diffs/_parallel_view.html.haml b/app/views/projects/diffs/_parallel_view.html.haml index 4ecc9528bd2..089ddf23240 100644 --- a/app/views/projects/diffs/_parallel_view.html.haml +++ b/app/views/projects/diffs/_parallel_view.html.haml @@ -4,21 +4,19 @@ - diff_file.parallel_diff_lines.each do |line| - left = line[:left] - right = line[:right] - %tr.line_holder.parallel + - holder_data = @comments_target.any? ? { data: { noteable_id: @comments_target[:noteable_id], noteable_type: @comments_target[:noteable_type], commit_id: @comments_target[:commit_id], note_type: LegacyDiffNote.name } } : {} + %tr.line_holder.parallel{ holder_data } - if left[:type] == 'match' - = render "projects/diffs/match_line_parallel", { line: left[:text], - line_old: left[:number], line_new: right[:number] } + = render "projects/diffs/match_line_parallel", { line: left[:text] } - elsif left[:type] == 'nonewline' - %td.old_line.diff-line-num + %td.old_line.diff-line-num.empty-cell %td.line_content.parallel.match= left[:text] - %td.new_line.diff-line-num + %td.new_line.diff-line-num.empty-cell %td.line_content.parallel.match= left[:text] - else - %td.old_line.diff-line-num{id: left[:line_code], class: "#{left[:type]} #{'empty-cell' if !left[:number]}"} - = link_to raw(left[:number]), "##{left[:line_code]}", id: left[:line_code] - - if !@diff_notes_disabled && can?(current_user, :create_note, @project) - = link_to_new_diff_note(left[:line_code], 'old') - %td.line_content{class: "parallel noteable_line #{left[:type]} #{left[:line_code]} #{'empty-cell' if left[:text].empty?}", data: { line_code: left[:line_code] }}= diff_line_content(left[:text]) + %td.old_line.diff-line-num{id: left[:line_code], class: "#{left[:type]} #{'empty-cell' if !left[:number]}", data: { linenumber: left[:number] }} + %a{href: "##{left[:line_code]}" }= raw(left[:number]) + %td.line_content{class: "parallel noteable_line #{left[:type]} #{'empty-cell' if left[:text].empty?}", data: { discussion_id: discussion_id(left[:line_code]), line_type: left[:type], line_code: left[:line_code] }}= diff_line_content(left[:text]) - if right[:type] == 'new' - new_line_class = 'new' @@ -27,11 +25,9 @@ - new_line_class = nil - new_line_code = left[:line_code] - %td.new_line.diff-line-num{id: new_line_code, class: "#{new_line_class} #{'empty-cell' if !right[:number]}", data: { linenumber: right[:number] }} - = link_to raw(right[:number]), "##{new_line_code}", id: new_line_code - - if !@diff_notes_disabled && can?(current_user, :create_note, @project) - = link_to_new_diff_note(new_line_code, 'new') - %td.line_content.parallel{class: "noteable_line #{new_line_class} #{new_line_code} #{'empty-cell' if right[:text].empty?}", data: { line_code: new_line_code }}= diff_line_content(right[:text]) + %td.new_line.diff-line-num{id: new_line_code, class: "#{new_line_class} #{'empty-cell' if !right[:number]}", data: { linenumber: right[:number] } } + %a{href: "##{new_line_code}" }= raw(right[:number]) + %td.line_content.parallel{class: "noteable_line #{new_line_class} #{'empty-cell' if right[:text].empty?}", data: { discussion_id: discussion_id(new_line_code), line_type: new_line_class, line_code: new_line_code }}= diff_line_content(right[:text]) - unless @diff_notes_disabled - notes_left, notes_right = organize_comments(left, right) diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index 58d8e068754..dd1cf680cfa 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -82,4 +82,4 @@ Archived project! Repository is read-only %div{class: "project-show-#{default_project_view}"} - = render default_project_view + = render default_project_view
\ No newline at end of file diff --git a/features/project/commits/diff_comments.feature b/features/project/commits/diff_comments.feature index 2bde4c8a99b..35687aac9ea 100644 --- a/features/project/commits/diff_comments.feature +++ b/features/project/commits/diff_comments.feature @@ -6,10 +6,6 @@ Feature: Project Commits Diff Comments And I visit project commit page @javascript - Scenario: I can access add diff comment buttons - Then I should see add a diff comment button - - @javascript Scenario: I can comment on a commit diff Given I leave a diff comment like "Typo, please fix" Then I should see a diff comment saying "Typo, please fix" diff --git a/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb index e8b1e4b4879..8dc461bdd95 100644 --- a/features/steps/shared/diff_note.rb +++ b/features/steps/shared/diff_note.rb @@ -32,8 +32,8 @@ module SharedDiffNote end step 'I leave a diff comment in a parallel view on the left side like "Old comment"' do - click_parallel_diff_line(sample_commit.line_code, 'old') - page.within("#{diff_file_selector} form[id$='#{sample_commit.line_code}-true']") do + click_parallel_diff_line(sample_commit.del_line_code, 'old') + page.within("#{diff_file_selector} form[id$='#{sample_commit.del_line_code}-true']") do fill_in "note[note]", with: "Old comment" find(".js-comment-button").trigger("click") end @@ -165,10 +165,6 @@ module SharedDiffNote end end - step 'I should see add a diff comment button' do - expect(page).to have_css('.js-add-diff-note-button') - end - step 'I should see an empty diff comment form' do page.within(diff_file_selector) do expect(page).to have_field("note[note]", with: "") @@ -227,10 +223,12 @@ module SharedDiffNote end def click_diff_line(code) - find("button[data-line-code='#{code}']").trigger('click') + find(".line_holder[id='#{code}'] td:nth-of-type(1)").hover + find(".line_holder[id='#{code}'] button").trigger('click') end def click_parallel_diff_line(code, line_type) - find("button[data-line-code='#{code}'][data-line-type='#{line_type}']").trigger('click') + find(".line_content.parallel.#{line_type}[data-line-code='#{code}']").trigger('mouseover') + find(".line_holder.parallel button[data-line-code='#{code}']").trigger('click') end end diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index 737efcef45d..bb53475bf6c 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -229,6 +229,7 @@ describe 'Comments', feature: true do end def click_diff_line(data = line_code) - execute_script("$('button[data-line-code=\"#{data}\"]').click()") + find(".line_holder[id='#{data}'] td:nth-of-type(1)").hover + find(".line_holder[id='#{data}'] button").trigger('click') end end |