Commit 81e9c284 authored by Bryce Johnson's avatar Bryce Johnson

Render add-diff-note button with server.

This commit moves the rendering of the button back to the server, and
shows/hides it using opacity rather than display. It also removes the
transform applied to the button on hover (scale). Previously, both of these
factors automatically triggered a reflow, which creates a performance
bottleneck on pages with larger DOM size.

MR: !12103
parent 066a6c8b
Pipeline #9449407 passed with stages
in 106 minutes and 41 seconds
/* eslint-disable class-methods-use-this */
import './lib/utils/url_utility';
import FilesCommentButton from './files_comment_button';
const UNFOLD_COUNT = 20;
let isBound = false;
......@@ -8,8 +9,10 @@ let isBound = false;
class Diff {
constructor() {
const $diffFile = $('.files .diff-file');
$diffFile.singleFileDiff();
$diffFile.filesCommentButton();
FilesCommentButton.init($diffFile);
$diffFile.each((index, file) => new gl.ImageFile(file));
......
......@@ -139,9 +139,9 @@ const DiffNoteAvatars = Vue.extend({
const notesCount = this.notesCount;
$(this.$el).closest('.js-avatar-container')
.toggleClass('js-no-comment-btn', notesCount > 0)
.toggleClass('no-comment-btn', notesCount > 0)
.nextUntil('.js-avatar-container')
.toggleClass('js-no-comment-btn', notesCount > 0);
.toggleClass('no-comment-btn', notesCount > 0);
},
toggleDiscussionsToggleState() {
const $notesHolders = $(this.$el).closest('.code').find('.notes_holder');
......
/* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, max-len, one-var, one-var-declaration-per-line, quotes, prefer-template, newline-per-chained-call, comma-dangle, new-cap, no-else-return, consistent-return */
/* global FilesCommentButton */
/* global notes */
let $commentButtonTemplate;
window.FilesCommentButton = (function() {
var COMMENT_BUTTON_CLASS, EMPTY_CELL_CLASS, LINE_COLUMN_CLASSES, LINE_CONTENT_CLASS, LINE_HOLDER_CLASS, LINE_NUMBER_CLASS, OLD_LINE_CLASS, TEXT_FILE_SELECTOR, UNFOLDABLE_LINE_CLASS;
COMMENT_BUTTON_CLASS = '.add-diff-note';
LINE_HOLDER_CLASS = '.line_holder';
LINE_NUMBER_CLASS = 'diff-line-num';
LINE_CONTENT_CLASS = 'line_content';
UNFOLDABLE_LINE_CLASS = 'js-unfold';
EMPTY_CELL_CLASS = 'empty-cell';
OLD_LINE_CLASS = 'old_line';
LINE_COLUMN_CLASSES = "." + LINE_NUMBER_CLASS + ", .line_content";
TEXT_FILE_SELECTOR = '.text-file';
function FilesCommentButton(filesContainerElement) {
this.render = this.render.bind(this);
this.hideButton = this.hideButton.bind(this);
this.isParallelView = notes.isParallelView();
filesContainerElement.on('mouseover', LINE_COLUMN_CLASSES, this.render)
.on('mouseleave', LINE_COLUMN_CLASSES, this.hideButton);
}
FilesCommentButton.prototype.render = function(e) {
var $currentTarget, buttonParentElement, lineContentElement, textFileElement, $button;
$currentTarget = $(e.currentTarget);
if ($currentTarget.hasClass('js-no-comment-btn')) return;
lineContentElement = this.getLineContent($currentTarget);
buttonParentElement = this.getButtonParent($currentTarget);
if (!this.validateButtonParent(buttonParentElement) || !this.validateLineContent(lineContentElement)) return;
$button = $(COMMENT_BUTTON_CLASS, buttonParentElement);
buttonParentElement.addClass('is-over')
.nextUntil(`.${LINE_CONTENT_CLASS}`).addClass('is-over');
if ($button.length) {
return;
/* Developer beware! Do not add logic to showButton or hideButton
* that will force a reflow. Doing so will create a signficant performance
* bottleneck for pages with large diffs. For a comprehensive list of what
* causes reflows, visit https://gist.github.com/paulirish/5d52fb081b3570c81e3a
*/
const LINE_NUMBER_CLASS = 'diff-line-num';
const UNFOLDABLE_LINE_CLASS = 'js-unfold';
const NO_COMMENT_CLASS = 'no-comment-btn';
const EMPTY_CELL_CLASS = 'empty-cell';
const OLD_LINE_CLASS = 'old_line';
const LINE_COLUMN_CLASSES = `.${LINE_NUMBER_CLASS}, .line_content`;
const DIFF_CONTAINER_SELECTOR = '.files';
const DIFF_EXPANDED_CLASS = 'diff-expanded';
export default {
init($diffFile) {
/* Caching is used only when the following members are *true*. This is because there are likely to be
* differently configured versions of diffs in the same session. However if these values are true, they
* will be true in all cases */
if (!this.userCanCreateNote) {
// data-can-create-note is an empty string when true, otherwise undefined
this.userCanCreateNote = $diffFile.closest(DIFF_CONTAINER_SELECTOR).data('can-create-note') === '';
}
textFileElement = this.getTextFileElement($currentTarget);
buttonParentElement.append(this.buildButton({
discussionID: lineContentElement.attr('data-discussion-id'),
lineType: lineContentElement.attr('data-line-type'),
noteableType: textFileElement.attr('data-noteable-type'),
noteableID: textFileElement.attr('data-noteable-id'),
commitID: textFileElement.attr('data-commit-id'),
noteType: lineContentElement.attr('data-note-type'),
// LegacyDiffNote
lineCode: lineContentElement.attr('data-line-code'),
// DiffNote
position: lineContentElement.attr('data-position')
}));
};
FilesCommentButton.prototype.hideButton = function(e) {
var $currentTarget = $(e.currentTarget);
var buttonParentElement = this.getButtonParent($currentTarget);
buttonParentElement.removeClass('is-over')
.nextUntil(`.${LINE_CONTENT_CLASS}`).removeClass('is-over');
};
FilesCommentButton.prototype.buildButton = function(buttonAttributes) {
return $commentButtonTemplate.clone().attr({
'data-discussion-id': buttonAttributes.discussionID,
'data-line-type': buttonAttributes.lineType,
'data-noteable-type': buttonAttributes.noteableType,
'data-noteable-id': buttonAttributes.noteableID,
'data-commit-id': buttonAttributes.commitID,
'data-note-type': buttonAttributes.noteType,
// LegacyDiffNote
'data-line-code': buttonAttributes.lineCode,
// DiffNote
'data-position': buttonAttributes.position
});
};
FilesCommentButton.prototype.getTextFileElement = function(hoveredElement) {
return hoveredElement.closest(TEXT_FILE_SELECTOR);
};
FilesCommentButton.prototype.getLineContent = function(hoveredElement) {
if (hoveredElement.hasClass(LINE_CONTENT_CLASS)) {
return hoveredElement;
}
if (!this.isParallelView) {
return $(hoveredElement).closest(LINE_HOLDER_CLASS).find("." + LINE_CONTENT_CLASS);
} else {
return $(hoveredElement).next("." + LINE_CONTENT_CLASS);
if (typeof notes !== 'undefined' && !this.isParallelView) {
this.isParallelView = notes.isParallelView && notes.isParallelView();
}
};
FilesCommentButton.prototype.getButtonParent = function(hoveredElement) {
if (!this.isParallelView) {
if (hoveredElement.hasClass(OLD_LINE_CLASS)) {
return hoveredElement;
}
return hoveredElement.parent().find("." + OLD_LINE_CLASS);
} else {
if (hoveredElement.hasClass(LINE_NUMBER_CLASS)) {
return hoveredElement;
}
return $(hoveredElement).prev("." + LINE_NUMBER_CLASS);
if (this.userCanCreateNote) {
$diffFile.on('mouseover', LINE_COLUMN_CLASSES, e => this.showButton(this.isParallelView, e))
.on('mouseleave', LINE_COLUMN_CLASSES, e => this.hideButton(this.isParallelView, e));
}
};
},
FilesCommentButton.prototype.validateButtonParent = function(buttonParentElement) {
return !buttonParentElement.hasClass(EMPTY_CELL_CLASS) && !buttonParentElement.hasClass(UNFOLDABLE_LINE_CLASS);
};
showButton(isParallelView, e) {
const buttonParentElement = this.getButtonParent(e.currentTarget, isParallelView);
FilesCommentButton.prototype.validateLineContent = function(lineContentElement) {
return lineContentElement.attr('data-note-type') && lineContentElement.attr('data-note-type') !== '';
};
if (!this.validateButtonParent(buttonParentElement)) return;
return FilesCommentButton;
})();
buttonParentElement.classList.add('is-over');
buttonParentElement.nextElementSibling.classList.add('is-over');
},
$.fn.filesCommentButton = function() {
$commentButtonTemplate = $('<button name="button" type="submit" class="add-diff-note js-add-diff-note-button" title="Add a comment to this line"><i class="fa fa-comment-o"></i></button>');
hideButton(isParallelView, e) {
const buttonParentElement = this.getButtonParent(e.currentTarget, isParallelView);
if (!(this && (this.parent().data('can-create-note') != null))) {
return;
}
return this.each(function() {
if (!$.data(this, 'filesCommentButton')) {
return $.data(this, 'filesCommentButton', new FilesCommentButton($(this)));
buttonParentElement.classList.remove('is-over');
buttonParentElement.nextElementSibling.classList.remove('is-over');
},
getButtonParent(hoveredElement, isParallelView) {
if (isParallelView) {
if (!hoveredElement.classList.contains(LINE_NUMBER_CLASS)) {
return hoveredElement.previousElementSibling;
}
} else if (!hoveredElement.classList.contains(OLD_LINE_CLASS)) {
return hoveredElement.parentNode.querySelector(`.${OLD_LINE_CLASS}`);
}
});
return hoveredElement;
},
validateButtonParent(buttonParentElement) {
return !buttonParentElement.classList.contains(EMPTY_CELL_CLASS) &&
!buttonParentElement.classList.contains(UNFOLDABLE_LINE_CLASS) &&
!buttonParentElement.classList.contains(NO_COMMENT_CLASS) &&
!buttonParentElement.parentNode.classList.contains(DIFF_EXPANDED_CLASS);
},
};
......@@ -829,6 +829,8 @@ export default class Notes {
*/
setupDiscussionNoteForm(dataHolder, form) {
// setup note target
const diffFileData = dataHolder.closest('.text-file');
var discussionID = dataHolder.data('discussionId');
if (discussionID) {
......@@ -839,9 +841,10 @@ export default class Notes {
form.attr('data-line-code', dataHolder.data('lineCode'));
form.find('#line_type').val(dataHolder.data('lineType'));
form.find('#note_noteable_type').val(dataHolder.data('noteableType'));
form.find('#note_noteable_id').val(dataHolder.data('noteableId'));
form.find('#note_commit_id').val(dataHolder.data('commitId'));
form.find('#note_noteable_type').val(diffFileData.data('noteableType'));
form.find('#note_noteable_id').val(diffFileData.data('noteableId'));
form.find('#note_commit_id').val(diffFileData.data('commitId'));
form.find('#note_type').val(dataHolder.data('noteType'));
// LegacyDiffNote
......
/* eslint-disable func-names, prefer-arrow-callback, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, one-var, one-var-declaration-per-line, consistent-return, no-param-reassign, max-len */
import FilesCommentButton from './files_comment_button';
(function() {
window.SingleFileDiff = (function() {
var COLLAPSED_HTML, ERROR_HTML, LOADING_HTML, WRAPPER;
......@@ -78,6 +80,8 @@
gl.diffNotesCompileComponents();
}
FilesCommentButton.init($(_this.file));
if (cb) cb();
};
})(this));
......
......@@ -476,6 +476,7 @@
height: 19px;
width: 19px;
margin-left: -15px;
z-index: 100;
&:hover {
.diff-comment-avatar,
......@@ -491,7 +492,7 @@
transform: translateX((($i * $x-pos) - $x-pos));
&:hover {
transform: translateX((($i * $x-pos) - $x-pos)) scale(1.2);
transform: translateX((($i * $x-pos) - $x-pos));
}
}
}
......@@ -542,6 +543,7 @@
height: 19px;
padding: 0;
transition: transform .1s ease-out;
z-index: 100;
svg {
position: absolute;
......@@ -555,10 +557,6 @@
fill: $white-light;
}
&:hover {
transform: scale(1.2);
}
&:focus {
outline: 0;
}
......
......@@ -628,8 +628,14 @@ ul.notes {
* Line note button on the side of diffs
*/
.line_holder .is-over:not(.no-comment-btn) {
.add-diff-note {
opacity: 1;
}
}
.add-diff-note {
display: none;
opacity: 0;
margin-top: -2px;
border-radius: 50%;
background: $white-light;
......@@ -642,13 +648,11 @@ ul.notes {
width: 23px;
height: 23px;
border: 1px solid $blue-500;
transition: transform .1s ease-in-out;
&:hover {
background: $blue-500;
border-color: $blue-600;
color: $white-light;
transform: scale(1.15);
}
&:active {
......
......@@ -47,6 +47,18 @@ module NotesHelper
data
end
def add_diff_note_button(line_code, position, line_type)
return if @diff_notes_disabled
button_tag '',
class: 'add-diff-note js-add-diff-note-button',
type: 'submit', name: 'button',
data: diff_view_line_data(line_code, position, line_type),
title: 'Add a comment to this line' do
icon('comment-o')
end
end
def link_to_reply_discussion(discussion, line_type = nil)
return unless current_user
......
......@@ -19,6 +19,7 @@
- if plain
= link_text
- else
= add_diff_note_button(line_code, diff_file.position(line), type)
%a{ href: "##{line_code}", data: { linenumber: link_text } }
- discussion = line_discussions.try(:first)
- if discussion && discussion.resolvable? && !plain
......@@ -29,7 +30,7 @@
= link_text
- else
%a{ href: "##{line_code}", data: { linenumber: link_text } }
%td.line_content.noteable_line{ class: type, data: (diff_view_line_data(line_code, diff_file.position(line), type) unless plain) }<
%td.line_content.noteable_line{ class: type }<
- if email
%pre= line.text
- else
......
/ Side-by-side diff view
.text-file.diff-wrap-lines.code.js-syntax-highlight{ data: diff_view_data }
%table
- diff_file.parallel_diff_lines.each do |line|
......@@ -18,11 +19,12 @@
- left_line_code = diff_file.line_code(left)
- left_position = diff_file.position(left)
%td.old_line.diff-line-num.js-avatar-container{ id: left_line_code, class: left.type, data: { linenumber: left.old_pos } }
= add_diff_note_button(left_line_code, left_position, 'old')
%a{ href: "##{left_line_code}", data: { linenumber: left.old_pos } }
- discussion_left = discussions_left.try(:first)
- if discussion_left && discussion_left.resolvable?
%diff-note-avatars{ "discussion-id" => discussion_left.id }
%td.line_content.parallel.noteable_line{ class: left.type, data: diff_view_line_data(left_line_code, left_position, 'old') }= diff_line_content(left.text)
%td.line_content.parallel.noteable_line{ class: left.type }= diff_line_content(left.text)
- else
%td.old_line.diff-line-num.empty-cell
%td.line_content.parallel
......@@ -38,11 +40,12 @@
- right_line_code = diff_file.line_code(right)
- right_position = diff_file.position(right)
%td.new_line.diff-line-num.js-avatar-container{ id: right_line_code, class: right.type, data: { linenumber: right.new_pos } }
= add_diff_note_button(right_line_code, right_position, 'new')
%a{ href: "##{right_line_code}", data: { linenumber: right.new_pos } }
- discussion_right = discussions_right.try(:first)
- if discussion_right && discussion_right.resolvable?
%diff-note-avatars{ "discussion-id" => discussion_right.id }
%td.line_content.parallel.noteable_line{ class: right.type, data: diff_view_line_data(right_line_code, right_position, 'new') }= diff_line_content(right.text)
%td.line_content.parallel.noteable_line{ class: right.type }= diff_line_content(right.text)
- else
%td.old_line.diff-line-num.empty-cell
%td.line_content.parallel
......
......@@ -232,7 +232,7 @@ module SharedDiffNote
end
def click_parallel_diff_line(code, line_type)
find(".line_content.parallel.#{line_type}[data-line-code='#{code}']").trigger 'mouseover'
find(".line_holder.parallel .diff-line-num[id='#{code}']").trigger 'mouseover'
find(".line_holder.parallel button[data-line-code='#{code}']").trigger 'click'
end
end
......@@ -129,7 +129,7 @@ feature 'Expand and collapse diffs', js: true, feature: true do
before do
large_diff.find('.diff-line-num', match: :prefer_exact).hover
large_diff.find('.add-diff-note').click
large_diff.find('.add-diff-note', match: :prefer_exact).click
large_diff.find('.note-textarea').send_keys comment_text
large_diff.find_button('Comment').click
wait_for_requests
......
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