Merge branch 'rs-mr-tabs-scroll' into 'master'

Prevent a JS error in MergeRequestTabs When `window.location.hash` is pointing to a note, e.g. `#note_1234`, `scrollToElement` would throw an error because a selector such as `.commits #note_1234` doesn't exist, so `offset()` returned `undefined`. This error would prevent subsequent calls from running, which caused the loading spinner to never be hidden. Now we ensure the selector returns a valid element before trying to scroll to it. This is the proper fix for !1553. See merge request !1597

Merge branch 'rs-mr-tabs-scroll' into 'master'
7ad68523 · Douwe Maan · 976400c1 · fc0d9274 · 7ad68523
Commit 7ad68523 authored Oct 15, 2015 by Douwe Maan
Hide whitespace changes
Inline Side-by-side

Showing with 4 additions and 4 deletions

merge_request_tabs.js.coffee app/assets/javascripts/merge_request_tabs.js.coffee +4 -4

No files found.
--- a/app/assets/javascripts/merge_request_tabs.js.coffee
+++ b/app/assets/javascripts/merge_request_tabs.js.coffee
@@ -68,8 +68,8 @@ class @MergeRequestTabs

  scrollToElement: (container) ->
    if window.location.hash
-      top = $(container + " " + window.location.hash).offset().top
-      $('body').scrollTo(top)
+      $el = $("#{container} #{window.location.hash}")
+      $('body').scrollTo($el.offset().top) if $el.length

  # Activate a tab based on the current action
  activateTab: (action) ->
@@ -127,7 +127,7 @@ class @MergeRequestTabs
        document.getElementById('commits').innerHTML = data.html
        $('.js-timeago').timeago()
        @commitsLoaded = true
-        @scrollToElement(".commits")
+        @scrollToElement("#commits")

  loadDiff: (source) ->
    return if @diffsLoaded
@@ -137,7 +137,7 @@ class @MergeRequestTabs
      success: (data) =>
        document.getElementById('diffs').innerHTML = data.html
        @diffsLoaded = true
-        @scrollToElement(".diffs")
+        @scrollToElement("#diffs")

  # Show or hide the loading spinner
  #