BigW Consortium Gitlab

Skip to content

G
gitlab-ce
Commit cc41ec97 by Phil Hughes
Options

Improved the diff comment button UX

It now shows the line will be commenting on my highlight the line number cells with a lighter color. The button has also been made smaller, it was previously way over the top & took over a lot more space than it should of done Closes #27543
parent 8b855eaf
Showing with 106 additions and 59 deletions
app/assets/javascripts/files_comment_button.js
......@@ -2,15 +2,14 @@
/* global FilesCommentButton */
(function() {
var $commentButtonTemplate;
var bind = function(fn, me) { return function() { return fn.apply(me, arguments); }; };
this.FilesCommentButton = (function() {
var COMMENT_BUTTON_CLASS, COMMENT_BUTTON_TEMPLATE, DEBOUNCE_TIMEOUT_DURATION, EMPTY_CELL_CLASS, LINE_COLUMN_CLASSES, LINE_CONTENT_CLASS, LINE_HOLDER_CLASS, LINE_NUMBER_CLASS, OLD_LINE_CLASS, TEXT_FILE_SELECTOR, UNFOLDABLE_LINE_CLASS;
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';
COMMENT_BUTTON_TEMPLATE = _.template('<button name="button" type="submit" class="btn <%- COMMENT_BUTTON_CLASS %> js-add-diff-note-button" title="Add a comment to this line"><i class="fa fa-comment-o"></i></button>');
LINE_HOLDER_CLASS = '.line_holder';
LINE_NUMBER_CLASS = 'diff-line-num';
......@@ -27,20 +26,18 @@
TEXT_FILE_SELECTOR = '.text-file';
DEBOUNCE_TIMEOUT_DURATION = 100;
function FilesCommentButton(filesContainerElement) {
var debounce;
this.filesContainerElement = filesContainerElement;
this.destroy = bind(this.destroy, this);
this.render = bind(this.render, this);
this.hideButton = bind(this.hideButton, this);
this.VIEW_TYPE = $('input#view[type=hidden]').val();
debounce = _.debounce(this.render, DEBOUNCE_TIMEOUT_DURATION);
$(this.filesContainerElement).off('mouseover', LINE_COLUMN_CLASSES).off('mouseleave', LINE_COLUMN_CLASSES).on('mouseover', LINE_COLUMN_CLASSES, debounce).on('mouseleave', LINE_COLUMN_CLASSES, this.destroy);
$(this.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;
var $currentTarget, buttonParentElement, lineContentElement, textFileElement, $button;
$currentTarget = $(e.currentTarget);
buttonParentElement = this.getButtonParent($currentTarget);
......@@ -48,6 +45,14 @@
lineContentElement = this.getLineContent($currentTarget);
if (!this.validateLineContent(lineContentElement)) return;
$button = $(COMMENT_BUTTON_CLASS, buttonParentElement);
buttonParentElement.addClass('is-over')
.nextUntil('.line_content').addClass('is-over');
if ($button.length) {
return;
}
textFileElement = this.getTextFileElement($currentTarget);
buttonParentElement.append(this.buildButton({
noteableType: textFileElement.attr('data-noteable-type'),
......@@ -61,19 +66,16 @@
}));
};
FilesCommentButton.prototype.destroy = function(e) {
if (this.isMovingToSameType(e)) {
return;
}
$(COMMENT_BUTTON_CLASS, this.getButtonParent($(e.currentTarget))).remove();
FilesCommentButton.prototype.hideButton = function(e) {
var $currentTarget = $(e.currentTarget);
var buttonParentElement = this.getButtonParent($currentTarget);
buttonParentElement.removeClass('is-over')
.nextUntil('.line_content').removeClass('is-over');
};
FilesCommentButton.prototype.buildButton = function(buttonAttributes) {
var initializedButtonTemplate;
initializedButtonTemplate = COMMENT_BUTTON_TEMPLATE({
COMMENT_BUTTON_CLASS: COMMENT_BUTTON_CLASS.substr(1)
});
return $(initializedButtonTemplate).attr({
return $commentButtonTemplate.clone().attr({
'data-noteable-type': buttonAttributes.noteableType,
'data-noteable-id': buttonAttributes.noteableID,
'data-commit-id': buttonAttributes.commitID,
......@@ -114,17 +116,8 @@
}
};
FilesCommentButton.prototype.isMovingToSameType = function(e) {
var newButtonParent;
newButtonParent = this.getButtonParent($(e.toElement));
if (!newButtonParent) {
return false;
}
return newButtonParent.is(this.getButtonParent($(e.currentTarget)));
};
FilesCommentButton.prototype.validateButtonParent = function(buttonParentElement) {
return !buttonParentElement.hasClass(EMPTY_CELL_CLASS) && !buttonParentElement.hasClass(UNFOLDABLE_LINE_CLASS) && $(COMMENT_BUTTON_CLASS, buttonParentElement).length === 0;
return !buttonParentElement.hasClass(EMPTY_CELL_CLASS) && !buttonParentElement.hasClass(UNFOLDABLE_LINE_CLASS);
};
FilesCommentButton.prototype.validateLineContent = function(lineContentElement) {
......@@ -135,6 +128,8 @@
})();
$.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>');
if (!(this && (this.parent().data('can-create-note') != null))) {
return;
}
......
app/assets/stylesheets/highlight/dark.scss
......@@ -139,6 +139,17 @@ $dark-il: #de935f;
}
}
.diff-line-num {
&.is-over {
background-color: #ded7fc;
border-color: darken(#ded7fc, 5%);
a {
color: darken(#ded7fc, 15%);
}
}
}
.line_content.match {
@include dark-diff-match-line;
}
......
app/assets/stylesheets/highlight/monokai.scss
......@@ -139,6 +139,17 @@ $monokai-gi: #a6e22e;
}
}
.diff-line-num {
&.is-over {
background-color: #9f9ab5;
border-color: darken(#9f9ab5, 5%);
a {
color: darken(#9f9ab5, 15%);
}
}
}
.line_content.match {
@include dark-diff-match-line;
}
......
app/assets/stylesheets/highlight/solarized_dark.scss
......@@ -143,6 +143,17 @@ $solarized-dark-il: #2aa198;
}
}
.diff-line-num {
&.is-over {
background-color: #9f9ab5;
border-color: darken(#9f9ab5, 5%);
a {
color: darken(#9f9ab5, 15%);
}
}
}
.line_content.match {
@include dark-diff-match-line;
}
......
app/assets/stylesheets/highlight/solarized_light.scss
......@@ -150,6 +150,17 @@ $solarized-light-il: #2aa198;
}
}
.diff-line-num {
&.is-over {
background-color: #ded7fc;
border-color: darken(#ded7fc, 5%);
a {
color: darken(#ded7fc, 15%);
}
}
}
.line_content.match {
@include matchLine;
}
......
app/assets/stylesheets/highlight/white.scss
......@@ -123,6 +123,15 @@ $white-gc-bg: #eaf2f5;
}
}
&.is-over {
background-color: #ded7fc;
border-color: darken(#ded7fc, 5%);
a {
color: darken(#ded7fc, 15%);
}
}
&.hll:not(.empty-cell) {
background-color: $line-number-select;
border-color: $line-select-yellow-dark;
......
app/assets/stylesheets/pages/diff.scss
......@@ -109,10 +109,6 @@
td.line_content.parallel {
width: 46%;
}
.add-diff-note {
margin-left: -65px;
}
}
.old_line,
......
app/assets/stylesheets/pages/notes.scss
......@@ -452,36 +452,35 @@ ul.notes {
* Line note button on the side of diffs
*/
.diff-file tr.line_holder {
@mixin show-add-diff-note {
display: inline-block;
}
.add-diff-note {
display: none;
margin-top: -2px;
border-radius: 50%;
background: $white-light;
padding: 2px 5px;
font-size: 12px;
color: $gl-link-color;
margin-left: -55px;
position: absolute;
z-index: 10;
width: 23px;
height: 23px;
border: 1px solid $border-color;
.add-diff-note {
margin-top: -8px;
border-radius: 40px;
background: $white-light;
padding: 4px;
font-size: 16px;
color: $gl-link-color;
margin-left: -56px;
position: absolute;
z-index: 10;
width: 32px;
// "hide" it by default
display: none;
&:hover {
background: $gl-info;
color: $white-light;
}
&:hover {
background: $gl-info;
color: $white-light;
@include show-add-diff-note;
}
&:active {
outline: 0;
}
}
// "show" the icon also if we just hover somewhere over the line
&:hover > td {
.diff-file {
.is-over {
.add-diff-note {
@include show-add-diff-note;
display: inline-block;
}
}
}
......
changelogs/unreleased/mr-diff-comment-button.yml 0 → 100644
---
title: Improved diff comment button UX
merge_request:
author:
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