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.

Prevent a JS error in MergeRequestTabs
fc0d9274 · Robert Speicher · 2fb02f92 · fc0d9274
Commit fc0d9274 authored Oct 14, 2015 by Robert Speicher
Show 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
+      $el = $("#{container} #{window.location.hash}")
-      $('body').scrollTo(top)
+      $('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
  #