From b87c53c72d7cb3226200b025ee7d7ca8fccece42 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 5 Dec 2017 01:35:37 -0600 Subject: Fix comment on image discussion icon alignment Fix https://gitlab.com/gitlab-org/gitlab-ce/issues/39608 Use SVG icons to avoid having to position things See https://gitlab.com/gitlab-org/gitlab-ce/issues/39608#note_50088917 --- app/assets/images/icon_image_comment.svg | 1 - app/assets/images/icon_image_comment@2x.svg | 1 - app/assets/images/icons.json | 2 +- app/assets/images/icons.svg | 2 +- .../illustrations/image_comment_light_cursor.svg | 1 + .../illustrations/image_comment_light_cursor@2x.svg | 1 + .../javascripts/image_diff/helpers/badge_helper.js | 7 ++----- app/assets/stylesheets/framework/buttons.scss | 4 ++-- app/assets/stylesheets/framework/variables.scss | 2 +- app/assets/stylesheets/pages/diff.scss | 19 ++++++++++++------- app/views/shared/notes/_note.html.haml | 4 ++-- ...8-comment-on-image-discussions-tab-alignment.yml | 5 +++++ package.json | 2 +- .../image_diff/helpers/badge_helper_spec.js | 9 +-------- yarn.lock | 21 ++++----------------- 15 files changed, 34 insertions(+), 47 deletions(-) delete mode 100644 app/assets/images/icon_image_comment.svg delete mode 100644 app/assets/images/icon_image_comment@2x.svg create mode 100644 app/assets/images/illustrations/image_comment_light_cursor.svg create mode 100644 app/assets/images/illustrations/image_comment_light_cursor@2x.svg create mode 100644 changelogs/unreleased/39608-comment-on-image-discussions-tab-alignment.yml diff --git a/app/assets/images/icon_image_comment.svg b/app/assets/images/icon_image_comment.svg deleted file mode 100644 index cf6cb972940..00000000000 --- a/app/assets/images/icon_image_comment.svg +++ /dev/null @@ -1 +0,0 @@ -cursor diff --git a/app/assets/images/icon_image_comment@2x.svg b/app/assets/images/icon_image_comment@2x.svg deleted file mode 100644 index 83be91d3705..00000000000 --- a/app/assets/images/icon_image_comment@2x.svg +++ /dev/null @@ -1 +0,0 @@ -cursor_2x diff --git a/app/assets/images/icons.json b/app/assets/images/icons.json index 4e967936ce0..68d6528758b 100644 --- a/app/assets/images/icons.json +++ b/app/assets/images/icons.json @@ -1 +1 @@ -{"iconCount":180,"spriteSize":82176,"icons":["abuse","account","admin","angle-double-left","angle-double-right","angle-down","angle-left","angle-right","angle-up","appearance","applications","approval","arrow-down","arrow-right","assignee","bold","book","branch","bullhorn","calendar","cancel","chart","chevron-down","chevron-left","chevron-right","chevron-up","clock","close","code","collapse","comment-dots","comment-next","comment","comments","commit","credit-card","cut","dashboard","disk","doc_code","doc_image","doc_text","double-headed-arrow","download","duplicate","earth","ellipsis_v","emoji_slightly_smiling_face","emoji_smile","emoji_smiley","epic","external-link","eye-slash","eye","file-addition","file-deletion","file-modified","filter","folder","fork","geo-nodes","git-merge","group","history","home","hook","hourglass","image-comment-dark","import","issue-block","issue-child","issue-close","issue-duplicate","issue-new","issue-open-m","issue-open","issue-parent","issues","italic","key-2","key","label","labels","leave","level-up","license","link","list-bulleted","list-numbered","location-dot","location","lock-open","lock","log","mail","menu","merge-request-close","messages","mobile-issue-close","monitor","more","notifications-off","notifications","overview","pencil-square","pencil","pipeline","play","plus-square-o","plus-square","plus","preferences","profile","project","push-rules","question-o","question","quote","redo","remove","repeat","retry","scale","screen-full","screen-normal","scroll_down","scroll_up","search","settings","shield","slight-frown","slight-smile","smile","smiley","snippet","spam","spinner","star-o","star","status_canceled_borderless","status_canceled","status_closed","status_created_borderless","status_created","status_failed_borderless","status_failed","status_manual_borderless","status_manual","status_notfound_borderless","status_open","status_pending_borderless","status_pending","status_running_borderless","status_running","status_skipped_borderless","status_skipped","status_success_borderless","status_success_solid","status_success","status_warning_borderless","status_warning","stop","task-done","template","terminal","thumb-down","thumb-up","thumbtack","timer","todo-add","todo-done","token","unapproval","unassignee","unlink","user","users","volume-up","warning","work"]} \ No newline at end of file +{"iconCount":181,"spriteSize":81482,"icons":["abuse","account","admin","angle-double-left","angle-double-right","angle-down","angle-left","angle-right","angle-up","appearance","applications","approval","arrow-down","arrow-right","assignee","bold","book","branch","bullhorn","calendar","cancel","chart","chevron-down","chevron-left","chevron-right","chevron-up","clock","close","code","collapse","comment-dots","comment-next","comment","comments","commit","credit-card","cut","dashboard","disk","doc_code","doc_image","doc_text","double-headed-arrow","download","duplicate","earth","ellipsis_v","emoji_slightly_smiling_face","emoji_smile","emoji_smiley","epic","external-link","eye-slash","eye","file-addition","file-deletion","file-modified","filter","folder","fork","geo-nodes","git-merge","group","history","home","hook","hourglass","image-comment-dark","image-comment-light","import","issue-block","issue-child","issue-close","issue-duplicate","issue-new","issue-open-m","issue-open","issue-parent","issues","italic","key-2","key","label","labels","leave","level-up","license","link","list-bulleted","list-numbered","location-dot","location","lock-open","lock","log","mail","menu","merge-request-close","messages","mobile-issue-close","monitor","more","notifications-off","notifications","overview","pencil-square","pencil","pipeline","play","plus-square-o","plus-square","plus","preferences","profile","project","push-rules","question-o","question","quote","redo","remove","repeat","retry","scale","screen-full","screen-normal","scroll_down","scroll_up","search","settings","shield","slight-frown","slight-smile","smile","smiley","snippet","spam","spinner","star-o","star","status_canceled_borderless","status_canceled","status_closed","status_created_borderless","status_created","status_failed_borderless","status_failed","status_manual_borderless","status_manual","status_notfound_borderless","status_open","status_pending_borderless","status_pending","status_running_borderless","status_running","status_skipped_borderless","status_skipped","status_success_borderless","status_success_solid","status_success","status_warning_borderless","status_warning","stop","task-done","template","terminal","thumb-down","thumb-up","thumbtack","timer","todo-add","todo-done","token","unapproval","unassignee","unlink","user","users","volume-up","warning","work"]} \ No newline at end of file diff --git a/app/assets/images/icons.svg b/app/assets/images/icons.svg index 77ce6b2d89f..fd8f7862911 100644 --- a/app/assets/images/icons.svg +++ b/app/assets/images/icons.svg @@ -1 +1 @@ -cursor_active \ No newline at end of file + \ No newline at end of file diff --git a/app/assets/images/illustrations/image_comment_light_cursor.svg b/app/assets/images/illustrations/image_comment_light_cursor.svg new file mode 100644 index 00000000000..ac712ea0c96 --- /dev/null +++ b/app/assets/images/illustrations/image_comment_light_cursor.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/app/assets/images/illustrations/image_comment_light_cursor@2x.svg b/app/assets/images/illustrations/image_comment_light_cursor@2x.svg new file mode 100644 index 00000000000..02943acd9d7 --- /dev/null +++ b/app/assets/images/illustrations/image_comment_light_cursor@2x.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/app/assets/javascripts/image_diff/helpers/badge_helper.js b/app/assets/javascripts/image_diff/helpers/badge_helper.js index 6a6a668308d..eddaeda9578 100644 --- a/app/assets/javascripts/image_diff/helpers/badge_helper.js +++ b/app/assets/javascripts/image_diff/helpers/badge_helper.js @@ -19,12 +19,9 @@ export function addImageBadge(containerEl, { coordinate, badgeText, noteId }) { } export function addImageCommentBadge(containerEl, { coordinate, noteId }) { - const buttonEl = createImageBadge(noteId, coordinate, ['image-comment-badge', 'inverted']); - const iconEl = document.createElement('i'); - iconEl.className = 'fa fa-comment-o'; - iconEl.setAttribute('aria-label', 'comment'); + const buttonEl = createImageBadge(noteId, coordinate, ['image-comment-badge']); + buttonEl.innerHTML = gl.utils.spriteIcon('image-comment-dark'); - buttonEl.appendChild(iconEl); containerEl.appendChild(buttonEl); } diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss index 3f630f82e29..fcc420923f9 100644 --- a/app/assets/stylesheets/framework/buttons.scss +++ b/app/assets/stylesheets/framework/buttons.scss @@ -4,8 +4,8 @@ padding: 1px 5px; font-size: 12px; color: $blue-500; - width: 23px; - height: 23px; + width: 24px; + height: 24px; border: 1px solid $blue-500; &:hover, diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 4f99c27eff1..1313d6388cf 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -722,7 +722,7 @@ $issuable-warning-icon-margin: 4px; Image Commenting cursor */ $image-comment-cursor-left-offset: 12; -$image-comment-cursor-top-offset: 30; +$image-comment-cursor-top-offset: 12; /* Popup diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 848d7f144dc..71a6c7a2bf9 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -732,18 +732,18 @@ .frame.click-to-comment { position: relative; - cursor: image-url('icon_image_comment.svg') + cursor: image-url('illustrations/image_comment_light_cursor.svg') $image-comment-cursor-left-offset $image-comment-cursor-top-offset, auto; // Retina cursor - cursor: -webkit-image-set(image-url('icon_image_comment.svg') 1x, image-url('icon_image_comment@2x.svg') 2x) + cursor: -webkit-image-set(image-url('illustrations/image_comment_light_cursor.svg') 1x, image-url('illustrations/image_comment_light_cursor@2x.svg') 2x) $image-comment-cursor-left-offset $image-comment-cursor-top-offset, auto; .comment-indicator { position: absolute; padding: 0; width: (2px * $image-comment-cursor-left-offset); - height: (1px * $image-comment-cursor-top-offset); + height: (2px * $image-comment-cursor-top-offset); // center the indicator to match the top left click region margin-top: (-1px * $image-comment-cursor-top-offset) + 2; margin-left: (-1px * $image-comment-cursor-left-offset) + 1; @@ -778,15 +778,20 @@ .frame .badge, .frame .image-comment-badge { // Center align badges on the frame - transform: translate3d(-50%, -50%, 0); + transform: translate(-50%, -50%); } .image-comment-badge { - @include btn-comment-icon; position: absolute; + width: 24px; + height: 24px; + padding: 0; + background: none; + border: 0; - &.inverted { - border-color: $white-light; + > svg { + width: 100%; + height: 100%; } } diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml index c978d9e4821..98e0161f7d1 100644 --- a/app/views/shared/notes/_note.html.haml +++ b/app/views/shared/notes/_note.html.haml @@ -20,8 +20,8 @@ - if note.is_a?(DiffNote) && note.on_image? - if show_image_comment_badge && note_counter == 0 -# Only show this for the first comment in the discussion - %span.image-comment-badge.inverted - = icon('comment-o') + %span.image-comment-badge + = sprite_icon('image-comment-dark') - elsif note_counter == 0 - counter = badge_counter if local_assigns[:badge_counter] - badge_class = "hidden" if @fresh_discussion || counter.nil? diff --git a/changelogs/unreleased/39608-comment-on-image-discussions-tab-alignment.yml b/changelogs/unreleased/39608-comment-on-image-discussions-tab-alignment.yml new file mode 100644 index 00000000000..5021fe88caf --- /dev/null +++ b/changelogs/unreleased/39608-comment-on-image-discussions-tab-alignment.yml @@ -0,0 +1,5 @@ +--- +title: Update comment on image cursor and icons +merge_request: 15760 +author: +type: fixed diff --git a/package.json b/package.json index c0a4db122bd..a08f4784931 100644 --- a/package.json +++ b/package.json @@ -80,7 +80,7 @@ "worker-loader": "^1.1.0" }, "devDependencies": { - "@gitlab-org/gitlab-svgs": "^1.1.1", + "@gitlab-org/gitlab-svgs": "^1.2.0", "babel-plugin-istanbul": "^4.1.5", "eslint": "^3.10.1", "eslint-config-airbnb-base": "^10.0.1", diff --git a/spec/javascripts/image_diff/helpers/badge_helper_spec.js b/spec/javascripts/image_diff/helpers/badge_helper_spec.js index fb9c7e59031..ce3add1fd90 100644 --- a/spec/javascripts/image_diff/helpers/badge_helper_spec.js +++ b/spec/javascripts/image_diff/helpers/badge_helper_spec.js @@ -89,15 +89,8 @@ describe('badge helper', () => { }); it('should create icon comment button', () => { - const iconEl = buttonEl.querySelector('i'); + const iconEl = buttonEl.querySelector('svg'); expect(iconEl).toBeDefined(); - expect(iconEl.classList.contains('fa')).toEqual(true); - expect(iconEl.classList.contains('fa-comment-o')).toEqual(true); - }); - - it('should have .image-comment-badge.inverted in button class', () => { - expect(buttonEl.classList.contains('image-comment-badge')).toEqual(true); - expect(buttonEl.classList.contains('inverted')).toEqual(true); }); }); diff --git a/yarn.lock b/yarn.lock index 69e8f8a0647..5ff75b161a4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -54,9 +54,9 @@ lodash "^4.2.0" to-fast-properties "^2.0.0" -"@gitlab-org/gitlab-svgs@^1.1.1": - version "1.1.3" - resolved "https://registry.yarnpkg.com/@gitlab-org/gitlab-svgs/-/gitlab-svgs-1.1.3.tgz#2beead1bcdd83e7400de29b01014bf17bf76318e" +"@gitlab-org/gitlab-svgs@^1.2.0": + version "1.2.0" + resolved "https://registry.yarnpkg.com/@gitlab-org/gitlab-svgs/-/gitlab-svgs-1.2.0.tgz#0b1181b5d2dd56a959528529750417c5f49159ca" "@types/jquery@^2.0.40": version "2.0.48" @@ -116,16 +116,7 @@ ajv@^4.7.0, ajv@^4.9.1: co "^4.6.0" json-stable-stringify "^1.0.1" -ajv@^5.0.0: - version "5.4.0" - resolved "https://registry.yarnpkg.com/ajv/-/ajv-5.4.0.tgz#32d1cf08dbc80c432f426f12e10b2511f6b46474" - dependencies: - co "^4.6.0" - fast-deep-equal "^1.0.0" - fast-json-stable-stringify "^2.0.0" - json-schema-traverse "^0.3.0" - -ajv@^5.1.5: +ajv@^5.0.0, ajv@^5.1.5: version "5.2.2" resolved "https://registry.yarnpkg.com/ajv/-/ajv-5.2.2.tgz#47c68d69e86f5d953103b0074a9430dc63da5e39" dependencies: @@ -2540,10 +2531,6 @@ fast-deep-equal@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/fast-deep-equal/-/fast-deep-equal-1.0.0.tgz#96256a3bc975595eb36d82e9929d060d893439ff" -fast-json-stable-stringify@^2.0.0: - version "2.0.0" - resolved "https://registry.yarnpkg.com/fast-json-stable-stringify/-/fast-json-stable-stringify-2.0.0.tgz#d5142c0caee6b1189f87d3a76111064f86c8bbf2" - fast-levenshtein@~2.0.4: version "2.0.6" resolved "https://registry.yarnpkg.com/fast-levenshtein/-/fast-levenshtein-2.0.6.tgz#3d8a5c66883a16a30ca8643e851f19baa7797917" -- cgit v1.2.1