From 96b83a58cc7f9a8c5b3fdf2396f5188029a4f280 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 1 Sep 2016 15:23:39 +0300 Subject: Improve merge request version switch/compare dropdown Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/pages/merge_requests.scss | 6 +- app/helpers/git_helper.rb | 4 ++ .../merge_requests/show/_versions.html.haml | 69 +++++++++++----------- .../merge_requests/merge_request_versions_spec.rb | 25 ++++++-- 4 files changed, 65 insertions(+), 39 deletions(-) diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 7fdd79fa8b9..dd3e79b37af 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -375,7 +375,7 @@ } } -.mr-version-switch { +.mr-version-controls { background: $background-color; padding: $gl-btn-padding; color: $gl-placeholder-color; @@ -383,6 +383,10 @@ a.btn-link { color: $gl-dark-link-color; } + + .compare-dots { + margin: 0 $btn-side-margin; + } } .merge-request-details { diff --git a/app/helpers/git_helper.rb b/app/helpers/git_helper.rb index 09684955233..9d7982e169c 100644 --- a/app/helpers/git_helper.rb +++ b/app/helpers/git_helper.rb @@ -2,4 +2,8 @@ module GitHelper def strip_gpg_signature(text) text.gsub(/-----BEGIN PGP SIGNATURE-----(.*)-----END PGP SIGNATURE-----/m, "") end + + def short_sha(text) + text[0...8] + end end diff --git a/app/views/projects/merge_requests/show/_versions.html.haml b/app/views/projects/merge_requests/show/_versions.html.haml index 13bec019988..bdedaa31e12 100644 --- a/app/views/projects/merge_requests/show/_versions.html.haml +++ b/app/views/projects/merge_requests/show/_versions.html.haml @@ -1,57 +1,58 @@ - merge_request_diffs = @merge_request.merge_request_diffs.select_without_diff +- compareable_diffs = merge_request_diffs.select { |diff| diff.id < @merge_request_diff.id } - if merge_request_diffs.size > 1 - .mr-version-switch - Version: - %span.dropdown.inline + .mr-version-controls + Version + %span.dropdown.inline.mr-version-dropdown %a.btn-link.dropdown-toggle{ data: {toggle: :dropdown} } %strong.monospace< - if @merge_request_diff.latest? - #{"latest"} + Latest: #{short_sha(@merge_request_diff.head_commit_sha)} - else - #{@merge_request_diff.head_commit.short_id} + #{short_sha(@merge_request_diff.head_commit_sha)} %span.caret %ul.dropdown-menu.dropdown-menu-selectable - - merge_request_diffs.each do |merge_request_diff| + - merge_request_diffs.each_with_index do |merge_request_diff, i| %li = link_to diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request, diff_id: merge_request_diff.id), class: ('is-active' if merge_request_diff == @merge_request_diff) do %strong.monospace + - if i.zero? + Latest: + - else + #{merge_request_diffs.size - i}. #{merge_request_diff.head_commit.short_id} %br %small #{number_with_delimiter(merge_request_diff.commits.count)} #{'commit'.pluralize(merge_request_diff.commits.count)}, = time_ago_with_tooltip(merge_request_diff.created_at) - %span.prepend-left-default - Compared with: - %span.dropdown.inline - %a.btn-link.dropdown-toggle{ data: {toggle: :dropdown} } - %strong.monospace< - - if params[:start_sha].present? - #{params[:start_sha][0...8]} - - else - #{"base"} - %span.caret - %ul.dropdown-menu.dropdown-menu-selectable - - merge_request_diffs.each do |merge_request_diff| - - next if merge_request_diff.id >= @merge_request_diff.id - %li - = link_to mr_version_path(@project, @merge_request, @merge_request_diff, merge_request_diff.head_commit_sha), class: ('is-active' if merge_request_diff.head_commit_sha == params[:start_sha]) do - %strong.monospace - #{merge_request_diff.head_commit.short_id} - %br - %small - = time_ago_with_tooltip(merge_request_diff.created_at) + %span.compare-dots ... + + Compared with + %span.dropdown.inline.mr-version-compare-dropdown + %a.btn-link.dropdown-toggle{ data: {toggle: :dropdown} } + %strong.monospace< + - if params[:start_sha].present? + #{short_sha(params[:start_sha])} + - else + Base: #{short_sha(@merge_request_diff.base_commit_sha)} + %span.caret + %ul.dropdown-menu.dropdown-menu-selectable + - compareable_diffs.each_with_index do |merge_request_diff, i| %li - = link_to mr_version_path(@project, @merge_request, @merge_request_diff), class: ('is-active' unless params[:start_sha].present?) do + = link_to mr_version_path(@project, @merge_request, @merge_request_diff, merge_request_diff.head_commit_sha), class: ('is-active' if merge_request_diff.head_commit_sha == params[:start_sha]) do %strong.monospace - base - + #{compareable_diffs.size - i}. #{short_sha(merge_request_diff.head_commit_sha)} + %br + %small + = time_ago_with_tooltip(merge_request_diff.created_at) + %li + = link_to mr_version_path(@project, @merge_request, @merge_request_diff), class: ('is-active' unless params[:start_sha].present?) do + %strong.monospace + Base: #{short_sha(@merge_request_diff.base_commit_sha)} - unless @merge_request_diff.latest? && params[:start_sha].blank? - .pull-right + .prepend-top-10 = icon('info-circle') - - if params[:start_sha].present? - Comments are disabled when compare with version different from base - - else - This version is not the latest one. Comments are disabled + Comments are disabled while viewing outdated merge versions or comparing to versions other than base. diff --git a/spec/features/merge_requests/merge_request_versions_spec.rb b/spec/features/merge_requests/merge_request_versions_spec.rb index 577c910f11b..df66dd23448 100644 --- a/spec/features/merge_requests/merge_request_versions_spec.rb +++ b/spec/features/merge_requests/merge_request_versions_spec.rb @@ -11,7 +11,7 @@ feature 'Merge Request versions', js: true, feature: true do end it 'show the latest version of the diff' do - page.within '.mr-version-switch' do + page.within '.mr-version-dropdown' do expect(page).to have_content 'Version: latest' end @@ -20,15 +20,32 @@ feature 'Merge Request versions', js: true, feature: true do describe 'switch between versions' do before do - page.within '.mr-version-switch' do + page.within '.mr-version-dropdown' do find('.btn-link').click click_link '6f6d7e7e' end end it 'should show older version' do - page.within '.mr-version-switch' do - expect(page).to have_content 'Version: 6f6d7e7e' + page.within '.mr-version-dropdown' do + expect(page).to have_content '6f6d7e7e' + end + + expect(page).to have_content '5 changed files' + end + end + + describe 'compare with older version' do + before do + page.within '.mr-version-compare-dropdown' do + find('.btn-link').click + click_link '6f6d7e7e' + end + end + + it 'should show older version' do + page.within '.mr-version-compare-dropdown' do + expect(page).to have_content '6f6d7e7e' end expect(page).to have_content '5 changed files' -- cgit v1.2.1