From 97dc1b24d34b718b57a9e4dbf5c05c6218315eca Mon Sep 17 00:00:00 2001 From: Bryce Johnson Date: Wed, 14 Sep 2016 13:55:07 +0200 Subject: [PATCH] Style merge request diff dropdowns & disable comments. --- CHANGELOG | 4 + .../stylesheets/pages/merge_requests.scss | 21 ++++- app/views/discussions/_jump_to_next.html.haml | 5 +- app/views/projects/diffs/_file.html.haml | 2 +- .../merge_requests/show/_versions.html.haml | 87 ++++++++++--------- .../merge_request_versions_spec.rb | 15 +++- 6 files changed, 87 insertions(+), 47 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index d133909748b..a348d790089 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -163,6 +163,10 @@ v 8.11.6 - Fix an error where we were unable to create a CommitStatus for running state. !6107 - Optimize discussion notes resolving and unresolving. !6141 - Fix GitLab import button. !6167 + - Frontend for Merge Request diff dropdowns + +v 8.11.6 (unreleased) + - Fix an error where we were unable to create a CommitStatus for running state - Restore SSH Key title auto-population behavior. !6186 - Fix DB schema to match latest migration. !6256 - Exclude some pending or inactivated rows in Member scopes. diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 96c06086867..fc61bdff072 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -373,11 +373,26 @@ .mr-version-controls { background: $background-color; - padding: $gl-btn-padding; + border: 1px solid $border-color; + border-top: 0; + padding: 0 16px; color: $gl-placeholder-color; - a.btn-link { - color: $gl-dark-link-color; + .mr-version-dropdown, + .mr-version-compare-dropdown { + margin: 0 10px; + } + + .comments-disabled-notif { + border-top: 1px solid $border-color; + } + + .dropdown-title { + color: $gl-text-color; + } + + .fa-info-circle { + color: $red-normal; } } diff --git a/app/views/discussions/_jump_to_next.html.haml b/app/views/discussions/_jump_to_next.html.haml index 69bd416c4de..1e41b54be38 100644 --- a/app/views/discussions/_jump_to_next.html.haml +++ b/app/views/discussions/_jump_to_next.html.haml @@ -1,3 +1,5 @@ +-if @merge_request_diff + - comments_disabled = @merge_request_diff.latest? && !!@start_sha - discussion = local_assigns.fetch(:discussion, nil) - if current_user %jump-to-discussion{ "inline-template" => true, ":discussion-id" => "'#{discussion.try(:id)}'" } @@ -5,5 +7,6 @@ %button.btn.btn-default.discussion-next-btn.has-tooltip{ "@click" => "jumpToNextUnresolvedDiscussion", title: "Jump to next unresolved discussion", "aria-label" => "Jump to next unresolved discussion", - data: { container: "body" } } + data: { container: "body" }, + disabled: comments_disabled } = custom_icon("next_discussion") diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml index ad2eb3e504f..1a51ccd4c7d 100644 --- a/app/views/projects/diffs/_file.html.haml +++ b/app/views/projects/diffs/_file.html.haml @@ -5,7 +5,7 @@ - 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 + = link_to '#', class: 'js-toggle-diff-comments btn active has-tooltip btn-file-option', title: "Toggle comments for this files", disabled: @diff_notes_disabled do = icon('comment') \ diff --git a/app/views/projects/merge_requests/show/_versions.html.haml b/app/views/projects/merge_requests/show/_versions.html.haml index 00287f2d245..1149c010962 100644 --- a/app/views/projects/merge_requests/show/_versions.html.haml +++ b/app/views/projects/merge_requests/show/_versions.html.haml @@ -1,42 +1,23 @@ - if @merge_request_diffs.size > 1 .mr-version-controls - Changes between - %span.dropdown.inline.mr-version-dropdown - %a.btn-link.dropdown-toggle{ data: {toggle: :dropdown} } - %strong - - if @merge_request_diff.latest? - latest version - - else - version #{version_index(@merge_request_diff)} - %span.caret - %ul.dropdown-menu.dropdown-menu-selectable - - @merge_request_diffs.each do |merge_request_diff| - %li - = link_to merge_request_version_path(@project, @merge_request, merge_request_diff), class: ('is-active' if merge_request_diff == @merge_request_diff) do - %strong - - if merge_request_diff.latest? - latest version - - else - version #{version_index(merge_request_diff)} - .monospace #{short_sha(merge_request_diff.head_commit_sha)} - %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) - - - if @merge_request_diff.base_commit_sha - and - %span.dropdown.inline.mr-version-compare-dropdown - %a.btn-link.dropdown-toggle{ data: {toggle: :dropdown} } - %strong - - if @start_sha - version #{version_index(@start_version)} + %div.mr-version-menus-container.content-block + Changes between + %span.dropdown.inline.mr-version-dropdown + %a.dropdown-toggle.btn.btn-default{ data: {toggle: :dropdown} } + %span + - if @merge_request_diff.latest? + latest version - else - #{@merge_request.target_branch} + version #{version_index(@merge_request_diff)} %span.caret %ul.dropdown-menu.dropdown-menu-selectable - - @comparable_diffs.each do |merge_request_diff| + .dropdown-title + %span Version: + %button.dropdown-title-button.dropdown-menu-close + %i.fa.fa-times.dropdown-menu-close-icon + - @merge_request_diffs.each do |merge_request_diff| %li - = link_to merge_request_version_path(@project, @merge_request, @merge_request_diff, merge_request_diff.head_commit_sha), class: ('is-active' if merge_request_diff == @start_version) do + = link_to merge_request_version_path(@project, @merge_request, merge_request_diff), class: ('is-active' if merge_request_diff == @merge_request_diff) do %strong - if merge_request_diff.latest? latest version @@ -44,15 +25,43 @@ version #{version_index(merge_request_diff)} .monospace #{short_sha(merge_request_diff.head_commit_sha)} %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) - %li - = link_to merge_request_version_path(@project, @merge_request, @merge_request_diff), class: ('is-active' unless @start_sha) do - %strong - #{@merge_request.target_branch} (base) - .monospace #{short_sha(@merge_request_diff.base_commit_sha)} + + - if @merge_request_diff.base_commit_sha + and + %span.dropdown.inline.mr-version-compare-dropdown + %a.btn.btn-default.dropdown-toggle{ data: {toggle: :dropdown} } + %span + - if @start_sha + version #{version_index(@start_version)} + - else + #{@merge_request.target_branch} + %span.caret + %ul.dropdown-menu.dropdown-menu-selectable + .dropdown-title + %span Compared with: + %button.dropdown-title-button.dropdown-menu-close + %i.fa.fa-times.dropdown-menu-close-icon + - @comparable_diffs.each do |merge_request_diff| + %li + = link_to merge_request_version_path(@project, @merge_request, @merge_request_diff, merge_request_diff.head_commit_sha), class: ('is-active' if merge_request_diff == @start_version) do + %strong + - if merge_request_diff.latest? + latest version + - else + version #{version_index(merge_request_diff)} + .monospace #{short_sha(merge_request_diff.head_commit_sha)} + %small + = time_ago_with_tooltip(merge_request_diff.created_at) + %li + = link_to merge_request_version_path(@project, @merge_request, @merge_request_diff), class: ('is-active' unless @start_sha) do + %strong + #{@merge_request.target_branch} (base) + .monospace #{short_sha(@merge_request_diff.base_commit_sha)} - unless @merge_request_diff.latest? && !@start_sha - .prepend-top-10 + .comments-disabled-notif.content-block = icon('info-circle') - if @start_sha Comments are disabled because you're comparing two versions of this merge request. diff --git a/spec/features/merge_requests/merge_request_versions_spec.rb b/spec/features/merge_requests/merge_request_versions_spec.rb index 9e759de3752..c2ea3d8b954 100644 --- a/spec/features/merge_requests/merge_request_versions_spec.rb +++ b/spec/features/merge_requests/merge_request_versions_spec.rb @@ -21,7 +21,7 @@ feature 'Merge Request versions', js: true, feature: true do describe 'switch between versions' do before do page.within '.mr-version-dropdown' do - find('.btn-link').click + find('.btn-default').click click_link 'version 1' end end @@ -37,17 +37,18 @@ feature 'Merge Request versions', js: true, feature: true do it 'show the message about disabled comments' do expect(page).to have_content 'Comments are disabled' end + end describe 'compare with older version' do before do page.within '.mr-version-compare-dropdown' do - find('.btn-link').click + find('.btn-default').click click_link 'version 1' end end - it 'should has correct value in the compare dropdown' do + it 'should have correct value in the compare dropdown' do page.within '.mr-version-compare-dropdown' do expect(page).to have_content 'version 1' end @@ -64,5 +65,13 @@ feature 'Merge Request versions', js: true, feature: true do it 'show diff between new and old version' do expect(page).to have_content '4 changed files with 15 additions and 6 deletions' end + + it 'should return to latest version when "Show latest version" button is clicked' do + click_link 'Show latest version' + page.within '.mr-version-dropdown' do + expect(page).to have_content 'latest version' + end + expect(page).to have_content '8 changed files' + end end end -- GitLab