summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlfredo Sumaran <alfredo@gitlab.com>2017-03-13 19:35:18 +0000
committerAlfredo Sumaran <alfredo@gitlab.com>2017-03-13 19:35:18 +0000
commit08f7e49d2980dcb23902cec3324a61dd8ba9c0e9 (patch)
treea7f023e704242bd944bc7d463562c5e77a4aa549
parentcc4ca1f3fe2e2f7470c4fae9a9c5676bbebb18b6 (diff)
parentc0242485393fe93397ee18889bc5345b67d5ea0d (diff)
downloadgitlab-ce-08f7e49d2980dcb23902cec3324a61dd8ba9c0e9.tar.gz
Merge branch '19742-permalink-blame-button-line-number-hash-links' into 'master'
Update permalink/blame buttons with line number fragment hash Closes #19742 See merge request !9461
-rw-r--r--app/assets/javascripts/blob/blob_line_permalink_updater.js35
-rw-r--r--app/assets/javascripts/dispatcher.js8
-rw-r--r--app/assets/javascripts/line_highlighter.js12
-rw-r--r--app/assets/stylesheets/framework/highlight.scss9
-rw-r--r--app/views/projects/blob/_actions.html.haml2
-rw-r--r--changelogs/unreleased/19742-permalink-blame-button-line-number-hash-links.yml4
-rw-r--r--spec/features/projects/blobs/blob_line_permalink_updater_spec.rb97
-rw-r--r--spec/javascripts/line_highlighter_spec.js18
8 files changed, 157 insertions, 28 deletions
diff --git a/app/assets/javascripts/blob/blob_line_permalink_updater.js b/app/assets/javascripts/blob/blob_line_permalink_updater.js
new file mode 100644
index 00000000000..c8f68860fbd
--- /dev/null
+++ b/app/assets/javascripts/blob/blob_line_permalink_updater.js
@@ -0,0 +1,35 @@
+const lineNumberRe = /^L[0-9]+/;
+
+const updateLineNumbersOnBlobPermalinks = (linksToUpdate) => {
+ const hash = gl.utils.getLocationHash();
+ if (hash && lineNumberRe.test(hash)) {
+ const hashUrlString = `#${hash}`;
+
+ [].concat(Array.prototype.slice.call(linksToUpdate)).forEach((permalinkButton) => {
+ const baseHref = permalinkButton.getAttribute('data-original-href') || (() => {
+ const href = permalinkButton.getAttribute('href');
+ permalinkButton.setAttribute('data-original-href', href);
+ return href;
+ })();
+ permalinkButton.setAttribute('href', `${baseHref}${hashUrlString}`);
+ });
+ }
+};
+
+function BlobLinePermalinkUpdater(blobContentHolder, lineNumberSelector, elementsToUpdate) {
+ const updateBlameAndBlobPermalinkCb = () => {
+ // Wait for the hash to update from the LineHighlighter callback
+ setTimeout(() => {
+ updateLineNumbersOnBlobPermalinks(elementsToUpdate);
+ }, 0);
+ };
+
+ blobContentHolder.addEventListener('click', (e) => {
+ if (e.target.matches(lineNumberSelector)) {
+ updateBlameAndBlobPermalinkCb();
+ }
+ });
+ updateBlameAndBlobPermalinkCb();
+}
+
+export default BlobLinePermalinkUpdater;
diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js
index 5739a28699f..f0967d4f470 100644
--- a/app/assets/javascripts/dispatcher.js
+++ b/app/assets/javascripts/dispatcher.js
@@ -40,6 +40,7 @@ import BindInOut from './behaviors/bind_in_out';
import GroupsList from './groups_list';
import ProjectsList from './projects_list';
import MiniPipelineGraph from './mini_pipeline_graph_dropdown';
+import BlobLinePermalinkUpdater from './blob/blob_line_permalink_updater';
const ShortcutsBlob = require('./shortcuts_blob');
const UserCallout = require('./user_callout');
@@ -272,6 +273,13 @@ const UserCallout = require('./user_callout');
break;
case 'projects:blame:show':
new LineHighlighter();
+
+ new BlobLinePermalinkUpdater(
+ document.querySelector('#blob-content-holder'),
+ '.diff-line-num[data-line-number]',
+ document.querySelectorAll('.js-data-file-blob-permalink-url, .js-blob-blame-link'),
+ );
+
shortcut_handler = new ShortcutsNavigation();
fileBlobPermalinkUrlElement = document.querySelector('.js-data-file-blob-permalink-url');
fileBlobPermalinkUrl = fileBlobPermalinkUrlElement && fileBlobPermalinkUrlElement.getAttribute('href');
diff --git a/app/assets/javascripts/line_highlighter.js b/app/assets/javascripts/line_highlighter.js
index 966fcd8ec47..1821ca18053 100644
--- a/app/assets/javascripts/line_highlighter.js
+++ b/app/assets/javascripts/line_highlighter.js
@@ -67,17 +67,7 @@ require('vendor/jquery.scrollTo');
}
LineHighlighter.prototype.bindEvents = function() {
- $('#blob-content-holder').on('mousedown', 'a[data-line-number]', this.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.
- $('#blob-content-holder').on('click', 'a[data-line-number]', function(event) {
- event.preventDefault();
- event.stopPropagation();
- });
+ $('#blob-content-holder').on('click', 'a[data-line-number]', this.clickHandler);
};
LineHighlighter.prototype.clickHandler = function(event) {
diff --git a/app/assets/stylesheets/framework/highlight.scss b/app/assets/stylesheets/framework/highlight.scss
index 909a0f4afda..6d27d7568cf 100644
--- a/app/assets/stylesheets/framework/highlight.scss
+++ b/app/assets/stylesheets/framework/highlight.scss
@@ -57,8 +57,13 @@
visibility: hidden;
}
- &:hover i {
- visibility: visible;
+ &:hover,
+ &:focus {
+ outline: none;
+
+ & i {
+ visibility: visible;
+ }
}
}
}
diff --git a/app/views/projects/blob/_actions.html.haml b/app/views/projects/blob/_actions.html.haml
index c44d8fcd430..14d42f7d9ec 100644
--- a/app/views/projects/blob/_actions.html.haml
+++ b/app/views/projects/blob/_actions.html.haml
@@ -12,7 +12,7 @@
class: 'btn btn-sm'
- else
= link_to 'Blame', namespace_project_blame_path(@project.namespace, @project, @id),
- class: 'btn btn-sm' unless @blob.empty?
+ class: 'btn btn-sm js-blob-blame-link' unless @blob.empty?
= link_to 'History', namespace_project_commits_path(@project.namespace, @project, @id),
class: 'btn btn-sm'
= link_to 'Permalink', namespace_project_blob_path(@project.namespace, @project,
diff --git a/changelogs/unreleased/19742-permalink-blame-button-line-number-hash-links.yml b/changelogs/unreleased/19742-permalink-blame-button-line-number-hash-links.yml
new file mode 100644
index 00000000000..199f1edec8b
--- /dev/null
+++ b/changelogs/unreleased/19742-permalink-blame-button-line-number-hash-links.yml
@@ -0,0 +1,4 @@
+---
+title: Update permalink/blame buttons with line number fragment hash
+merge_request:
+author:
diff --git a/spec/features/projects/blobs/blob_line_permalink_updater_spec.rb b/spec/features/projects/blobs/blob_line_permalink_updater_spec.rb
new file mode 100644
index 00000000000..d94204230f6
--- /dev/null
+++ b/spec/features/projects/blobs/blob_line_permalink_updater_spec.rb
@@ -0,0 +1,97 @@
+require 'spec_helper'
+
+feature 'Blob button line permalinks (BlobLinePermalinkUpdater)', feature: true, js: true do
+ include TreeHelper
+
+ let(:project) { create(:project, :public, :repository) }
+ let(:path) { 'CHANGELOG' }
+ let(:sha) { project.repository.commit.sha }
+
+ describe 'On a file(blob)' do
+ def get_absolute_url(path = "")
+ "http://#{page.server.host}:#{page.server.port}#{path}"
+ end
+
+ def visit_blob(fragment = nil)
+ visit namespace_project_blob_path(project.namespace, project, tree_join('master', path), anchor: fragment)
+ end
+
+ describe 'Click "Permalink" button' do
+ it 'works with no initial line number fragment hash' do
+ visit_blob
+
+ expect(find('.js-data-file-blob-permalink-url')['href']).to eq(get_absolute_url(namespace_project_blob_path(project.namespace, project, tree_join(sha, path))))
+ end
+
+ it 'maintains intitial fragment hash' do
+ fragment = "L3"
+
+ visit_blob(fragment)
+
+ expect(find('.js-data-file-blob-permalink-url')['href']).to eq(get_absolute_url(namespace_project_blob_path(project.namespace, project, tree_join(sha, path), anchor: fragment)))
+ end
+
+ it 'changes fragment hash if line number clicked' do
+ ending_fragment = "L5"
+
+ visit_blob
+
+ find('#L3').click
+ find("##{ending_fragment}").click
+
+ expect(find('.js-data-file-blob-permalink-url')['href']).to eq(get_absolute_url(namespace_project_blob_path(project.namespace, project, tree_join(sha, path), anchor: ending_fragment)))
+ end
+
+ it 'with initial fragment hash, changes fragment hash if line number clicked' do
+ fragment = "L1"
+ ending_fragment = "L5"
+
+ visit_blob(fragment)
+
+ find('#L3').click
+ find("##{ending_fragment}").click
+
+ expect(find('.js-data-file-blob-permalink-url')['href']).to eq(get_absolute_url(namespace_project_blob_path(project.namespace, project, tree_join(sha, path), anchor: ending_fragment)))
+ end
+ end
+
+ describe 'Click "Blame" button' do
+ it 'works with no initial line number fragment hash' do
+ visit_blob
+
+ expect(find('.js-blob-blame-link')['href']).to eq(get_absolute_url(namespace_project_blame_path(project.namespace, project, tree_join('master', path))))
+ end
+
+ it 'maintains intitial fragment hash' do
+ fragment = "L3"
+
+ visit_blob(fragment)
+
+ expect(find('.js-blob-blame-link')['href']).to eq(get_absolute_url(namespace_project_blame_path(project.namespace, project, tree_join('master', path), anchor: fragment)))
+ end
+
+ it 'changes fragment hash if line number clicked' do
+ ending_fragment = "L5"
+
+ visit_blob
+
+ find('#L3').click
+ find("##{ending_fragment}").click
+
+ expect(find('.js-blob-blame-link')['href']).to eq(get_absolute_url(namespace_project_blame_path(project.namespace, project, tree_join('master', path), anchor: ending_fragment)))
+ end
+
+ it 'with initial fragment hash, changes fragment hash if line number clicked' do
+ fragment = "L1"
+ ending_fragment = "L5"
+
+ visit_blob(fragment)
+
+ find('#L3').click
+ find("##{ending_fragment}").click
+
+ expect(find('.js-blob-blame-link')['href']).to eq(get_absolute_url(namespace_project_blame_path(project.namespace, project, tree_join('master', path), anchor: ending_fragment)))
+ end
+ end
+ end
+end
diff --git a/spec/javascripts/line_highlighter_spec.js b/spec/javascripts/line_highlighter_spec.js
index a0b2ebc221b..a1fd2d38968 100644
--- a/spec/javascripts/line_highlighter_spec.js
+++ b/spec/javascripts/line_highlighter_spec.js
@@ -7,16 +7,12 @@ require('~/line_highlighter');
describe('LineHighlighter', function() {
var clickLine;
preloadFixtures('static/line_highlighter.html.raw');
- clickLine = function(number, eventData) {
- var e;
- if (eventData == null) {
- eventData = {};
- }
+ clickLine = function(number, eventData = {}) {
if ($.isEmptyObject(eventData)) {
- return $("#L" + number).mousedown().click();
+ return $("#L" + number).click();
} else {
- e = $.Event('mousedown', eventData);
- return $("#L" + number).trigger(e).click();
+ const e = $.Event('click', eventData);
+ return $("#L" + number).trigger(e);
}
};
beforeEach(function() {
@@ -63,12 +59,6 @@ require('~/line_highlighter');
});
});
describe('#clickHandler', function() {
- it('discards the mousedown event', function() {
- var spy;
- spy = spyOnEvent('a[data-line-number]', 'mousedown');
- clickLine(13);
- return expect(spy).toHaveBeenPrevented();
- });
it('handles clicking on a child icon element', function() {
var spy;
spy = spyOn(this["class"], 'setHash').and.callThrough();