Skip to content

G
gitlab-ce
Commit efa0146c authored by Robert Speicher's avatar Robert Speicher
Browse files
Options

Merge branch 'rs-blobview-js-refactor' into 'master' 

Refactor and spec line highlighting Javascript

- Replaces the fragile Spinach tests for this feature with Jasmine tests
- The "Don't scroll the page when we click a line number" behavior now works as expected

See merge request !820
parents e13b523c 7f5b255c
Hide whitespace changes
Inline Side-by-side
Showing with 311 additions and 220 deletions
app/assets/javascripts/blob/blob.js.coffee deleted 100644 → 0
View file @ e13b523c
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)
app/assets/javascripts/dispatcher.js.coffee
View file @ efa0146c
......@@ -87,7 +87,7 @@ class Dispatcher
new TreeView()
shortcut_handler = new ShortcutsNavigation()
when 'projects:blob:show'
new BlobView()
new LineHighlighter()
shortcut_handler = new ShortcutsNavigation()
when 'projects:labels:new', 'projects:labels:edit'
new Labels()
......
app/assets/javascripts/line_highlighter.js.coffee 0 → 100644
View file @ efa0146c
# LineHighlighter
#
# 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 @LineHighlighter
# CSS class applied to highlighted lines
highlightClass: 'hll'
# Internal copy of location.hash so we're not dependent on `location` in tests
_hash: ''
# Initialize a LineHighlighter object
#
# hash - String URL hash for dependency injection in tests
constructor: (hash = location.hash) ->
@_hash = hash
@bindEvents()
unless hash == ''
range = @hashToRange(hash)
if 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()
@clearHighlight()
lineNumber = $(event.target).data('line-number')
current = @hashToRange(@_hash)
unless current[0] && 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)
# Unhighlight previously highlighted lines
clearHighlight: ->
$(".#{@highlightClass}").removeClass(@highlightClass)
# Convert a URL hash String into line numbers
#
# hash - Hash String
#
# Examples:
#
# hashToRange('#L5') # => [5, null]
# hashToRange('#L5-15') # => [5, 15]
# hashToRange('#foo') # => [null, null]
#
# Returns an Array
hashToRange: (hash) ->
matches = hash.match(/^#?L(\d+)(?:-(\d+))?$/)
if matches && matches.length
first = parseInt(matches[1])
last = if matches[2] then parseInt(matches[2]) else null
[first, last]
else
[null, null]
# Highlight a single line
#
# lineNumber - Line number to highlight
highlightLine: (lineNumber) =>
$("#LC#{lineNumber}").addClass(@highlightClass)
# Highlight all lines within a range
#
# range - Array containing the starting and ending line numbers
highlightRange: (range) ->
if range[1]
for lineNumber in [range[0]..range[1]]
@highlightLine(lineNumber)
else
@highlightLine(range[0])
# Set the URL hash string
setHash: (firstLineNumber, lastLineNumber) =>
if lastLineNumber
hash = "#L#{firstLineNumber}-#{lastLineNumber}"
else
hash = "#L#{firstLineNumber}"
@_hash = hash
@__setLocationHash__(hash)
# Make the actual hash change in the browser
#
# This method is stubbed in tests.
__setLocationHash__: (value) ->
# We're using pushState instead of assigning location.hash directly to
# prevent the page from scrolling on the hashchange event
history.pushState({turbolinks: false, url: value}, document.title, value)
app/views/shared/_file_highlight.html.haml
View file @ efa0146c
.file-content.code{class: user_color_scheme_class}
.line-numbers
- if blob.data.present?
- blob.data.lines.to_a.size.times do |index|
- blob.data.lines.each_index 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
......
features/project/source/multiselect_blob.feature deleted 100644 → 0
View file @ e13b523c
Feature: Project Source Multiselect Blob
Background:
Given I sign in as a user
And I own project "Shop"
And I visit ".gitignore" file in repo
@javascript
Scenario: I click line 1 in file
When I click line 1 in file
Then I should see "L1" as URI fragment
And I should see line 1 highlighted
@javascript
Scenario: I shift-click line 1 in file
When I shift-click line 1 in file
Then I should see "L1" as URI fragment
And I should see line 1 highlighted
@javascript
Scenario: I click line 1 then click line 2 in file
When I click line 1 in file
Then I should see "L1" as URI fragment
And I should see line 1 highlighted
Then I click line 2 in file
Then I should see "L2" as URI fragment
And I should see line 2 highlighted
@javascript
Scenario: I click various line numbers to test multiselect
Then I click line 1 in file
Then I should see "L1" as URI fragment
And I should see line 1 highlighted
Then I shift-click line 2 in file
Then I should see "L1-2" as URI fragment
And I should see lines 1-2 highlighted
Then I shift-click line 3 in file
Then I should see "L1-3" as URI fragment
And I should see lines 1-3 highlighted
Then I click line 3 in file
Then I should see "L3" as URI fragment
And I should see line 3 highlighted
Then I shift-click line 1 in file
Then I should see "L1-3" as URI fragment
And I should see lines 1-3 highlighted
Then I shift-click line 5 in file
Then I should see "L1-5" as URI fragment
And I should see lines 1-5 highlighted
Then I shift-click line 4 in file
Then I should see "L1-4" as URI fragment
And I should see lines 1-4 highlighted
Then I click line 5 in file
Then I should see "L5" as URI fragment
And I should see line 5 highlighted
Then I shift-click line 3 in file
Then I should see "L3-5" as URI fragment
And I should see lines 3-5 highlighted
Then I shift-click line 1 in file
Then I should see "L1-3" as URI fragment
And I should see lines 1-3 highlighted
Then I shift-click line 1 in file
Then I should see "L1" as URI fragment
And I should see line 1 highlighted
@javascript
Scenario: I multiselect lines 1-5 and then go back and forward in history
When I click line 1 in file
And I shift-click line 3 in file
And I shift-click line 2 in file
And I shift-click line 5 in file
Then I should see "L1-5" as URI fragment
And I should see lines 1-5 highlighted
Then I go back in history
Then I should see "L1-2" as URI fragment
And I should see lines 1-2 highlighted
Then I go back in history
Then I should see "L1-3" as URI fragment
And I should see lines 1-3 highlighted
Then I go back in history
Then I should see "L1" as URI fragment
And I should see line 1 highlighted
Then I go forward in history
And I go forward in history
And I go forward in history
Then I should see "L1-5" as URI fragment
And I should see lines 1-5 highlighted
features/steps/project/source/multiselect_blob.rb deleted 100644 → 0
View file @ e13b523c
class Spinach::Features::ProjectSourceMultiselectBlob < Spinach::FeatureSteps
include SharedAuthentication
include SharedProject
include SharedPaths
class << self
def click_line_steps(*line_numbers)
line_numbers.each do |line_number|
step "I click line #{line_number} in file" do
find("#L#{line_number}").click
end
step "I shift-click line #{line_number} in file" do
script = "$('#L#{line_number}').trigger($.Event('click', { shiftKey: true }));"
execute_script(script)
end
end
end
def check_state_steps(*ranges)
ranges.each do |range|
fragment = range.kind_of?(Array) ? "L#{range.first}-#{range.last}" : "L#{range}"
pluralization = range.kind_of?(Array) ? "s" : ""
step "I should see \"#{fragment}\" as URI fragment" do
expect(URI.parse(current_url).fragment).to eq fragment
end
step "I should see line#{pluralization} #{fragment[1..-1]} highlighted" do
ids = Array(range).map { |n| "LC#{n}" }
extra = false
highlighted = page.all("#tree-content-holder .highlight .line.hll")
highlighted.each do |element|
extra ||= ids.delete(element[:id]).nil?
end
expect(extra).to be_false and ids.should be_empty
end
end
end
end
click_line_steps *Array(1..5)
check_state_steps *Array(1..5), Array(1..2), Array(1..3), Array(1..4), Array(1..5), Array(3..5)
step 'I go back in history' do
go_back
end
step 'I go forward in history' do
go_forward
end
step 'I click on ".gitignore" file in repo' do
click_link ".gitignore"
end
end
spec/javascripts/fixtures/line_highlighter.html.haml 0 → 100644
View file @ efa0146c
#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}"
spec/javascripts/line_highlighter_spec.js.coffee 0 → 100644
View file @ efa0146c
#= require line_highlighter
describe 'LineHighlighter', ->
fixture.preload('line_highlighter.html')
clickLine = (number, eventData = {}) ->
if $.isEmptyObject(eventData)
$("#L#{number}").mousedown().click()
else
e = $.Event 'mousedown', eventData
$("#L#{number}").trigger(e).click()
beforeEach ->
fixture.load('line_highlighter.html')
@class = new LineHighlighter()
@css = @class.highlightClass
@spies = {
__setLocationHash__: spyOn(@class, '__setLocationHash__').and.callFake ->
}
describe 'behavior', ->
it 'highlights one line given in the URL hash', ->
new LineHighlighter('#L13')
expect($('#LC13')).toHaveClass(@css)
it 'highlights a range of lines given in the URL hash', ->
new LineHighlighter('#L5-25')
expect($(".#{@css}").length).toBe(21)
expect($("#LC#{line}")).toHaveClass(@css) for line in [5..25]
it 'scrolls to the first highlighted line on initial load', ->
spy = spyOn($, 'scrollTo')
new LineHighlighter('#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 LineHighlighter('#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(@css)
it 'unhighlights previously highlighted lines', ->
clickLine(13)
clickLine(20)
expect($('#LC13')).not.toHaveClass(@css)
expect($('#LC20')).toHaveClass(@css)
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(@css)
expect($(".#{@css}").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($(".#{@css}").length).toBe(6)
expect($("#LC#{line}")).toHaveClass(@css) for line in [15..20]
it 'uses existing line as first line when target is greater', ->
clickLine(5)
clickLine(10, shiftKey: true)
expect($(".#{@css}").length).toBe(6)
expect($("#LC#{line}")).toHaveClass(@css) 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($(".#{@css}").length).toBe(6)
expect($("#LC#{line}")).toHaveClass(@css) for line in [5..10]
it 'uses target as last line when it is greater than existing first line', ->
clickLine(15, shiftKey: true)
expect($(".#{@css}").length).toBe(6)
expect($("#LC#{line}")).toHaveClass(@css) 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, null])
it 'extracts a range of line numbers from the hash', ->
expect(@subject('#L5-15')).toEqual([5, 15])
it 'returns [null, null] when the hash is not a line number', ->
expect(@subject('#foo')).toEqual([null, null])
describe '#highlightLine', ->
beforeEach ->
@subject = @class.highlightLine
it 'highlights the specified line', ->
@subject(13)
expect($('#LC13')).toHaveClass(@css)
it 'accepts a String-based number', ->
@subject('13')
expect($('#LC13')).toHaveClass(@css)
describe '#setHash', ->
beforeEach ->
@subject = @class.setHash
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')
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7