Skip to content

G
gitlab-ce
Commit 46dfee55 authored by Annabel Dunstone Gray's avatar Annabel Dunstone Gray
Browse files
Options

Merge branch 'mr-diff-comment-button' into 'master' 

Improved the diff comment button UX

Closes #27543

See merge request !9448
parents 737ea1a8 f0e51180
Hide whitespace changes
Inline Side-by-side
Showing with 129 additions and 68 deletions
app/assets/javascripts/files_comment_button.js
View file @ 46dfee55
/* 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 */ /* 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 FilesCommentButton */
/* global notes */
(function() { (function() {
let $commentButtonTemplate;
var bind = function(fn, me) { return function() { return fn.apply(me, arguments); }; }; var bind = function(fn, me) { return function() { return fn.apply(me, arguments); }; };
this.FilesCommentButton = (function() { 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_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_HOLDER_CLASS = '.line_holder';
LINE_NUMBER_CLASS = 'diff-line-num'; LINE_NUMBER_CLASS = 'diff-line-num';
...@@ -27,26 +27,29 @@ ...@@ -27,26 +27,29 @@
TEXT_FILE_SELECTOR = '.text-file'; TEXT_FILE_SELECTOR = '.text-file';
DEBOUNCE_TIMEOUT_DURATION = 100;
function FilesCommentButton(filesContainerElement) { function FilesCommentButton(filesContainerElement) {
var debounce;
this.filesContainerElement = filesContainerElement;
this.destroy = bind(this.destroy, this);
this.render = bind(this.render, this); this.render = bind(this.render, this);
this.VIEW_TYPE = $('input#view[type=hidden]').val(); this.hideButton = bind(this.hideButton, this);
debounce = _.debounce(this.render, DEBOUNCE_TIMEOUT_DURATION); this.isParallelView = notes.isParallelView();
$(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); filesContainerElement.on('mouseover', LINE_COLUMN_CLASSES, this.render)
.on('mouseleave', LINE_COLUMN_CLASSES, this.hideButton);
} }
FilesCommentButton.prototype.render = function(e) { FilesCommentButton.prototype.render = function(e) {
var $currentTarget, buttonParentElement, lineContentElement, textFileElement; var $currentTarget, buttonParentElement, lineContentElement, textFileElement, $button;
$currentTarget = $(e.currentTarget); $currentTarget = $(e.currentTarget);
buttonParentElement = this.getButtonParent($currentTarget);
if (!this.validateButtonParent(buttonParentElement)) return;
lineContentElement = this.getLineContent($currentTarget); lineContentElement = this.getLineContent($currentTarget);
if (!this.validateLineContent(lineContentElement)) return; 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;
}
textFileElement = this.getTextFileElement($currentTarget); textFileElement = this.getTextFileElement($currentTarget);
buttonParentElement.append(this.buildButton({ buttonParentElement.append(this.buildButton({
...@@ -61,19 +64,16 @@ ...@@ -61,19 +64,16 @@
})); }));
}; };
FilesCommentButton.prototype.destroy = function(e) { FilesCommentButton.prototype.hideButton = function(e) {
if (this.isMovingToSameType(e)) { var $currentTarget = $(e.currentTarget);
return; var buttonParentElement = this.getButtonParent($currentTarget);
}
$(COMMENT_BUTTON_CLASS, this.getButtonParent($(e.currentTarget))).remove(); buttonParentElement.removeClass('is-over')
.nextUntil(`.${LINE_CONTENT_CLASS}`).removeClass('is-over');
}; };
FilesCommentButton.prototype.buildButton = function(buttonAttributes) { FilesCommentButton.prototype.buildButton = function(buttonAttributes) {
var initializedButtonTemplate; return $commentButtonTemplate.clone().attr({
initializedButtonTemplate = COMMENT_BUTTON_TEMPLATE({
COMMENT_BUTTON_CLASS: COMMENT_BUTTON_CLASS.substr(1)
});
return $(initializedButtonTemplate).attr({
'data-noteable-type': buttonAttributes.noteableType, 'data-noteable-type': buttonAttributes.noteableType,
'data-noteable-id': buttonAttributes.noteableID, 'data-noteable-id': buttonAttributes.noteableID,
'data-commit-id': buttonAttributes.commitID, 'data-commit-id': buttonAttributes.commitID,
...@@ -86,14 +86,14 @@ ...@@ -86,14 +86,14 @@
}; };
FilesCommentButton.prototype.getTextFileElement = function(hoveredElement) { FilesCommentButton.prototype.getTextFileElement = function(hoveredElement) {
return $(hoveredElement.closest(TEXT_FILE_SELECTOR)); return hoveredElement.closest(TEXT_FILE_SELECTOR);
}; };
FilesCommentButton.prototype.getLineContent = function(hoveredElement) { FilesCommentButton.prototype.getLineContent = function(hoveredElement) {
if (hoveredElement.hasClass(LINE_CONTENT_CLASS)) { if (hoveredElement.hasClass(LINE_CONTENT_CLASS)) {
return hoveredElement; return hoveredElement;
} }
if (this.VIEW_TYPE === 'inline') { if (!this.isParallelView) {
return $(hoveredElement).closest(LINE_HOLDER_CLASS).find("." + LINE_CONTENT_CLASS); return $(hoveredElement).closest(LINE_HOLDER_CLASS).find("." + LINE_CONTENT_CLASS);
} else { } else {
return $(hoveredElement).next("." + LINE_CONTENT_CLASS); return $(hoveredElement).next("." + LINE_CONTENT_CLASS);
...@@ -101,7 +101,7 @@ ...@@ -101,7 +101,7 @@
}; };
FilesCommentButton.prototype.getButtonParent = function(hoveredElement) { FilesCommentButton.prototype.getButtonParent = function(hoveredElement) {
if (this.VIEW_TYPE === 'inline') { if (!this.isParallelView) {
if (hoveredElement.hasClass(OLD_LINE_CLASS)) { if (hoveredElement.hasClass(OLD_LINE_CLASS)) {
return hoveredElement; return hoveredElement;
} }
...@@ -114,17 +114,8 @@ ...@@ -114,17 +114,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) { 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) { FilesCommentButton.prototype.validateLineContent = function(lineContentElement) {
...@@ -135,6 +126,8 @@ ...@@ -135,6 +126,8 @@
})(); })();
$.fn.filesCommentButton = function() { $.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))) { if (!(this && (this.parent().data('can-create-note') != null))) {
return; return;
} }
......
app/assets/stylesheets/highlight/dark.scss
View file @ 46dfee55
...@@ -20,6 +20,7 @@ $dark-highlight-bg: #ffe792; ...@@ -20,6 +20,7 @@ $dark-highlight-bg: #ffe792;
$dark-highlight-color: $black; $dark-highlight-color: $black;
$dark-pre-hll-bg: #373b41; $dark-pre-hll-bg: #373b41;
$dark-hll-bg: #373b41; $dark-hll-bg: #373b41;
$dark-over-bg: #9f9ab5;
$dark-c: #969896; $dark-c: #969896;
$dark-err: #c66; $dark-err: #c66;
$dark-k: #b294bb; $dark-k: #b294bb;
...@@ -139,6 +140,18 @@ $dark-il: #de935f; ...@@ -139,6 +140,18 @@ $dark-il: #de935f;
} }
} }
.diff-line-num {
&.is-over,
&.hll:not(.empty-cell).is-over {
background-color: $dark-over-bg;
border-color: darken($dark-over-bg, 5%);
a {
color: darken($dark-over-bg, 15%);
}
}
}
.line_content.match { .line_content.match {
@include dark-diff-match-line; @include dark-diff-match-line;
} }
......
app/assets/stylesheets/highlight/monokai.scss
View file @ 46dfee55
...@@ -13,6 +13,7 @@ $monokai-line-empty-bg: #49483e; ...@@ -13,6 +13,7 @@ $monokai-line-empty-bg: #49483e;
$monokai-line-empty-border: darken($monokai-line-empty-bg, 15%); $monokai-line-empty-border: darken($monokai-line-empty-bg, 15%);
$monokai-diff-border: #808080; $monokai-diff-border: #808080;
$monokai-highlight-bg: #ffe792; $monokai-highlight-bg: #ffe792;
$monokai-over-bg: #9f9ab5;
$monokai-new-bg: rgba(166, 226, 46, 0.1); $monokai-new-bg: rgba(166, 226, 46, 0.1);
$monokai-new-idiff: rgba(166, 226, 46, 0.15); $monokai-new-idiff: rgba(166, 226, 46, 0.15);
...@@ -139,6 +140,18 @@ $monokai-gi: #a6e22e; ...@@ -139,6 +140,18 @@ $monokai-gi: #a6e22e;
} }
} }
.diff-line-num {
&.is-over,
&.hll:not(.empty-cell).is-over {
background-color: $monokai-over-bg;
border-color: darken($monokai-over-bg, 5%);
a {
color: darken($monokai-over-bg, 15%);
}
}
}
.line_content.match { .line_content.match {
@include dark-diff-match-line; @include dark-diff-match-line;
} }
......
app/assets/stylesheets/highlight/solarized_dark.scss
View file @ 46dfee55
...@@ -17,6 +17,7 @@ $solarized-dark-line-color-new: #5a766c; ...@@ -17,6 +17,7 @@ $solarized-dark-line-color-new: #5a766c;
$solarized-dark-line-color-old: #7a6c71; $solarized-dark-line-color-old: #7a6c71;
$solarized-dark-highlight: #094554; $solarized-dark-highlight: #094554;
$solarized-dark-hll-bg: #174652; $solarized-dark-hll-bg: #174652;
$solarized-dark-over-bg: #9f9ab5;
$solarized-dark-c: #586e75; $solarized-dark-c: #586e75;
$solarized-dark-err: #93a1a1; $solarized-dark-err: #93a1a1;
$solarized-dark-g: #93a1a1; $solarized-dark-g: #93a1a1;
...@@ -143,6 +144,18 @@ $solarized-dark-il: #2aa198; ...@@ -143,6 +144,18 @@ $solarized-dark-il: #2aa198;
} }
} }
.diff-line-num {
&.is-over,
&.hll:not(.empty-cell).is-over {
background-color: $solarized-dark-over-bg;
border-color: darken($solarized-dark-over-bg, 5%);
a {
color: darken($solarized-dark-over-bg, 15%);
}
}
}
.line_content.match { .line_content.match {
@include dark-diff-match-line; @include dark-diff-match-line;
} }
......
app/assets/stylesheets/highlight/solarized_light.scss
View file @ 46dfee55
...@@ -18,6 +18,7 @@ $solarized-light-line-color-new: #a1a080; ...@@ -18,6 +18,7 @@ $solarized-light-line-color-new: #a1a080;
$solarized-light-line-color-old: #ad9186; $solarized-light-line-color-old: #ad9186;
$solarized-light-highlight: #eee8d5; $solarized-light-highlight: #eee8d5;
$solarized-light-hll-bg: #ddd8c5; $solarized-light-hll-bg: #ddd8c5;
$solarized-light-over-bg: #ded7fc;
$solarized-light-c: #93a1a1; $solarized-light-c: #93a1a1;
$solarized-light-err: #586e75; $solarized-light-err: #586e75;
$solarized-light-g: #586e75; $solarized-light-g: #586e75;
...@@ -150,6 +151,18 @@ $solarized-light-il: #2aa198; ...@@ -150,6 +151,18 @@ $solarized-light-il: #2aa198;
} }
} }
.diff-line-num {
&.is-over,
&.hll:not(.empty-cell).is-over {
background-color: $solarized-light-over-bg;
border-color: darken($solarized-light-over-bg, 5%);
a {
color: darken($solarized-light-over-bg, 15%);
}
}
}
.line_content.match { .line_content.match {
@include matchLine; @include matchLine;
} }
......
app/assets/stylesheets/highlight/white.scss
View file @ 46dfee55
...@@ -7,6 +7,7 @@ $white-code-color: $gl-text-color; ...@@ -7,6 +7,7 @@ $white-code-color: $gl-text-color;
$white-highlight: #fafe3d; $white-highlight: #fafe3d;
$white-pre-hll-bg: #f8eec7; $white-pre-hll-bg: #f8eec7;
$white-hll-bg: #f8f8f8; $white-hll-bg: #f8f8f8;
$white-over-bg: #ded7fc;
$white-c: #998; $white-c: #998;
$white-err: #a61717; $white-err: #a61717;
$white-err-bg: #e3d2d2; $white-err-bg: #e3d2d2;
...@@ -123,6 +124,16 @@ $white-gc-bg: #eaf2f5; ...@@ -123,6 +124,16 @@ $white-gc-bg: #eaf2f5;
} }
} }
&.is-over,
&.hll:not(.empty-cell).is-over {
background-color: $white-over-bg;
border-color: darken($white-over-bg, 5%);
a {
color: darken($white-over-bg, 15%);
}
}
&.hll:not(.empty-cell) { &.hll:not(.empty-cell) {
background-color: $line-number-select; background-color: $line-number-select;
border-color: $line-select-yellow-dark; border-color: $line-select-yellow-dark;
......
app/assets/stylesheets/pages/diff.scss
View file @ 46dfee55
...@@ -89,6 +89,10 @@ ...@@ -89,6 +89,10 @@
.diff-line-num { .diff-line-num {
width: 50px; width: 50px;
a {
transition: none;
}
} }
.line_holder td { .line_holder td {
...@@ -109,10 +113,6 @@ ...@@ -109,10 +113,6 @@
td.line_content.parallel { td.line_content.parallel {
width: 46%; width: 46%;
} }
.add-diff-note {
margin-left: -65px;
}
} }
.old_line, .old_line,
......
app/assets/stylesheets/pages/notes.scss
View file @ 46dfee55
...@@ -452,36 +452,37 @@ ul.notes { ...@@ -452,36 +452,37 @@ ul.notes {
* Line note button on the side of diffs * Line note button on the side of diffs
*/ */
.diff-file tr.line_holder { .add-diff-note {
@mixin show-add-diff-note { display: none;
display: inline-block; margin-top: -2px;
} border-radius: 50%;
background: $white-light;
padding: 1px 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;
transition: transform .1s ease-in-out;
.add-diff-note { &:hover {
margin-top: -8px; background: $gl-info;
border-radius: 40px; color: $white-light;
background: $white-light; transform: scale(1.15);
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 { &:active {
background: $gl-info; outline: 0;
color: $white-light;
@include show-add-diff-note;
}
} }
}
// "show" the icon also if we just hover somewhere over the line .diff-file {
&:hover > td { .is-over {
.add-diff-note { .add-diff-note {
@include show-add-diff-note; display: inline-block;
} }
} }
} }
......
changelogs/unreleased/mr-diff-comment-button.yml 0 → 100644
View file @ 46dfee55
---
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7