summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFatih Acet <acetfatih@gmail.com>2016-12-03 08:59:19 +0000
committerFatih Acet <acetfatih@gmail.com>2016-12-03 08:59:19 +0000
commitff23636c5f3736ad16988119c48b9b9969082b1f (patch)
treefb1110e3cfde1960b86d6be72ec1d7370eba7b16
parent668deeaee9401d67c908d8c5c46c8537b408e44f (diff)
parented5f22cb93cbabcd2bcf38e7a6b22efb03d7e212 (diff)
downloadgitlab-ce-ff23636c5f3736ad16988119c48b9b9969082b1f.tar.gz
Merge branch '23696-fix-diff-view-highlighting' into 'master'
Resolve "Highlighting lines is broken" ## What does this MR do? Add line highlighting back to diff view. This was working in the MR "changes" tab, but not on a commit page such as https://gitlab.com/winniehell/reproduction-area/commit/9101e66f5761929002956e0f2dd65d7f8643903d ~~This MR also fixes the `scrollToElement` method in `MergeRequestTabs` to account for the extra height of the tab links which are now fixed in place once they are scrolled to the top of the screen.~~ (removed in favor of !7051) This MR also refactors much of the `Diff` and `MergeRequestTabs` classes to es6 syntax in an effort to increase readability. ## Are there points in the code the reviewer needs to double check? Check out both MR "change" tabs and commit diff pages and ensure that line highlighting works and that loading a page with one of these permalink hashes correctly highlights and scrolls to the line. Ensure I didn't break anything in the transition to es6 syntax. Check the functionality of the tabs on MR pages, as well as diff page interactivity (unfolding hidden lines in diff files, adding comments to diffs, etc). I have checked these myself, but another set of eyes would be a good idea. ## Does this MR meet the acceptance criteria? - [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG.md) entry added - Tests - [x] All builds are passing - [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html) - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if it does - rebase it please) - [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) ## What are the relevant issue numbers? Closes #23696 See merge request !7090
-rw-r--r--app/assets/javascripts/diff.js73
-rw-r--r--app/assets/javascripts/diff.js.es6109
-rw-r--r--app/assets/javascripts/dispatcher.js.es610
-rw-r--r--app/assets/javascripts/merge_request.js2
-rw-r--r--app/assets/javascripts/merge_request_tabs.js443
-rw-r--r--app/assets/javascripts/merge_request_tabs.js.es6390
-rw-r--r--app/assets/javascripts/single_file_diff.js5
-rw-r--r--changelogs/unreleased/23696-fix-diff-view-highlighting.yml4
-rw-r--r--spec/javascripts/merge_request_tabs_spec.js109
9 files changed, 568 insertions, 577 deletions
diff --git a/app/assets/javascripts/diff.js b/app/assets/javascripts/diff.js
deleted file mode 100644
index 00da5f17f9f..00000000000
--- a/app/assets/javascripts/diff.js
+++ /dev/null
@@ -1,73 +0,0 @@
-/* eslint-disable func-names, space-before-function-paren, wrap-iife, no-var, max-len, one-var, camelcase, one-var-declaration-per-line, no-unused-vars, no-unused-expressions, no-sequences, object-shorthand, comma-dangle, prefer-arrow-callback, semi, radix, padded-blocks, max-len */
-(function() {
- this.Diff = (function() {
- var UNFOLD_COUNT;
-
- UNFOLD_COUNT = 20;
-
- function Diff() {
- $('.files .diff-file').singleFileDiff();
- this.filesCommentButton = $('.files .diff-file').filesCommentButton();
- if (this.diffViewType() === 'parallel') {
- $('.content-wrapper .container-fluid').removeClass('container-limited');
- }
- $(document).off('click', '.js-unfold');
- $(document).on('click', '.js-unfold', (function(_this) {
- return function(event) {
- var line_number, link, file, offset, old_line, params, prev_new_line, prev_old_line, ref, ref1, since, target, to, unfold, unfoldBottom;
- target = $(event.target);
- unfoldBottom = target.hasClass('js-unfold-bottom');
- unfold = true;
- ref = _this.lineNumbers(target.parent()), old_line = ref[0], line_number = ref[1];
- offset = line_number - old_line;
- if (unfoldBottom) {
- line_number += 1;
- since = line_number;
- to = line_number + UNFOLD_COUNT;
- } else {
- ref1 = _this.lineNumbers(target.parent().prev()), prev_old_line = ref1[0], prev_new_line = ref1[1];
- line_number -= 1;
- to = line_number;
- if (line_number - UNFOLD_COUNT > prev_new_line + 1) {
- since = line_number - UNFOLD_COUNT;
- } else {
- since = prev_new_line + 1;
- unfold = false;
- }
- }
- file = target.parents('.diff-file');
- link = file.data('blob-diff-path');
- params = {
- since: since,
- to: to,
- bottom: unfoldBottom,
- offset: offset,
- unfold: unfold,
- view: file.data('view')
- };
- return $.get(link, params, function(response) {
- return target.parent().replaceWith(response);
- });
- };
- })(this));
- }
-
- Diff.prototype.diffViewType = function() {
- return $('.inline-parallel-buttons a.active').data('view-type');
- }
-
- Diff.prototype.lineNumbers = function(line) {
- if (!line.children().length) {
- return [0, 0];
- }
-
- return line.find('.diff-line-num').map(function() {
- return parseInt($(this).data('linenumber'));
- });
- };
-
- return Diff;
-
- })();
-
-}).call(this);
diff --git a/app/assets/javascripts/diff.js.es6 b/app/assets/javascripts/diff.js.es6
new file mode 100644
index 00000000000..ecf9d1de81c
--- /dev/null
+++ b/app/assets/javascripts/diff.js.es6
@@ -0,0 +1,109 @@
+/* eslint-disable class-methods-use-this */
+
+(() => {
+ const UNFOLD_COUNT = 20;
+
+ class Diff {
+ constructor() {
+ $('.files .diff-file').singleFileDiff();
+ $('.files .diff-file').filesCommentButton();
+
+ if (this.diffViewType() === 'parallel') {
+ $('.content-wrapper .container-fluid').removeClass('container-limited');
+ }
+
+ $(document)
+ .off('click', '.js-unfold, .diff-line-num a')
+ .on('click', '.js-unfold', this.handleClickUnfold.bind(this))
+ .on('click', '.diff-line-num a', this.handleClickLineNum.bind(this));
+
+ this.highlighSelectedLine();
+ }
+
+ handleClickUnfold(e) {
+ const $target = $(e.target);
+ // current babel config relies on iterators implementation, so we cannot simply do:
+ // const [oldLineNumber, newLineNumber] = this.lineNumbers($target.parent());
+ const ref = this.lineNumbers($target.parent());
+ const oldLineNumber = ref[0];
+ const newLineNumber = ref[1];
+ const offset = newLineNumber - oldLineNumber;
+ const bottom = $target.hasClass('js-unfold-bottom');
+ let since;
+ let to;
+ let unfold = true;
+
+ if (bottom) {
+ const lineNumber = newLineNumber + 1;
+ since = lineNumber;
+ to = lineNumber + UNFOLD_COUNT;
+ } else {
+ const lineNumber = newLineNumber - 1;
+ since = lineNumber - UNFOLD_COUNT;
+ to = lineNumber;
+
+ // make sure we aren't loading more than we need
+ const prevNewLine = this.lineNumbers($target.parent().prev())[1];
+ if (since <= prevNewLine + 1) {
+ since = prevNewLine + 1;
+ unfold = false;
+ }
+ }
+
+ const file = $target.parents('.diff-file');
+ const link = file.data('blob-diff-path');
+ const view = file.data('view');
+
+ const params = { since, to, bottom, offset, unfold, view };
+ $.get(link, params, response => $target.parent().replaceWith(response));
+ }
+
+ openAnchoredDiff(anchoredDiff, cb) {
+ const diffTitle = $(`#file-path-${anchoredDiff}`);
+ const diffFile = diffTitle.closest('.diff-file');
+ const nothingHereBlock = $('.nothing-here-block:visible', diffFile);
+ if (nothingHereBlock.length) {
+ diffFile.singleFileDiff(true, cb);
+ } else {
+ cb();
+ }
+ }
+
+ handleClickLineNum(e) {
+ const hash = $(e.currentTarget).attr('href');
+ e.preventDefault();
+ if (window.history.pushState) {
+ window.history.pushState(null, null, hash);
+ } else {
+ window.location.hash = hash;
+ }
+ this.highlighSelectedLine();
+ }
+
+ diffViewType() {
+ return $('.inline-parallel-buttons a.active').data('view-type');
+ }
+
+ lineNumbers(line) {
+ if (!line.children().length) {
+ return [0, 0];
+ }
+ return line.find('.diff-line-num').map((i, elm) => parseInt($(elm).data('linenumber'), 10));
+ }
+
+ highlighSelectedLine() {
+ const $diffFiles = $('.diff-file');
+ $diffFiles.find('.hll').removeClass('hll');
+
+ if (window.location.hash !== '') {
+ const hash = window.location.hash.replace('#', '');
+ $diffFiles
+ .find(`tr#${hash}:not(.match) td, td#${hash}, td[data-line-code="${hash}"]`)
+ .addClass('hll');
+ }
+ }
+ }
+
+ window.gl = window.gl || {};
+ window.gl.Diff = Diff;
+})();
diff --git a/app/assets/javascripts/dispatcher.js.es6 b/app/assets/javascripts/dispatcher.js.es6
index ab521c6c1fc..3a7c5ff3681 100644
--- a/app/assets/javascripts/dispatcher.js.es6
+++ b/app/assets/javascripts/dispatcher.js.es6
@@ -61,7 +61,7 @@
new ZenMode();
break;
case 'projects:compare:show':
- new Diff();
+ new gl.Diff();
break;
case 'projects:issues:new':
case 'projects:issues:edit':
@@ -74,7 +74,7 @@
break;
case 'projects:merge_requests:new':
case 'projects:merge_requests:edit':
- new Diff();
+ new gl.Diff();
shortcut_handler = new ShortcutsNavigation();
new GLForm($('.merge-request-form'));
new IssuableForm($('.merge-request-form'));
@@ -91,7 +91,7 @@
new GLForm($('.release-form'));
break;
case 'projects:merge_requests:show':
- new Diff();
+ new gl.Diff();
shortcut_handler = new ShortcutsIssuable(true);
new ZenMode();
new MergedButtons();
@@ -101,7 +101,7 @@
new MergedButtons();
break;
case "projects:merge_requests:diffs":
- new Diff();
+ new gl.Diff();
new ZenMode();
new MergedButtons();
break;
@@ -117,7 +117,7 @@
break;
case 'projects:commit:show':
new Commit();
- new Diff();
+ new gl.Diff();
new ZenMode();
shortcut_handler = new ShortcutsNavigation();
break;
diff --git a/app/assets/javascripts/merge_request.js b/app/assets/javascripts/merge_request.js
index a4b4db14db8..88c3636be6c 100644
--- a/app/assets/javascripts/merge_request.js
+++ b/app/assets/javascripts/merge_request.js
@@ -40,7 +40,7 @@
if (window.mrTabs) {
window.mrTabs.unbindEvents();
}
- window.mrTabs = new MergeRequestTabs(this.opts);
+ window.mrTabs = new gl.MergeRequestTabs(this.opts);
};
MergeRequest.prototype.showAllCommits = function() {
diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js
deleted file mode 100644
index 4a192ca5796..00000000000
--- a/app/assets/javascripts/merge_request_tabs.js
+++ /dev/null
@@ -1,443 +0,0 @@
-/* eslint-disable max-len, func-names, space-before-function-paren, no-var, space-before-blocks, prefer-rest-params, wrap-iife, no-use-before-define, no-underscore-dangle, no-undef, one-var, one-var-declaration-per-line, quotes, comma-dangle, consistent-return, prefer-template, no-param-reassign, camelcase, vars-on-top, space-in-parens, curly, prefer-arrow-callback, no-unused-vars, no-return-assign, semi, object-shorthand, operator-assignment, padded-blocks, max-len */
-// MergeRequestTabs
-//
-// Handles persisting and restoring the current tab selection and lazily-loading
-// content on the MergeRequests#show page.
-//
-/*= require js.cookie */
-
-//
-// ### Example Markup
-//
-// <ul class="nav-links merge-request-tabs">
-// <li class="notes-tab active">
-// <a data-action="notes" data-target="#notes" data-toggle="tab" href="/foo/bar/merge_requests/1">
-// Discussion
-// </a>
-// </li>
-// <li class="commits-tab">
-// <a data-action="commits" data-target="#commits" data-toggle="tab" href="/foo/bar/merge_requests/1/commits">
-// Commits
-// </a>
-// </li>
-// <li class="diffs-tab">
-// <a data-action="diffs" data-target="#diffs" data-toggle="tab" href="/foo/bar/merge_requests/1/diffs">
-// Diffs
-// </a>
-// </li>
-// </ul>
-//
-// <div class="tab-content">
-// <div class="notes tab-pane active" id="notes">
-// Notes Content
-// </div>
-// <div class="commits tab-pane" id="commits">
-// Commits Content
-// </div>
-// <div class="diffs tab-pane" id="diffs">
-// Diffs Content
-// </div>
-// </div>
-//
-// <div class="mr-loading-status">
-// <div class="loading">
-// Loading Animation
-// </div>
-// </div>
-//
-(function() {
- var bind = function(fn, me){ return function(){ return fn.apply(me, arguments); }; };
-
- this.MergeRequestTabs = (function() {
- MergeRequestTabs.prototype.diffsLoaded = false;
-
- MergeRequestTabs.prototype.buildsLoaded = false;
-
- MergeRequestTabs.prototype.pipelinesLoaded = false;
-
- MergeRequestTabs.prototype.commitsLoaded = false;
-
- MergeRequestTabs.prototype.fixedLayoutPref = null;
-
- function MergeRequestTabs(opts) {
- this.opts = opts != null ? opts : {};
- this.opts.setUrl = this.opts.setUrl !== undefined ? this.opts.setUrl : true;
-
- this.buildsLoaded = this.opts.buildsLoaded || false;
-
- this.setCurrentAction = bind(this.setCurrentAction, this);
- this.tabShown = bind(this.tabShown, this);
- this.showTab = bind(this.showTab, this);
- // Store the `location` object, allowing for easier stubbing in tests
- this._location = location;
- this.bindEvents();
- this.activateTab(this.opts.action);
- this.initAffix();
- }
-
- MergeRequestTabs.prototype.bindEvents = function() {
- $(document).on('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown);
- $(document).on('click', '.js-show-tab', this.showTab);
- };
-
- MergeRequestTabs.prototype.unbindEvents = function() {
- $(document).off('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown);
- $(document).off('click', '.js-show-tab', this.showTab);
- };
-
- MergeRequestTabs.prototype.showTab = function(event) {
- event.preventDefault();
- return this.activateTab($(event.target).data('action'));
- };
-
- MergeRequestTabs.prototype.tabShown = function(event) {
- var $target, action, navBarHeight;
- $target = $(event.target);
- action = $target.data('action');
- if (action === 'commits') {
- this.loadCommits($target.attr('href'));
- this.expandView();
- this.resetViewContainer();
- } else if (this.isDiffAction(action)) {
- this.loadDiff($target.attr('href'));
- if ((typeof bp !== "undefined" && bp !== null) && bp.getBreakpointSize() !== 'lg') {
- this.shrinkView();
- }
- if (this.diffViewType() === 'parallel') {
- this.expandViewContainer();
- }
- navBarHeight = $('.navbar-gitlab').outerHeight();
- $.scrollTo(".merge-request-details .merge-request-tabs", {
- offset: -navBarHeight
- });
- } else if (action === 'builds') {
- this.loadBuilds($target.attr('href'));
- this.expandView();
- this.resetViewContainer();
- } else if (action === 'pipelines') {
- this.loadPipelines($target.attr('href'));
- this.expandView();
- this.resetViewContainer();
- } else {
- this.expandView();
- this.resetViewContainer();
- }
- if (this.opts.setUrl) {
- this.setCurrentAction(action);
- }
- };
-
- MergeRequestTabs.prototype.scrollToElement = function(container) {
- var $el, navBarHeight;
- if (window.location.hash) {
- navBarHeight = $('.navbar-gitlab').outerHeight() + $('.layout-nav').outerHeight() + document.querySelector('.js-tabs-affix').offsetHeight;
- $el = $(container + " " + window.location.hash + ":not(.match)");
- if ($el.length) {
- return $.scrollTo(container + " " + window.location.hash + ":not(.match)", {
- offset: -navBarHeight
- });
- }
- }
- };
-
- // Activate a tab based on the current action
- MergeRequestTabs.prototype.activateTab = function(action) {
- if (action === 'show') {
- action = 'notes';
- }
- // important note: the .tab('show') method triggers 'shown.bs.tab' event itself
- $(".merge-request-tabs a[data-action='" + action + "']").tab('show');
- };
-
- // Replaces the current Merge Request-specific action in the URL with a new one
- //
- // If the action is "notes", the URL is reset to the standard
- // `MergeRequests#show` route.
- //
- // Examples:
- //
- // location.pathname # => "/namespace/project/merge_requests/1"
- // setCurrentAction('diffs')
- // location.pathname # => "/namespace/project/merge_requests/1/diffs"
- //
- // location.pathname # => "/namespace/project/merge_requests/1/diffs"
- // setCurrentAction('notes')
- // location.pathname # => "/namespace/project/merge_requests/1"
- //
- // location.pathname # => "/namespace/project/merge_requests/1/diffs"
- // setCurrentAction('commits')
- // location.pathname # => "/namespace/project/merge_requests/1/commits"
- //
- // Returns the new URL String
- MergeRequestTabs.prototype.setCurrentAction = function(action) {
- var new_state;
- // Normalize action, just to be safe
- if (action === 'show') {
- action = 'notes';
- }
- this.currentAction = action;
- // Remove a trailing '/commits' '/diffs' '/builds' '/pipelines' '/new' '/new/diffs'
- new_state = this._location.pathname.replace(/\/(commits|diffs|builds|pipelines|new|new\/diffs)(\.html)?\/?$/, '');
-
- // Append the new action if we're on a tab other than 'notes'
- if (action !== 'notes') {
- new_state += "/" + action;
- }
- // Ensure parameters and hash come along for the ride
- new_state += this._location.search + this._location.hash;
- history.replaceState({
- turbolinks: true,
- url: new_state
- // Replace the current history state with the new one without breaking
- // Turbolinks' history.
- //
- // See https://github.com/rails/turbolinks/issues/363
- }, document.title, new_state);
- return new_state;
- };
-
- MergeRequestTabs.prototype.loadCommits = function(source) {
- if (this.commitsLoaded) {
- return;
- }
- return this._get({
- url: source + ".json",
- success: (function(_this) {
- return function(data) {
- document.querySelector("div#commits").innerHTML = data.html;
- gl.utils.localTimeAgo($('.js-timeago', 'div#commits'));
- _this.commitsLoaded = true;
- return _this.scrollToElement("#commits");
- };
- })(this)
- });
- };
-
- MergeRequestTabs.prototype.loadDiff = function(source) {
- if (this.diffsLoaded) {
- return;
- }
-
- // We extract pathname for the current Changes tab anchor href
- // some pages like MergeRequestsController#new has query parameters on that anchor
- var url = document.createElement('a');
- url.href = source;
-
- return this._get({
- url: (url.pathname + ".json") + this._location.search,
- success: (function(_this) {
- return function(data) {
- $('#diffs').html(data.html);
-
- if (typeof gl.diffNotesCompileComponents !== 'undefined') {
- gl.diffNotesCompileComponents();
- }
-
- gl.utils.localTimeAgo($('.js-timeago', 'div#diffs'));
- $('#diffs .js-syntax-highlight').syntaxHighlight();
- $('#diffs .diff-file').singleFileDiff();
- if (_this.diffViewType() === 'parallel' && (_this.isDiffAction(_this.currentAction)) ) {
- _this.expandViewContainer();
- }
- _this.diffsLoaded = true;
- var anchoredDiff = gl.utils.getLocationHash();
- if (anchoredDiff) _this.openAnchoredDiff(anchoredDiff, function() {
- _this.scrollToElement("#diffs");
- _this.highlighSelectedLine();
- });
- _this.filesCommentButton = $('.files .diff-file').filesCommentButton();
- return $(document).off('click', '.diff-line-num a').on('click', '.diff-line-num a', function(e) {
- e.preventDefault();
- window.location.hash = $(e.currentTarget).attr('href');
- _this.highlighSelectedLine();
- return _this.scrollToElement("#diffs");
- });
- };
- })(this)
- });
- };
-
- MergeRequestTabs.prototype.openAnchoredDiff = function(anchoredDiff, cb) {
- var diffTitle = $('#file-path-' + anchoredDiff);
- var diffFile = diffTitle.closest('.diff-file');
- var nothingHereBlock = $('.nothing-here-block:visible', diffFile);
- if (nothingHereBlock.length) {
- diffFile.singleFileDiff(true, cb);
- } else {
- cb();
- }
- };
-
- MergeRequestTabs.prototype.highlighSelectedLine = function() {
- var $diffLine, diffLineTop, hashClassString, locationHash, navBarHeight;
- $('.hll').removeClass('hll');
- locationHash = window.location.hash;
- if (locationHash !== '') {
- dataLineString = '[data-line-code="' + locationHash.replace('#', '') + '"]';
- $diffLine = $(locationHash + ":not(.match)", $('#diffs'));
- if (!$diffLine.is('tr')) {
- $diffLine = $('#diffs').find("td" + locationHash + ", td" + dataLineString);
- } else {
- $diffLine = $diffLine.find('td');
- }
- if ($diffLine.length) {
- $diffLine.addClass('hll');
- diffLineTop = $diffLine.offset().top;
- return navBarHeight = $('.navbar-gitlab').outerHeight();
- }
- }
- };
-
- MergeRequestTabs.prototype.loadBuilds = function(source) {
- if (this.buildsLoaded) {
- return;
- }
- return this._get({
- url: source + ".json",
- success: (function(_this) {
- return function(data) {
- document.querySelector("div#builds").innerHTML = data.html;
- gl.utils.localTimeAgo($('.js-timeago', 'div#builds'));
- _this.buildsLoaded = true;
- if (!this.pipelines) this.pipelines = new gl.Pipelines();
- return _this.scrollToElement("#builds");
- };
- })(this)
- });
- };
-
- MergeRequestTabs.prototype.loadPipelines = function(source) {
- if (this.pipelinesLoaded) {
- return;
- }
- return this._get({
- url: source + ".json",
- success: function(data) {
- $('#pipelines').html(data.html);
- gl.utils.localTimeAgo($('.js-timeago', '#pipelines'));
- this.pipelinesLoaded = true;
- return this.scrollToElement("#pipelines");
- }.bind(this)
- });
- };
-
- // Show or hide the loading spinner
- //
- // status - Boolean, true to show, false to hide
- MergeRequestTabs.prototype.toggleLoading = function(status) {
- return $('.mr-loading-status .loading').toggle(status);
- };
-
- MergeRequestTabs.prototype._get = function(options) {
- var defaults;
- defaults = {
- beforeSend: (function(_this) {
- return function() {
- return _this.toggleLoading(true);
- };
- })(this),
- complete: (function(_this) {
- return function() {
- return _this.toggleLoading(false);
- };
- })(this),
- dataType: 'json',
- type: 'GET'
- };
- options = $.extend({}, defaults, options);
- return $.ajax(options);
- };
-
- MergeRequestTabs.prototype.diffViewType = function() {
- return $('.inline-parallel-buttons a.active').data('view-type');
- };
-
- MergeRequestTabs.prototype.isDiffAction = function(action) {
- return action === 'diffs' || action === 'new/diffs'
- };
-
- MergeRequestTabs.prototype.expandViewContainer = function() {
- var $wrapper = $('.content-wrapper .container-fluid');
- if (this.fixedLayoutPref === null) {
- this.fixedLayoutPref = $wrapper.hasClass('container-limited');
- }
- $wrapper.removeClass('container-limited');
- };
-
- MergeRequestTabs.prototype.resetViewContainer = function() {
- if (this.fixedLayoutPref !== null) {
- $('.content-wrapper .container-fluid')
- .toggleClass('container-limited', this.fixedLayoutPref);
- }
- };
-
- MergeRequestTabs.prototype.shrinkView = function() {
- var $gutterIcon;
- $gutterIcon = $('.js-sidebar-toggle i:visible');
- return setTimeout(function() {
- if ($gutterIcon.is('.fa-angle-double-right')) {
- return $gutterIcon.closest('a').trigger('click', [true]);
- }
- // Wait until listeners are set
- // Only when sidebar is expanded
- }, 0);
- };
-
- MergeRequestTabs.prototype.expandView = function() {
- var $gutterIcon;
- if (Cookies.get('collapsed_gutter') === 'true') {
- return;
- }
- $gutterIcon = $('.js-sidebar-toggle i:visible');
- return setTimeout(function() {
- if ($gutterIcon.is('.fa-angle-double-left')) {
- return $gutterIcon.closest('a').trigger('click', [true]);
- }
- }, 0);
- // Expand the issuable sidebar unless the user explicitly collapsed it
- // Wait until listeners are set
- // Only when sidebar is collapsed
- };
-
- MergeRequestTabs.prototype.initAffix = function () {
- var $tabs = $('.js-tabs-affix');
-
- // Screen space on small screens is usually very sparse
- // So we dont affix the tabs on these
- if (Breakpoints.get().getBreakpointSize() === 'xs' || !$tabs.length) return;
-
- var $diffTabs = $('#diff-notes-app'),
- $fixedNav = $('.navbar-fixed-top'),
- $layoutNav = $('.layout-nav');
-
- $tabs.off('affix.bs.affix affix-top.bs.affix')
- .affix({
- offset: {
- top: function () {
- var tabsTop = $diffTabs.offset().top - $tabs.height();
- tabsTop = tabsTop - ($fixedNav.height() + $layoutNav.height());
-
- return tabsTop;
- }
- }
- }).on('affix.bs.affix', function () {
- $diffTabs.css({
- marginTop: $tabs.height()
- });
- }).on('affix-top.bs.affix', function () {
- $diffTabs.css({
- marginTop: ''
- });
- });
-
- // Fix bug when reloading the page already scrolling
- if ($tabs.hasClass('affix')) {
- $tabs.trigger('affix.bs.affix');
- }
- };
-
- return MergeRequestTabs;
-
- })();
-
-}).call(this);
diff --git a/app/assets/javascripts/merge_request_tabs.js.es6 b/app/assets/javascripts/merge_request_tabs.js.es6
new file mode 100644
index 00000000000..583fb9fc03d
--- /dev/null
+++ b/app/assets/javascripts/merge_request_tabs.js.es6
@@ -0,0 +1,390 @@
+/* eslint-disable no-new, class-methods-use-this */
+/* global Breakpoints */
+/* global Cookies */
+/* global DiffNotesApp */
+/* global Flash */
+
+/*= require js.cookie */
+/*= require breakpoints */
+
+/* eslint-disable max-len */
+// MergeRequestTabs
+//
+// Handles persisting and restoring the current tab selection and lazily-loading
+// content on the MergeRequests#show page.
+//
+// ### Example Markup
+//
+// <ul class="nav-links merge-request-tabs">
+// <li class="notes-tab active">
+// <a data-action="notes" data-target="#notes" data-toggle="tab" href="/foo/bar/merge_requests/1">
+// Discussion
+// </a>
+// </li>
+// <li class="commits-tab">
+// <a data-action="commits" data-target="#commits" data-toggle="tab" href="/foo/bar/merge_requests/1/commits">
+// Commits
+// </a>
+// </li>
+// <li class="diffs-tab">
+// <a data-action="diffs" data-target="#diffs" data-toggle="tab" href="/foo/bar/merge_requests/1/diffs">
+// Diffs
+// </a>
+// </li>
+// </ul>
+//
+// <div class="tab-content">
+// <div class="notes tab-pane active" id="notes">
+// Notes Content
+// </div>
+// <div class="commits tab-pane" id="commits">
+// Commits Content
+// </div>
+// <div class="diffs tab-pane" id="diffs">
+// Diffs Content
+// </div>
+// </div>
+//
+// <div class="mr-loading-status">
+// <div class="loading">
+// Loading Animation
+// </div>
+// </div>
+//
+/* eslint-enable max-len */
+
+(() => {
+ // Store the `location` object, allowing for easier stubbing in tests
+ let location = window.location;
+
+ class MergeRequestTabs {
+
+ constructor({ action, setUrl, buildsLoaded, stubLocation } = {}) {
+ this.diffsLoaded = false;
+ this.buildsLoaded = false;
+ this.pipelinesLoaded = false;
+ this.commitsLoaded = false;
+ this.fixedLayoutPref = null;
+
+ this.setUrl = setUrl !== undefined ? setUrl : true;
+ this.buildsLoaded = buildsLoaded || false;
+
+ this.setCurrentAction = this.setCurrentAction.bind(this);
+ this.tabShown = this.tabShown.bind(this);
+ this.showTab = this.showTab.bind(this);
+
+ if (stubLocation) {
+ location = stubLocation;
+ }
+
+ this.bindEvents();
+ this.activateTab(action);
+ this.initAffix();
+ }
+
+ bindEvents() {
+ $(document)
+ .on('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown)
+ .on('click', '.js-show-tab', this.showTab);
+ }
+
+ unbindEvents() {
+ $(document)
+ .off('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown)
+ .off('click', '.js-show-tab', this.showTab);
+ }
+
+ showTab(e) {
+ e.preventDefault();
+ this.activateTab($(e.target).data('action'));
+ }
+
+ tabShown(e) {
+ const $target = $(e.target);
+ const action = $target.data('action');
+
+ if (action === 'commits') {
+ this.loadCommits($target.attr('href'));
+ this.expandView();
+ this.resetViewContainer();
+ } else if (this.isDiffAction(action)) {
+ this.loadDiff($target.attr('href'));
+ if (Breakpoints.get().getBreakpointSize() !== 'lg') {
+ this.shrinkView();
+ }
+ if (this.diffViewType() === 'parallel') {
+ this.expandViewContainer();
+ }
+ const navBarHeight = $('.navbar-gitlab').outerHeight();
+ $.scrollTo('.merge-request-details .merge-request-tabs', {
+ offset: -navBarHeight,
+ });
+ } else if (action === 'builds') {
+ this.loadBuilds($target.attr('href'));
+ this.expandView();
+ this.resetViewContainer();
+ } else if (action === 'pipelines') {
+ this.loadPipelines($target.attr('href'));
+ this.expandView();
+ this.resetViewContainer();
+ } else {
+ this.expandView();
+ this.resetViewContainer();
+ }
+ if (this.setUrl) {
+ this.setCurrentAction(action);
+ }
+ }
+
+ scrollToElement(container) {
+ if (location.hash) {
+ const offset = 0 - (
+ $('.navbar-gitlab').outerHeight() +
+ $('.layout-nav').outerHeight() +
+ $('.js-tabs-affix').outerHeight()
+ );
+ const $el = $(`${container} ${location.hash}:not(.match)`);
+ if ($el.length) {
+ $.scrollTo($el[0], { offset });
+ }
+ }
+ }
+
+ // Activate a tab based on the current action
+ activateTab(action) {
+ const activate = action === 'show' ? 'notes' : action;
+ // important note: the .tab('show') method triggers 'shown.bs.tab' event itself
+ $(`.merge-request-tabs a[data-action='${activate}']`).tab('show');
+ }
+
+ // Replaces the current Merge Request-specific action in the URL with a new one
+ //
+ // If the action is "notes", the URL is reset to the standard
+ // `MergeRequests#show` route.
+ //
+ // Examples:
+ //
+ // location.pathname # => "/namespace/project/merge_requests/1"
+ // setCurrentAction('diffs')
+ // location.pathname # => "/namespace/project/merge_requests/1/diffs"
+ //
+ // location.pathname # => "/namespace/project/merge_requests/1/diffs"
+ // setCurrentAction('notes')
+ // location.pathname # => "/namespace/project/merge_requests/1"
+ //
+ // location.pathname # => "/namespace/project/merge_requests/1/diffs"
+ // setCurrentAction('commits')
+ // location.pathname # => "/namespace/project/merge_requests/1/commits"
+ //
+ // Returns the new URL String
+ setCurrentAction(action) {
+ this.currentAction = action === 'show' ? 'notes' : action;
+
+ // Remove a trailing '/commits' '/diffs' '/builds' '/pipelines' '/new' '/new/diffs'
+ let newState = location.pathname.replace(/\/(commits|diffs|builds|pipelines|new|new\/diffs)(\.html)?\/?$/, '');
+
+ // Append the new action if we're on a tab other than 'notes'
+ if (this.currentAction !== 'notes') {
+ newState += `/${this.currentAction}`;
+ }
+
+ // Ensure parameters and hash come along for the ride
+ newState += location.search + location.hash;
+
+ // Replace the current history state with the new one without breaking
+ // Turbolinks' history.
+ //
+ // See https://github.com/rails/turbolinks/issues/363
+ window.history.replaceState({
+ turbolinks: true,
+ url: newState,
+ }, document.title, newState);
+
+ return newState;
+ }
+
+ loadCommits(source) {
+ if (this.commitsLoaded) {
+ return;
+ }
+ this.ajaxGet({
+ url: `${source}.json`,
+ success: (data) => {
+ document.querySelector('div#commits').innerHTML = data.html;
+ gl.utils.localTimeAgo($('.js-timeago', 'div#commits'));
+ this.commitsLoaded = true;
+ this.scrollToElement('#commits');
+ },
+ });
+ }
+
+ loadDiff(source) {
+ if (this.diffsLoaded) {
+ return;
+ }
+
+ // We extract pathname for the current Changes tab anchor href
+ // some pages like MergeRequestsController#new has query parameters on that anchor
+ const url = document.createElement('a');
+ url.href = source;
+
+ this.ajaxGet({
+ url: `${url.pathname}.json${location.search}`,
+ success: (data) => {
+ $('#diffs').html(data.html);
+
+ if (typeof gl.diffNotesCompileComponents !== 'undefined') {
+ gl.diffNotesCompileComponents();
+ }
+
+ gl.utils.localTimeAgo($('.js-timeago', 'div#diffs'));
+ $('#diffs .js-syntax-highlight').syntaxHighlight();
+
+ if (this.diffViewType() === 'parallel' && this.isDiffAction(this.currentAction)) {
+ this.expandViewContainer();
+ }
+ this.diffsLoaded = true;
+
+ const diffPage = new gl.Diff();
+
+ const locationHash = gl.utils.getLocationHash();
+ const anchoredDiff = locationHash && locationHash.split('_')[0];
+ if (anchoredDiff) {
+ diffPage.openAnchoredDiff(anchoredDiff, () => this.scrollToElement('#diffs'));
+ }
+ },
+ });
+ }
+
+ loadBuilds(source) {
+ if (this.buildsLoaded) {
+ return;
+ }
+ this.ajaxGet({
+ url: `${source}.json`,
+ success: (data) => {
+ document.querySelector('div#builds').innerHTML = data.html;
+ gl.utils.localTimeAgo($('.js-timeago', 'div#builds'));
+ this.buildsLoaded = true;
+ new gl.Pipelines();
+ this.scrollToElement('#builds');
+ },
+ });
+ }
+
+ loadPipelines(source) {
+ if (this.pipelinesLoaded) {
+ return;
+ }
+ this.ajaxGet({
+ url: `${source}.json`,
+ success: (data) => {
+ $('#pipelines').html(data.html);
+ gl.utils.localTimeAgo($('.js-timeago', '#pipelines'));
+ this.pipelinesLoaded = true;
+ this.scrollToElement('#pipelines');
+ },
+ });
+ }
+
+ // Show or hide the loading spinner
+ //
+ // status - Boolean, true to show, false to hide
+ toggleLoading(status) {
+ $('.mr-loading-status .loading').toggle(status);
+ }
+
+ ajaxGet(options) {
+ const defaults = {
+ beforeSend: () => this.toggleLoading(true),
+ error: () => new Flash('An error occurred while fetching this tab.', 'alert'),
+ complete: () => this.toggleLoading(false),
+ dataType: 'json',
+ type: 'GET',
+ };
+ $.ajax($.extend({}, defaults, options));
+ }
+
+ diffViewType() {
+ return $('.inline-parallel-buttons a.active').data('view-type');
+ }
+
+ isDiffAction(action) {
+ return action === 'diffs' || action === 'new/diffs';
+ }
+
+ expandViewContainer() {
+ const $wrapper = $('.content-wrapper .container-fluid');
+ if (this.fixedLayoutPref === null) {
+ this.fixedLayoutPref = $wrapper.hasClass('container-limited');
+ }
+ $wrapper.removeClass('container-limited');
+ }
+
+ resetViewContainer() {
+ if (this.fixedLayoutPref !== null) {
+ $('.content-wrapper .container-fluid')
+ .toggleClass('container-limited', this.fixedLayoutPref);
+ }
+ }
+
+ shrinkView() {
+ const $gutterIcon = $('.js-sidebar-toggle i:visible');
+
+ // Wait until listeners are set
+ setTimeout(() => {
+ // Only when sidebar is expanded
+ if ($gutterIcon.is('.fa-angle-double-right')) {
+ $gutterIcon.closest('a').trigger('click', [true]);
+ }
+ }, 0);
+ }
+
+ // Expand the issuable sidebar unless the user explicitly collapsed it
+ expandView() {
+ if (Cookies.get('collapsed_gutter') === 'true') {
+ return;
+ }
+ const $gutterIcon = $('.js-sidebar-toggle i:visible');
+
+ // Wait until listeners are set
+ setTimeout(() => {
+ // Only when sidebar is collapsed
+ if ($gutterIcon.is('.fa-angle-double-left')) {
+ $gutterIcon.closest('a').trigger('click', [true]);
+ }
+ }, 0);
+ }
+
+ initAffix() {
+ const $tabs = $('.js-tabs-affix');
+
+ // Screen space on small screens is usually very sparse
+ // So we dont affix the tabs on these
+ if (Breakpoints.get().getBreakpointSize() === 'xs' || !$tabs.length) return;
+
+ const $diffTabs = $('#diff-notes-app');
+ const $fixedNav = $('.navbar-fixed-top');
+ const $layoutNav = $('.layout-nav');
+
+ $tabs.off('affix.bs.affix affix-top.bs.affix')
+ .affix({
+ offset: {
+ top: () => (
+ $diffTabs.offset().top - $tabs.height() - $fixedNav.height() - $layoutNav.height()
+ ),
+ },
+ })
+ .on('affix.bs.affix', () => $diffTabs.css({ marginTop: $tabs.height() }))
+ .on('affix-top.bs.affix', () => $diffTabs.css({ marginTop: '' }));
+
+ // Fix bug when reloading the page already scrolling
+ if ($tabs.hasClass('affix')) {
+ $tabs.trigger('affix.bs.affix');
+ }
+ }
+ }
+
+ window.gl = window.gl || {};
+ window.gl.MergeRequestTabs = MergeRequestTabs;
+})();
diff --git a/app/assets/javascripts/single_file_diff.js b/app/assets/javascripts/single_file_diff.js
index 2767849e673..0d48e69cce9 100644
--- a/app/assets/javascripts/single_file_diff.js
+++ b/app/assets/javascripts/single_file_diff.js
@@ -14,6 +14,7 @@
COLLAPSED_HTML = '<div class="nothing-here-block diff-collapsed">This diff is collapsed. <a class="click-to-expand">Click to expand it.</a></div>';
function SingleFileDiff(file, forceLoad, cb) {
+ var clickTarget;
this.file = file;
this.toggleDiff = bind(this.toggleDiff, this);
this.content = $('.diff-content', this.file);
@@ -31,9 +32,9 @@
this.content.after(this.collapsedContent);
this.$toggleIcon.addClass('fa-caret-down');
}
- $('.file-title, .click-to-expand', this.file).on('click', this.toggleDiff);
+ clickTarget = $('.file-title, .click-to-expand', this.file).on('click', this.toggleDiff);
if (forceLoad) {
- this.toggleDiff(null, cb);
+ this.toggleDiff({ target: clickTarget }, cb);
}
}
diff --git a/changelogs/unreleased/23696-fix-diff-view-highlighting.yml b/changelogs/unreleased/23696-fix-diff-view-highlighting.yml
new file mode 100644
index 00000000000..db523caffed
--- /dev/null
+++ b/changelogs/unreleased/23696-fix-diff-view-highlighting.yml
@@ -0,0 +1,4 @@
+---
+title: Fix diff view permalink highlighting
+merge_request: 7090
+author:
diff --git a/spec/javascripts/merge_request_tabs_spec.js b/spec/javascripts/merge_request_tabs_spec.js
index 971222c44e1..65e4177ecfe 100644
--- a/spec/javascripts/merge_request_tabs_spec.js
+++ b/spec/javascripts/merge_request_tabs_spec.js
@@ -1,108 +1,111 @@
-/* eslint-disable space-before-function-paren, no-var, comma-dangle, dot-notation, quotes, no-undef, no-return-assign, no-underscore-dangle, camelcase, padded-blocks, max-len */
+/* eslint-disable no-var, comma-dangle, object-shorthand */
/*= require merge_request_tabs */
//= require breakpoints
-(function() {
- describe('MergeRequestTabs', function() {
- var stubLocation;
- stubLocation = function(stubs) {
- var defaults;
- defaults = {
+(function () {
+ describe('MergeRequestTabs', function () {
+ var stubLocation = {};
+ var setLocation = function (stubs) {
+ var defaults = {
pathname: '',
search: '',
hash: ''
};
- return $.extend(defaults, stubs);
+ $.extend(stubLocation, defaults, stubs || {});
};
fixture.preload('merge_request_tabs.html');
- beforeEach(function() {
- this["class"] = new MergeRequestTabs();
- return this.spies = {
- ajax: spyOn($, 'ajax').and.callFake(function() {}),
- history: spyOn(history, 'replaceState').and.callFake(function() {})
+
+ beforeEach(function () {
+ this.class = new gl.MergeRequestTabs({ stubLocation: stubLocation });
+ setLocation();
+
+ this.spies = {
+ ajax: spyOn($, 'ajax').and.callFake(function () {}),
+ history: spyOn(window.history, 'replaceState').and.callFake(function () {})
};
});
- describe('#activateTab', function() {
- beforeEach(function() {
+
+ describe('#activateTab', function () {
+ beforeEach(function () {
fixture.load('merge_request_tabs.html');
- return this.subject = this["class"].activateTab;
+ this.subject = this.class.activateTab;
});
- it('shows the first tab when action is show', function() {
+ it('shows the first tab when action is show', function () {
this.subject('show');
- return expect($('#notes')).toHaveClass('active');
+ expect($('#notes')).toHaveClass('active');
});
- it('shows the notes tab when action is notes', function() {
+ it('shows the notes tab when action is notes', function () {
this.subject('notes');
- return expect($('#notes')).toHaveClass('active');
+ expect($('#notes')).toHaveClass('active');
});
- it('shows the commits tab when action is commits', function() {
+ it('shows the commits tab when action is commits', function () {
this.subject('commits');
- return expect($('#commits')).toHaveClass('active');
+ expect($('#commits')).toHaveClass('active');
});
- return it('shows the diffs tab when action is diffs', function() {
+ it('shows the diffs tab when action is diffs', function () {
this.subject('diffs');
- return expect($('#diffs')).toHaveClass('active');
+ expect($('#diffs')).toHaveClass('active');
});
});
- return describe('#setCurrentAction', function() {
- beforeEach(function() {
- return this.subject = this["class"].setCurrentAction;
+
+ describe('#setCurrentAction', function () {
+ beforeEach(function () {
+ this.subject = this.class.setCurrentAction;
});
- it('changes from commits', function() {
- this["class"]._location = stubLocation({
+ it('changes from commits', function () {
+ setLocation({
pathname: '/foo/bar/merge_requests/1/commits'
});
expect(this.subject('notes')).toBe('/foo/bar/merge_requests/1');
- return expect(this.subject('diffs')).toBe('/foo/bar/merge_requests/1/diffs');
+ expect(this.subject('diffs')).toBe('/foo/bar/merge_requests/1/diffs');
});
- it('changes from diffs', function() {
- this["class"]._location = stubLocation({
+ it('changes from diffs', function () {
+ setLocation({
pathname: '/foo/bar/merge_requests/1/diffs'
});
expect(this.subject('notes')).toBe('/foo/bar/merge_requests/1');
- return expect(this.subject('commits')).toBe('/foo/bar/merge_requests/1/commits');
+ expect(this.subject('commits')).toBe('/foo/bar/merge_requests/1/commits');
});
- it('changes from diffs.html', function() {
- this["class"]._location = stubLocation({
+ it('changes from diffs.html', function () {
+ setLocation({
pathname: '/foo/bar/merge_requests/1/diffs.html'
});
expect(this.subject('notes')).toBe('/foo/bar/merge_requests/1');
- return expect(this.subject('commits')).toBe('/foo/bar/merge_requests/1/commits');
+ expect(this.subject('commits')).toBe('/foo/bar/merge_requests/1/commits');
});
- it('changes from notes', function() {
- this["class"]._location = stubLocation({
+ it('changes from notes', function () {
+ setLocation({
pathname: '/foo/bar/merge_requests/1'
});
expect(this.subject('diffs')).toBe('/foo/bar/merge_requests/1/diffs');
- return expect(this.subject('commits')).toBe('/foo/bar/merge_requests/1/commits');
+ expect(this.subject('commits')).toBe('/foo/bar/merge_requests/1/commits');
});
- it('includes search parameters and hash string', function() {
- this["class"]._location = stubLocation({
+ it('includes search parameters and hash string', function () {
+ setLocation({
pathname: '/foo/bar/merge_requests/1/diffs',
search: '?view=parallel',
hash: '#L15-35'
});
- return expect(this.subject('show')).toBe('/foo/bar/merge_requests/1?view=parallel#L15-35');
+ expect(this.subject('show')).toBe('/foo/bar/merge_requests/1?view=parallel#L15-35');
});
- it('replaces the current history state', function() {
- var new_state;
- this["class"]._location = stubLocation({
+ it('replaces the current history state', function () {
+ var newState;
+ setLocation({
pathname: '/foo/bar/merge_requests/1'
});
- new_state = this.subject('commits');
- return expect(this.spies.history).toHaveBeenCalledWith({
+ newState = this.subject('commits');
+ expect(this.spies.history).toHaveBeenCalledWith({
turbolinks: true,
- url: new_state
- }, document.title, new_state);
+ url: newState
+ }, document.title, newState);
});
- return it('treats "show" like "notes"', function() {
- this["class"]._location = stubLocation({
+ it('treats "show" like "notes"', function () {
+ setLocation({
pathname: '/foo/bar/merge_requests/1/commits'
});
- return expect(this.subject('show')).toBe('/foo/bar/merge_requests/1');
+ expect(this.subject('show')).toBe('/foo/bar/merge_requests/1');
});
});
});
-
}).call(this);