diff --git a/CHANGELOG b/CHANGELOG index 017cf92160f4d3a7d909eaec88097f6045049d49..a2982cdb7ea819fea311b14b69c2646d086dc435 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -142,6 +142,7 @@ v 8.10.0 (unreleased) - Redesign Builds and Pipelines pages - Change status color and icon for running builds - Fix commenting issue in side by side diff view for unchanged lines + - Unify diff options in merge requests page with a options dropdown - Fix markdown rendering for: consecutive labels references, label references that begin with a digit or contains `.` - Project export filename now includes the project and namespace path - Fix last update timestamp on issues not preserved on gitlab.com and project imports diff --git a/app/assets/javascripts/application.js.coffee b/app/assets/javascripts/application.js.coffee index eceff6d91d5e8bcc4c50d400dcf781763fc11497..46dc852e308f7f7ccb50b2ba74d4cf8b03101d65 100644 --- a/app/assets/javascripts/application.js.coffee +++ b/app/assets/javascripts/application.js.coffee @@ -206,12 +206,6 @@ $ -> $('.header-content .navbar-collapse').toggle() $('.navbar-toggle').toggleClass('active') - # Show/hide comments on diff - $body.on "click", ".js-toggle-diff-comments", (e) -> - $(@).toggleClass('active') - $(@).closest(".diff-file").find(".notes_holder").toggle() - e.preventDefault() - $document.off "click", '.js-confirm-danger' $document.on "click", '.js-confirm-danger', (e) -> e.preventDefault() diff --git a/app/assets/javascripts/merge_request_tabs.js.coffee b/app/assets/javascripts/merge_request_tabs.js.coffee index 86539e0d725a81605b1143a1fba9f8acf04c3b63..fd8ce9c4bcf7447fb86125f115818506b1d3d647 100644 --- a/app/assets/javascripts/merge_request_tabs.js.coffee +++ b/app/assets/javascripts/merge_request_tabs.js.coffee @@ -164,6 +164,7 @@ class @MergeRequestTabs @diffsLoaded = true @scrollToElement("#diffs") @highlighSelectedLine() + @bindDiffOptionsEvents() @filesCommentButton = $('.files .diff-file').filesCommentButton() $(document) @@ -174,6 +175,26 @@ class @MergeRequestTabs @highlighSelectedLine() @scrollToElement("#diffs") + + bindDiffOptionsEvents: -> + + $('.inline-parallel-buttons .js-diff-comments-button').on 'click', (e) -> + e.preventDefault() + + $el = $ this + state = $el.data 'state' + newState = 'hidden' + newLabel = 'Show all comments' + + if state is 'hidden' + newLabel = 'Hide all comments' + newState = 'visible' + + $("#diffs .notes_holder").toggle() + $el.text newLabel + $el.data 'state', newState + + highlighSelectedLine: -> $('.hll').removeClass 'hll' locationHash = window.location.hash diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 75b029365f93a54c40c875ee72b6ebe652beae58..52d1f09e82caa74c51e08a66b132ccde4b23520d 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -153,9 +153,6 @@ def params_with_whitespace end def toggle_whitespace_link(url, options) - options[:class] ||= '' - options[:class] << ' btn btn-default' - link_to "#{hide_whitespace? ? 'Show' : 'Hide'} whitespace changes", url, class: options[:class] end end diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml index 8ae433b48235da9b68dc904a14cd425d454c39de..a2092d01cda27dabbcaed00b58eb2e4c45a48d13 100644 --- a/app/views/projects/diffs/_diffs.html.haml +++ b/app/views/projects/diffs/_diffs.html.haml @@ -5,16 +5,25 @@ - diff_files = safe_diff_files(diffs, diff_refs: diff_refs, repository: project.repository) .content-block.oneline-block.files-changed - .inline-parallel-buttons - - if !expand_all_diffs? && diff_files.any? { |diff_file| diff_file.collapsed? } - = link_to 'Expand all', url_for(params.merge(expand_all_diffs: 1, format: 'html')), class: 'btn btn-default' - - if show_whitespace_toggle - - if current_controller?(:commit) - = commit_diff_whitespace_link(@project, @commit, class: 'hidden-xs') - - elsif current_controller?(:merge_requests) - = diff_merge_request_whitespace_link(@project, @merge_request, class: 'hidden-xs') - - elsif current_controller?(:compare) - = diff_compare_whitespace_link(@project, params[:from], params[:to], class: 'hidden-xs') + .inline-parallel-buttons.dropdown + %button.btn.btn-default{ type: "button", data: { toggle: "dropdown" } } + Options + %span.caret + %ul.dropdown-menu + %li + %a.js-diff-comments-button{ href: "#", data: { state: "visible" } } + Hide all comments + %li + - if show_whitespace_toggle + - if current_controller?(:commit) + = commit_diff_whitespace_link(@project, @commit, {}) + - elsif current_controller?(:merge_requests) + = diff_merge_request_whitespace_link(@project, @merge_request, {}) + - elsif current_controller?(:compare) + = diff_compare_whitespace_link(@project, params[:from], params[:to], {}) + %li + - if !expand_all_diffs? && diff_files.any? { |diff_file| diff_file.collapsed? } + = link_to 'Expand all', url_for(params.merge(expand_all_diffs: 1, format: 'html')), {class: 'expand-all'} .btn-group = inline_diff_btn = parallel_diff_btn diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml index c306909fb1ae6e23b37cbb2e6ddc6c9d243ce9ca..9af0e76842048731c9de3aac48725faa9e49fc85 100644 --- a/app/views/projects/diffs/_file.html.haml +++ b/app/views/projects/diffs/_file.html.haml @@ -4,11 +4,6 @@ - unless diff_file.submodule? .file-actions.hidden-xs - - if blob_text_viewable?(blob) - = link_to '#', class: 'js-toggle-diff-comments btn active has-tooltip btn-file-option', title: "Toggle comments for this file" do - = icon('comment') - \ - - if editable_diff?(diff_file) = edit_blob_link(@merge_request.source_project, @merge_request.source_branch, diff_file.new_path, diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature index 21768c15c170d4e29fabcb958d83681b7f9b057f..c1d4ec822ebd48591cb90369d3da8632063c8258 100644 --- a/features/project/merge_requests.feature +++ b/features/project/merge_requests.feature @@ -194,7 +194,7 @@ Feature: Project Merge Requests And I visit merge request page "Bug NS-05" And I click on the Changes tab And I leave a comment like "Line is wrong" on line 39 of the third file - And I click link "Hide inline discussion" of the third file + And I click link "Hide inline discussion" button Then I should not see a comment like "Line is wrong here" in the third file @javascript @@ -212,9 +212,8 @@ Feature: Project Merge Requests And I click on the Changes tab And I leave a comment like "Line is correct" on line 12 of the second file And I leave a comment like "Line is wrong" on line 39 of the third file - And I click link "Hide inline discussion" of the third file + And I click link "Hide inline discussion" button Then I should not see a comment like "Line is wrong here" in the third file - And I should still see a comment like "Line is correct" in the second file @javascript Scenario: I show comments on a merge request diff with comments in multiple files @@ -223,8 +222,8 @@ Feature: Project Merge Requests And I click on the Changes tab And I leave a comment like "Line is correct" on line 12 of the second file And I leave a comment like "Line is wrong" on line 39 of the third file - And I click link "Hide inline discussion" of the third file - And I click link "Show inline discussion" of the third file + And I click link "Hide inline discussion" button + And I click link "Show inline discussion" button Then I should see a comment like "Line is wrong" in the third file And I should still see a comment like "Line is correct" in the second file diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index da848afd48ebfe9aa4e06785106a0bf9755944fe..77602646d062f98121b4da437e0f0e8274b67a26 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -401,15 +401,17 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps end end - step 'I click link "Hide inline discussion" of the third file' do - page.within '.files [id^=diff]:nth-child(3)' do - find('.js-toggle-diff-comments').trigger('click') + step 'I click link "Hide inline discussion" button' do + page.within '#diffs' do + find('.inline-parallel-buttons [data-toggle=dropdown]').trigger('click') + find('.js-diff-comments-button').trigger('click') end end - step 'I click link "Show inline discussion" of the third file' do - page.within '.files [id^=diff]:nth-child(3)' do - find('.js-toggle-diff-comments').trigger('click') + step 'I click link "Show inline discussion" button' do + page.within '#diffs' do + find('.inline-parallel-buttons [data-toggle=dropdown]').trigger('click') + find('.js-diff-comments-button').trigger('click') end end diff --git a/spec/features/expand_collapse_diffs_spec.rb b/spec/features/expand_collapse_diffs_spec.rb index 688f68d3cff30c56dd75ed2919af35e398b27ed0..853441426dbd002ba57d000f2fc91bda42b4b41a 100644 --- a/spec/features/expand_collapse_diffs_spec.rb +++ b/spec/features/expand_collapse_diffs_spec.rb @@ -210,7 +210,8 @@ def file_container(filename) context 'expanding all diffs' do before do - click_link('Expand all') + find('.inline-parallel-buttons [data-toggle=dropdown]').trigger('click') + find('.inline-parallel-buttons .expand-all').trigger('click') wait_for_ajax execute_script('window.ajaxUris = []; $(document).ajaxSend(function(event, xhr, settings) { ajaxUris.push(settings.url) });') end