diff options
author | Annabel Dunstone Gray <annabel.dunstone@gmail.com> | 2017-02-27 19:08:36 +0300 |
---|---|---|
committer | Annabel Dunstone Gray <annabel.dunstone@gmail.com> | 2017-02-27 19:08:36 +0300 |
commit | 46dfee555f9b9144d07735343ff56c87291b7efa (patch) | |
tree | 5fe81b54128b4c3e9a5233d4e4b262d488e8de89 /app | |
parent | 737ea1a896452a8d42dcdf4dd9bd1db5710be07d (diff) | |
parent | f0e51180de11b3fd2974f4e3bda0306fc25abb0d (diff) |
Merge branch 'mr-diff-comment-button' into 'master'
Improved the diff comment button UX
Closes #27543
See merge request !9448
Diffstat (limited to 'app')
-rw-r--r-- | app/assets/javascripts/files_comment_button.js | 71 | ||||
-rw-r--r-- | app/assets/stylesheets/highlight/dark.scss | 13 | ||||
-rw-r--r-- | app/assets/stylesheets/highlight/monokai.scss | 13 | ||||
-rw-r--r-- | app/assets/stylesheets/highlight/solarized_dark.scss | 13 | ||||
-rw-r--r-- | app/assets/stylesheets/highlight/solarized_light.scss | 13 | ||||
-rw-r--r-- | app/assets/stylesheets/highlight/white.scss | 11 | ||||
-rw-r--r-- | app/assets/stylesheets/pages/diff.scss | 8 | ||||
-rw-r--r-- | app/assets/stylesheets/pages/notes.scss | 51 |
8 files changed, 125 insertions, 68 deletions
diff --git a/app/assets/javascripts/files_comment_button.js b/app/assets/javascripts/files_comment_button.js index 698870d0ce1..6d86888dcb8 100644 --- a/app/assets/javascripts/files_comment_button.js +++ b/app/assets/javascripts/files_comment_button.js @@ -1,16 +1,16 @@ /* 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 */ (function() { + let $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,26 +27,29 @@ 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.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.hideButton = bind(this.hideButton, 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; + var $currentTarget, buttonParentElement, lineContentElement, textFileElement, $button; $currentTarget = $(e.currentTarget); - - buttonParentElement = this.getButtonParent($currentTarget); - if (!this.validateButtonParent(buttonParentElement)) return; 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); buttonParentElement.append(this.buildButton({ @@ -61,19 +64,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_CLASS}`).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, @@ -86,14 +86,14 @@ }; FilesCommentButton.prototype.getTextFileElement = function(hoveredElement) { - return $(hoveredElement.closest(TEXT_FILE_SELECTOR)); + return hoveredElement.closest(TEXT_FILE_SELECTOR); }; FilesCommentButton.prototype.getLineContent = function(hoveredElement) { if (hoveredElement.hasClass(LINE_CONTENT_CLASS)) { return hoveredElement; } - if (this.VIEW_TYPE === 'inline') { + if (!this.isParallelView) { return $(hoveredElement).closest(LINE_HOLDER_CLASS).find("." + LINE_CONTENT_CLASS); } else { return $(hoveredElement).next("." + LINE_CONTENT_CLASS); @@ -101,7 +101,7 @@ }; FilesCommentButton.prototype.getButtonParent = function(hoveredElement) { - if (this.VIEW_TYPE === 'inline') { + if (!this.isParallelView) { if (hoveredElement.hasClass(OLD_LINE_CLASS)) { return hoveredElement; } @@ -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) { - 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 +126,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; } diff --git a/app/assets/stylesheets/highlight/dark.scss b/app/assets/stylesheets/highlight/dark.scss index 6f2e746d4b0..9c76e58dfc8 100644 --- a/app/assets/stylesheets/highlight/dark.scss +++ b/app/assets/stylesheets/highlight/dark.scss @@ -20,6 +20,7 @@ $dark-highlight-bg: #ffe792; $dark-highlight-color: $black; $dark-pre-hll-bg: #373b41; $dark-hll-bg: #373b41; +$dark-over-bg: #9f9ab5; $dark-c: #969896; $dark-err: #c66; $dark-k: #b294bb; @@ -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 { @include dark-diff-match-line; } diff --git a/app/assets/stylesheets/highlight/monokai.scss b/app/assets/stylesheets/highlight/monokai.scss index 2144a5f7466..6488a099c74 100644 --- a/app/assets/stylesheets/highlight/monokai.scss +++ b/app/assets/stylesheets/highlight/monokai.scss @@ -13,6 +13,7 @@ $monokai-line-empty-bg: #49483e; $monokai-line-empty-border: darken($monokai-line-empty-bg, 15%); $monokai-diff-border: #808080; $monokai-highlight-bg: #ffe792; +$monokai-over-bg: #9f9ab5; $monokai-new-bg: rgba(166, 226, 46, 0.1); $monokai-new-idiff: rgba(166, 226, 46, 0.15); @@ -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 { @include dark-diff-match-line; } diff --git a/app/assets/stylesheets/highlight/solarized_dark.scss b/app/assets/stylesheets/highlight/solarized_dark.scss index 2cb1d18f12f..00079cc2b5b 100644 --- a/app/assets/stylesheets/highlight/solarized_dark.scss +++ b/app/assets/stylesheets/highlight/solarized_dark.scss @@ -17,6 +17,7 @@ $solarized-dark-line-color-new: #5a766c; $solarized-dark-line-color-old: #7a6c71; $solarized-dark-highlight: #094554; $solarized-dark-hll-bg: #174652; +$solarized-dark-over-bg: #9f9ab5; $solarized-dark-c: #586e75; $solarized-dark-err: #93a1a1; $solarized-dark-g: #93a1a1; @@ -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 { @include dark-diff-match-line; } diff --git a/app/assets/stylesheets/highlight/solarized_light.scss b/app/assets/stylesheets/highlight/solarized_light.scss index b72c4326730..2e3b68f1a58 100644 --- a/app/assets/stylesheets/highlight/solarized_light.scss +++ b/app/assets/stylesheets/highlight/solarized_light.scss @@ -18,6 +18,7 @@ $solarized-light-line-color-new: #a1a080; $solarized-light-line-color-old: #ad9186; $solarized-light-highlight: #eee8d5; $solarized-light-hll-bg: #ddd8c5; +$solarized-light-over-bg: #ded7fc; $solarized-light-c: #93a1a1; $solarized-light-err: #586e75; $solarized-light-g: #586e75; @@ -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 { @include matchLine; } diff --git a/app/assets/stylesheets/highlight/white.scss b/app/assets/stylesheets/highlight/white.scss index 398fbfd3b18..0eca30e570f 100644 --- a/app/assets/stylesheets/highlight/white.scss +++ b/app/assets/stylesheets/highlight/white.scss @@ -7,6 +7,7 @@ $white-code-color: $gl-text-color; $white-highlight: #fafe3d; $white-pre-hll-bg: #f8eec7; $white-hll-bg: #f8f8f8; +$white-over-bg: #ded7fc; $white-c: #998; $white-err: #a61717; $white-err-bg: #e3d2d2; @@ -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) { background-color: $line-number-select; border-color: $line-select-yellow-dark; diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 92d7772da57..339cdcde480 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -89,6 +89,10 @@ .diff-line-num { width: 50px; + + a { + transition: none; + } } .line_holder td { @@ -109,10 +113,6 @@ td.line_content.parallel { width: 46%; } - - .add-diff-note { - margin-left: -65px; - } } .old_line, diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index aa130a1abb0..00f5f2645b3 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -452,36 +452,37 @@ 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: 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 { - 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; + transform: scale(1.15); + } - &: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; } } } |