diff --git a/app/assets/javascripts/merge_request.js.coffee b/app/assets/javascripts/merge_request.js.coffee index 7c1e2b822d71a69918e1a50be989daae93556b52..3937c428e24931e46a9397686b7e25d2d637ba35 100644 --- a/app/assets/javascripts/merge_request.js.coffee +++ b/app/assets/javascripts/merge_request.js.coffee @@ -3,15 +3,30 @@ #= require task_list class @MergeRequest + # Initialize MergeRequest behavior + # + # Options: + # action - String, current controller action + # diffs_loaded - Boolean, have diffs been pre-rendered server-side? + # (default: true if `action` is 'diffs', otherwise false) + # commits_loaded - Boolean, have commits been pre-rendered server-side? + # (default: false) + # + # check_enable - Boolean, whether to check automerge status + # url_to_automerge_check - String, URL to use to check automerge status + # current_status - String, current automerge status + # ci_enable - Boolean, whether a CI service is enabled + # url_to_ci_check - String, URL to use to check CI status + # constructor: (@opts) -> @initContextWidget() this.$el = $('.merge-request') - @diffs_loaded = if @opts.action == 'diffs' then true else false - @commits_loaded = false - this.activateTab(@opts.action) + @diffs_loaded = @opts.diffs_loaded or @opts.action == 'diffs' + @commits_loaded = @opts.commits_loaded or false this.bindEvents() + this.activateTabFromHash() this.initMergeWidget() this.$('.show-all-commits').on 'click', => @@ -65,8 +80,21 @@ class @MergeRequest , 'json' bindEvents: -> - this.$('.merge-request-tabs').on 'click', 'li', (event) => - this.activateTab($(event.currentTarget).data('action')) + this.$('.merge-request-tabs a[data-toggle="tab"]').on 'shown.bs.tab', (e) => + $target = $(e.target) + + # Nothing else to be done if we're on the first tab + return if $target.data('action') == 'notes' + + # Persist current tab selection via URL + href = $target.attr('href') + if href.substr(0,1) == '#' + location.replace("#!#{href.substr(1)}") + + # Lazy-load diffs + if $target.data('action') == 'diffs' + this.loadDiff() unless @diffs_loaded + $('.diff-header').trigger("sticky_kit:recalc") this.$('.accept_merge_request').on 'click', -> $('.automerge_widget.can_be_merged').hide() @@ -84,21 +112,27 @@ class @MergeRequest this.$('.remove_source_branch_in_progress').hide() this.$('.remove_source_branch_widget.failed').show() - activateTab: (action) -> - this.$('.merge-request-tabs li').removeClass 'active' - this.$('.tab-content').hide() - switch action - when 'diffs' - this.$('.merge-request-tabs .diffs-tab').addClass 'active' - this.loadDiff() unless @diffs_loaded - this.$('.diffs').show() - $(".diff-header").trigger("sticky_kit:recalc") - when 'commits' - this.$('.merge-request-tabs .commits-tab').addClass 'active' - this.$('.commits').show() - else - this.$('.merge-request-tabs .notes-tab').addClass 'active' - this.$('.notes').show() + # Activates a tab section based on the `#!` URL hash + # + # If no hash value is present (i.e., on the initial page load), the first tab + # is selected by default. + # + # ... unless the current controller action is `diffs`, in which case that tab + # is selected instead. Fun, right? + # + # Note: We use a `#!` instead of a standard URL hash for two reasons: + # + # 1. Prevents the hash acting like an anchor and scrolling the page. + # 2. Prevents mutating browser history. + activateTabFromHash: -> + # Correct the hash if we came here directly via the `/diffs` path + if location.hash == '' and @opts.action == 'diffs' + location.replace('#!diffs') + + if location.hash == '' + this.$('.merge-request-tabs a[data-toggle="tab"]:first').tab('show') + else if location.hash.substr(0,2) == '#!' + this.$(".merge-request-tabs a[href='##{location.hash.substr(2)}']").tab("show") showState: (state) -> $('.automerge_widget').hide() @@ -127,7 +161,7 @@ class @MergeRequest loadDiff: (event) -> $.ajax type: 'GET' - url: this.$('.merge-request-tabs .diffs-tab a').attr('href') + ".json" + url: this.$('.merge-request-tabs .diffs-tab a').data('source') + ".json" beforeSend: => this.$('.mr-loading-status .loading').show() complete: => diff --git a/app/views/projects/merge_requests/_new_submit.html.haml b/app/views/projects/merge_requests/_new_submit.html.haml index 24a9563dd4ddf6e9b0d5dd70164d9f0549dd949f..e83b76499284561969beb433a369f5315cb5e810 100644 --- a/app/views/projects/merge_requests/_new_submit.html.haml +++ b/app/views/projects/merge_requests/_new_submit.html.haml @@ -19,30 +19,31 @@ .mr-compare.merge-request %ul.nav.nav-tabs.merge-request-tabs - %li.commits-tab{data: {action: 'commits', toggle: 'tab'}} - = link_to url_for(params) do - %i.fa.fa-history + %li.commits-tab + = link_to '#commits', data: {action: 'commits', toggle: 'tab'} do + = icon('history') Commits %span.badge= @commits.size - %li.diffs-tab{data: {action: 'diffs', toggle: 'tab'}} - = link_to url_for(params) do - %i.fa.fa-list-alt + %li.diffs-tab + = link_to '#diffs', data: {action: 'diffs', toggle: 'tab'} do + = icon('list-alt') Changes %span.badge= @diffs.size - .commits.tab-content - = render "projects/commits/commits", project: @project - .diffs.tab-content - - if @diffs.present? - = render "projects/diffs/diffs", diffs: @diffs, project: @project - - elsif @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE - .alert.alert-danger - %h4 This comparison includes more than #{MergeRequestDiff::COMMITS_SAFE_SIZE} commits. - %p To preserve performance the line changes are not shown. - - else - .alert.alert-danger - %h4 This comparison includes a huge diff. - %p To preserve performance the line changes are not shown. + .tab-content + #commits.commits.tab-pane + = render "projects/commits/commits", project: @project + #diffs.diffs.tab-pane + - if @diffs.present? + = render "projects/diffs/diffs", diffs: @diffs, project: @project + - elsif @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE + .alert.alert-danger + %h4 This comparison includes more than #{MergeRequestDiff::COMMITS_SAFE_SIZE} commits. + %p To preserve performance the line changes are not shown. + - else + .alert.alert-danger + %h4 This comparison includes a huge diff. + %p To preserve performance the line changes are not shown. :javascript $('.assign-to-me-link').on('click', function(e){ @@ -55,6 +56,8 @@ :javascript var merge_request merge_request = new MergeRequest({ - action: 'commits' + action: 'diffs', + diffs_loaded: true, + commits_loaded: true }); diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index c2f5cdacae74d4ecc33f98c85b9b0f2a530cede1..0d894e360ea7ee8df361679a4db6e84ddc276a1e 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -25,7 +25,7 @@ %span.pull-right .btn-group %a.btn.dropdown-toggle{ data: {toggle: :dropdown} } - %i.fa.fa-download + = icon('download') Download as %span.caret %ul.dropdown-menu @@ -37,29 +37,30 @@ - if @commits.present? %ul.nav.nav-tabs.merge-request-tabs - %li.notes-tab{data: {action: 'notes', toggle: 'tab'}} - = link_to merge_request_path(@merge_request) do - %i.fa.fa-comments + %li.notes-tab + = link_to '#notes', data: {action: 'notes', toggle: 'tab'} do + = icon('comments') Discussion %span.badge= @merge_request.mr_and_commit_notes.user.count - %li.commits-tab{data: {action: 'commits', toggle: 'tab'}} - = link_to merge_request_path(@merge_request), title: 'Commits' do - %i.fa.fa-history + %li.commits-tab + = link_to '#commits', data: {action: 'commits', toggle: 'tab'} do + = icon('history') Commits %span.badge= @commits.size - %li.diffs-tab{data: {action: 'diffs', toggle: 'tab'}} - = link_to diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request) do - %i.fa.fa-list-alt + %li.diffs-tab + = link_to '#diffs', data: {source: diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), action: 'diffs', toggle: 'tab'} do + = icon('list-alt') Changes %span.badge= @merge_request.diffs.size - .notes.tab-content.voting_notes#notes{ class: (controller.action_name == 'show') ? "" : "hide" } - = render "projects/merge_requests/discussion" - .commits.tab-content - = render "projects/merge_requests/show/commits" - .diffs.tab-content - - if current_page?(action: 'diffs') - = render "projects/merge_requests/show/diffs" + .tab-content + #notes.notes.tab-pane.voting_notes + = render "projects/merge_requests/discussion" + #commits.commits.tab-pane + = render "projects/merge_requests/show/commits" + #diffs.diffs.tab-pane + - if current_page?(action: 'diffs') + = render "projects/merge_requests/show/diffs" .mr-loading-status = spinner diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index 92de94a75d35a9bd087a5ce93385bc3ae560421d..48bb316e20352b9ac3ca2b4f95d7e63f9058a270 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -113,7 +113,10 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps end step 'I click on the Changes tab via Javascript' do - find('.diffs-tab').click + within '.merge-request-tabs' do + click_link 'Changes' + end + sleep 2 end