diff options
author | Robert Speicher <rspeicher@gmail.com> | 2015-06-06 23:15:32 -0400 |
---|---|---|
committer | Robert Speicher <rspeicher@gmail.com> | 2015-06-19 04:31:23 -0400 |
commit | b3f9be06398e8872cc64a966f99866b67e18c337 (patch) | |
tree | 07a88eb847b26d16b5e2301962d36e4bfb26772b | |
parent | 74a6732c0dfc1416cad382203544d9c4d6246f58 (diff) | |
download | gitlab-ce-b3f9be06398e8872cc64a966f99866b67e18c337.tar.gz |
Refactor and spec BlobView JS
-rw-r--r-- | app/assets/javascripts/blob/blob.js.coffee | 257 | ||||
-rw-r--r-- | app/views/shared/_file_highlight.html.haml | 4 | ||||
-rw-r--r-- | spec/javascripts/blob/blob_spec.js.coffee | 169 | ||||
-rw-r--r-- | spec/javascripts/fixtures/blob.html.haml | 9 |
4 files changed, 365 insertions, 74 deletions
diff --git a/app/assets/javascripts/blob/blob.js.coffee b/app/assets/javascripts/blob/blob.js.coffee index 37a175fdbc7..b7caae23f31 100644 --- a/app/assets/javascripts/blob/blob.js.coffee +++ b/app/assets/javascripts/blob/blob.js.coffee @@ -1,73 +1,186 @@ +# BlobView +# +# Handles single- and multi-line selection and highlight for blob views. +# +#= require jquery.scrollTo +# +# ### Example Markup +# +# <div id="tree-content-holder"> +# <div class="file-content"> +# <div class="line-numbers"> +# <a href="#L1" id="L1" data-line-number="1">1</a> +# <a href="#L2" id="L2" data-line-number="2">2</a> +# <a href="#L3" id="L3" data-line-number="3">3</a> +# <a href="#L4" id="L4" data-line-number="4">4</a> +# <a href="#L5" id="L5" data-line-number="5">5</a> +# </div> +# <pre class="code highlight"> +# <code> +# <span id="LC1" class="line">...</span> +# <span id="LC2" class="line">...</span> +# <span id="LC3" class="line">...</span> +# <span id="LC4" class="line">...</span> +# <span id="LC5" class="line">...</span> +# </code> +# </pre> +# </div> +# </div> class @BlobView - constructor: -> - # handle multi-line select - handleMultiSelect = (e) -> - [ first_line, last_line ] = parseSelectedLines() - [ line_number ] = parseSelectedLines($(this).attr("id")) - hash = "L#{line_number}" - - if e.shiftKey and not isNaN(first_line) and not isNaN(line_number) - if line_number < first_line - last_line = first_line - first_line = line_number - else - last_line = line_number - - hash = if first_line == last_line then "L#{first_line}" else "L#{first_line}-#{last_line}" - - setHash(hash) - e.preventDefault() - - # See if there are lines selected - # "#L12" and "#L34-56" supported - highlightBlobLines = (e) -> - [ first_line, last_line ] = parseSelectedLines() - - unless isNaN first_line - $("#tree-content-holder .highlight .line").removeClass("hll") - $("#LC#{line}").addClass("hll") for line in [first_line..last_line] - $.scrollTo("#L#{first_line}", offset: -50) unless e? - - # parse selected lines from hash - # always return first and last line (initialized to NaN) - parseSelectedLines = (str) -> - first_line = NaN - last_line = NaN - hash = str || window.location.hash - - if hash isnt "" - matches = hash.match(/\#?L(\d+)(\-(\d+))?/) - first_line = parseInt(matches?[1]) - last_line = parseInt(matches?[3]) - last_line = first_line if isNaN(last_line) - - [ first_line, last_line ] - - setHash = (hash) -> - hash = hash.replace(/^\#/, "") - nodes = $("#" + hash) - # if any nodes are using this id, they must be temporarily changed - # also, add a temporary div at the top of the screen to prevent scrolling - if nodes.length > 0 - scroll_top = $(document).scrollTop() - nodes.attr("id", "") - tmp = $("<div></div>") - .css({ position: "absolute", visibility: "hidden", top: scroll_top + "px" }) - .attr("id", hash) - .appendTo(document.body) - - window.location.hash = hash - - # restore the nodes - if nodes.length > 0 - tmp.remove() - nodes.attr("id", hash) - - # initialize multi-line select - $("#tree-content-holder .line-numbers a[id^=L]").on("click", handleMultiSelect) - - # Highlight the correct lines on load - highlightBlobLines() - - # Highlight the correct lines when the hash part of the URL changes - $(window).on("hashchange", highlightBlobLines) + # Internal copy of location.hash so we're not dependent on `location` in tests + @_hash = '' + + # Initialize a BlobView object + # + # hash - String URL hash for dependency injection in tests + constructor: (hash = location.hash) -> + @_hash = hash + + @bindEvents() + + unless hash == '' + range = @hashToRange(hash) + + unless isNaN(range[0]) + @highlightRange(range) + + # Scroll to the first highlighted line on initial load + # Offset -50 for the sticky top bar, and another -100 for some context + $.scrollTo("#L#{range[0]}", offset: -150) + + bindEvents: -> + $('#tree-content-holder').on 'mousedown', 'a[data-line-number]', @clickHandler + + # While it may seem odd to bind to the mousedown event and then throw away + # the click event, there is a method to our madness. + # + # If not done this way, the line number anchor will sometimes keep its + # active state even when the event is cancelled, resulting in an ugly border + # around the link and/or a persisted underline text decoration. + + $('#tree-content-holder').on 'click', 'a[data-line-number]', (event) -> + event.preventDefault() + + clickHandler: (event) => + event.preventDefault() + + lineNumber = $(event.target).data('line-number') + current = @hashToRange(@_hash) + + # Unhighlight previously highlighted lines + $('.hll').removeClass('hll') + + if isNaN(current[0]) or !event.shiftKey + # If there's no current selection, or there is but Shift wasn't held, + # treat this like a single-line selection. + @setHash(lineNumber) + @highlightLine(lineNumber) + else if event.shiftKey + if lineNumber < current[0] + range = [lineNumber, current[0]] + else + range = [current[0], lineNumber] + + @setHash(range[0], range[1]) + @highlightRange(range) + + # Convert a URL hash String into line numbers + # + # hash - Hash String + # + # Examples: + # + # hashToRange('#L5') # => [5, NaN] + # hashToRange('#L5-15') # => [5, 15] + # hashToRange('#foo') # => [NaN, NaN] + # + # Returns an Array + hashToRange: (hash) -> + first = parseInt(hash.replace(/^#L(\d+)/, '$1')) + last = parseInt(hash.replace(/^#L\d+-(\d+)/, '$1')) + + [first, last] + + # Highlight a single line + # + # lineNumber - Number to highlight. Must be parsable as an Integer. + # + # Returns undefined if lineNumber is not parsable as an Integer. + highlightLine: (lineNumber) -> + return if isNaN(parseInt(lineNumber)) + + $("#LC#{lineNumber}").addClass('hll') + + # Highlight all lines within a range + # + # range - An Array of starting and ending line numbers. + # + # Examples: + # + # # Highlight lines 5 through 15 + # highlightRange([5, 15]) + # + # # The first value is required, and must be a number + # highlightRange(['foo', 15]) # Invalid, returns undefined + # highlightRange([NaN, NaN]) # Invalid, returns undefined + # + # # The second value is optional; if omitted, only highlights the first line + # highlightRange([5, NaN]) # Valid + # + # Returns undefined if the first line is NaN. + highlightRange: (range) -> + return if isNaN(range[0]) + + if isNaN(range[1]) + @highlightLine(range[0]) + else + for lineNumber in [range[0]..range[1]] + @highlightLine(lineNumber) + + setHash: (firstLineNumber, lastLineNumber) => + return if isNaN(parseInt(firstLineNumber)) + + if isNaN(parseInt(lastLineNumber)) + hash = "#L#{firstLineNumber}" + else + hash = "#L#{firstLineNumber}-#{lastLineNumber}" + + @setHashWithoutScroll(hash) + + # Prevents the page from scrolling when `location.hash` is set + # + # This is accomplished by removing the `id` attribute of the matching element, + # creating a temporary div at the top of the current viewport, setting the + # hash, and then removing the div and restoring the `id` attribute. + # + # See http://stackoverflow.com/a/1489802/223897 + # + # FIXME (rspeicher): This is still super buggy for me. + setHashWithoutScroll: (hash) -> + @_hash = hash + + # Extract the first ID, in case we were given a range + firstID = hash.replace(/-\d+$/, '') + + $node = $(firstID) + $node.removeAttr('id') + + $tmp = $('<div></div>') + .css( + position: 'absolute' + top: "#{$(window).scrollTop()}px" + visibility: 'hidden' + ) + .attr('id', firstID) + .appendTo($('body')) + + @__setLocationHash__(hash) + + $tmp.remove() + $node.attr('id', firstID) + + # Make the actual `location.hash` change + # + # This method is stubbed in tests. + __setLocationHash__: (value) -> + location.hash = value diff --git a/app/views/shared/_file_highlight.html.haml b/app/views/shared/_file_highlight.html.haml index 86921f0a777..ab70f4770bc 100644 --- a/app/views/shared/_file_highlight.html.haml +++ b/app/views/shared/_file_highlight.html.haml @@ -4,8 +4,8 @@ - blob.data.lines.to_a.size.times do |index| - offset = defined?(first_line_number) ? first_line_number : 1 - i = index + offset - / We're not using `link_to` because it is too slow once we get to thousands of lines. - %a{href: "#L#{i}", id: "L#{i}", rel: "#L#{i}"} + -# We're not using `link_to` because it is too slow once we get to thousands of lines. + %a{href: "#L#{i}", id: "L#{i}", 'data-line-number' => i} %i.fa.fa-link = i :preserve diff --git a/spec/javascripts/blob/blob_spec.js.coffee b/spec/javascripts/blob/blob_spec.js.coffee new file mode 100644 index 00000000000..a6f68a53f99 --- /dev/null +++ b/spec/javascripts/blob/blob_spec.js.coffee @@ -0,0 +1,169 @@ +#= require blob/blob + +describe 'BlobView', -> + fixture.preload('blob.html') + + clickLine = (number, eventData = {}) -> + if $.isEmptyObject(eventData) + $("#L#{number}").mousedown().click() + else + e = $.Event 'mousedown', eventData + $("#L#{number}").trigger(e).click() + + beforeEach -> + fixture.load('blob.html') + @class = new BlobView() + @spies = { + __setLocationHash__: spyOn(@class, '__setLocationHash__').and.callFake -> + } + + describe 'behavior', -> + it 'highlights one line given in the URL hash', -> + new BlobView('#L13') + expect($('#LC13')).toHaveClass('hll') + + it 'highlights a range of lines given in the URL hash', -> + new BlobView('#L5-25') + expect($('.hll').length).toBe(21) + expect($("#LC#{line}")).toHaveClass('hll') for line in [5..25] + + it 'scrolls to the first highlighted line on initial load', -> + spy = spyOn($, 'scrollTo') + new BlobView('#L5-25') + expect(spy).toHaveBeenCalledWith('#L5', jasmine.anything()) + + it 'discards click events', -> + spy = spyOnEvent('a[data-line-number]', 'click') + clickLine(13) + expect(spy).toHaveBeenPrevented() + + it 'handles garbage input from the hash', -> + func = -> new BlobView('#tree-content-holder') + expect(func).not.toThrow() + + describe '#clickHandler', -> + it 'discards the mousedown event', -> + spy = spyOnEvent('a[data-line-number]', 'mousedown') + clickLine(13) + expect(spy).toHaveBeenPrevented() + + describe 'without shiftKey', -> + it 'highlights one line when clicked', -> + clickLine(13) + expect($('#LC13')).toHaveClass('hll') + + it 'unhighlights previously highlighted lines', -> + clickLine(13) + clickLine(20) + + expect($('#LC13')).not.toHaveClass('hll') + expect($('#LC20')).toHaveClass('hll') + + it 'sets the hash', -> + spy = spyOn(@class, 'setHash').and.callThrough() + clickLine(13) + expect(spy).toHaveBeenCalledWith(13) + + describe 'with shiftKey', -> + it 'sets the hash', -> + spy = spyOn(@class, 'setHash').and.callThrough() + clickLine(13) + clickLine(20, shiftKey: true) + expect(spy).toHaveBeenCalledWith(13) + expect(spy).toHaveBeenCalledWith(13, 20) + + describe 'without existing highlight', -> + it 'highlights the clicked line', -> + clickLine(13, shiftKey: true) + expect($('#LC13')).toHaveClass('hll') + expect($('.hll').length).toBe(1) + + it 'sets the hash', -> + spy = spyOn(@class, 'setHash') + clickLine(13, shiftKey: true) + expect(spy).toHaveBeenCalledWith(13) + + describe 'with existing single-line highlight', -> + it 'uses existing line as last line when target is lesser', -> + clickLine(20) + clickLine(15, shiftKey: true) + expect($('.hll').length).toBe(6) + expect($("#LC#{line}")).toHaveClass('hll') for line in [15..20] + + it 'uses existing line as first line when target is greater', -> + clickLine(5) + clickLine(10, shiftKey: true) + expect($('.hll').length).toBe(6) + expect($("#LC#{line}")).toHaveClass('hll') for line in [5..10] + + describe 'with existing multi-line highlight', -> + beforeEach -> + clickLine(10, shiftKey: true) + clickLine(13, shiftKey: true) + + it 'uses target as first line when it is less than existing first line', -> + clickLine(5, shiftKey: true) + expect($('.hll').length).toBe(6) + expect($("#LC#{line}")).toHaveClass('hll') for line in [5..10] + + it 'uses target as last line when it is greater than existing first line', -> + clickLine(15, shiftKey: true) + expect($('.hll').length).toBe(6) + expect($("#LC#{line}")).toHaveClass('hll') for line in [10..15] + + describe '#hashToRange', -> + beforeEach -> + @subject = @class.hashToRange + + it 'extracts a single line number from the hash', -> + expect(@subject('#L5')).toEqual([5, NaN]) + + it 'extracts a range of line numbers from the hash', -> + expect(@subject('#L5-15')).toEqual([5, 15]) + + it 'returns [NaN, NaN] when the hash is not a line number', -> + expect(@subject('#foo')).toEqual([NaN, NaN]) + + describe '#highlightLine', -> + beforeEach -> + @subject = @class.highlightLine + + it 'highlights the specified line', -> + @subject(13) + expect($('#LC13')).toHaveClass('hll') + + it 'accepts a String-based number', -> + @subject('13') + expect($('#LC13')).toHaveClass('hll') + + it 'returns undefined when given NaN', -> + expect(@subject(NaN)).toBe(undefined) + expect(@subject('foo')).toBe(undefined) + + describe '#highlightRange', -> + beforeEach -> + @subject = @class.highlightRange + + it 'returns undefined when first line is NaN', -> + expect(@subject([NaN, 15])).toBe(undefined) + expect(@subject(['foo', 15])).toBe(undefined) + + it 'returns undefined when given an invalid first line', -> + expect(@subject(['foo', 15])).toBe(undefined) + expect(@subject([NaN, NaN])).toBe(undefined) + expect(@subject('foo')).toBe(undefined) + + describe '#setHash', -> + beforeEach -> + @subject = @class.setHash + + it 'returns undefined when given an invalid first line', -> + expect(@subject('foo', 15)).toBe(undefined) + + it 'sets the location hash for a single line', -> + @subject(5) + expect(@spies.__setLocationHash__).toHaveBeenCalledWith('#L5') + + it 'sets the location hash for a range', -> + @subject(5, 15) + expect(@spies.__setLocationHash__).toHaveBeenCalledWith('#L5-15') diff --git a/spec/javascripts/fixtures/blob.html.haml b/spec/javascripts/fixtures/blob.html.haml new file mode 100644 index 00000000000..15ad1d8968f --- /dev/null +++ b/spec/javascripts/fixtures/blob.html.haml @@ -0,0 +1,9 @@ +#tree-content-holder + .file-content + .line-numbers + - 1.upto(25) do |i| + %a{href: "#L#{i}", id: "L#{i}", 'data-line-number' => i}= i + %pre.code.highlight + %code + - 1.upto(25) do |i| + %span.line{id: "LC#{i}"}= "Line #{i}" |