diff options
21 files changed, 221 insertions, 44 deletions
diff --git a/app/assets/javascripts/behaviors/shortcuts/shortcuts_issuable.js b/app/assets/javascripts/behaviors/shortcuts/shortcuts_issuable.js index 5e48bf5a35c..5f86fc9e63d 100644 --- a/app/assets/javascripts/behaviors/shortcuts/shortcuts_issuable.js +++ b/app/assets/javascripts/behaviors/shortcuts/shortcuts_issuable.js @@ -4,6 +4,7 @@ import _ from 'underscore'; import Sidebar from '../../right_sidebar'; import Shortcuts from './shortcuts'; import { CopyAsGFM } from '../markdown/copy_as_gfm'; +import { getSelectedFragment } from '~/lib/utils/common_utils'; export default class ShortcutsIssuable extends Shortcuts { constructor(isMergeRequest) { @@ -24,17 +25,43 @@ export default class ShortcutsIssuable extends Shortcuts { static replyWithSelectedText() { const $replyField = $('.js-main-target-form .js-vue-comment-form'); - const documentFragment = window.gl.utils.getSelectedFragment(); - if (!$replyField.length) { + if (!$replyField.length || $replyField.is(':hidden') /* Other tab selected in MR */) { return false; } + const documentFragment = getSelectedFragment(document.querySelector('.issuable-details')); + if (!documentFragment) { $replyField.focus(); return false; } + // Sanity check: Make sure the selected text comes from a discussion : it can either contain a message... + let foundMessage = !!documentFragment.querySelector('.md, .wiki'); + + // ... Or come from a message + if (!foundMessage) { + if (documentFragment.originalNodes) { + documentFragment.originalNodes.forEach(e => { + let node = e; + do { + // Text nodes don't define the `matches` method + if (node.matches && node.matches('.md, .wiki')) { + foundMessage = true; + } + node = node.parentNode; + } while (node && !foundMessage); + }); + } + + // If there is no message, just select the reply field + if (!foundMessage) { + $replyField.focus(); + return false; + } + } + const el = CopyAsGFM.transformGFMSelection(documentFragment.cloneNode(true)); const selected = CopyAsGFM.nodeToGFM(el); diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index e14fff7a610..3186ae9c133 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -226,7 +226,17 @@ export const getParameterByName = (name, urlToParse) => { return decodeURIComponent(results[2].replace(/\+/g, ' ')); }; -const handleSelectedRange = range => { +const handleSelectedRange = (range, restrictToNode) => { + // Make sure this range is within the restricting container + if (restrictToNode && !range.intersectsNode(restrictToNode)) return null; + + // If only a part of the range is within the wanted container, we need to restrict the range to it + if (restrictToNode && !restrictToNode.contains(range.commonAncestorContainer)) { + if (!restrictToNode.contains(range.startContainer)) range.setStart(restrictToNode, 0); + if (!restrictToNode.contains(range.endContainer)) + range.setEnd(restrictToNode, restrictToNode.childNodes.length); + } + const container = range.commonAncestorContainer; // add context to fragment if needed if (container.tagName === 'OL') { @@ -237,14 +247,22 @@ const handleSelectedRange = range => { return range.cloneContents(); }; -export const getSelectedFragment = () => { +export const getSelectedFragment = restrictToNode => { const selection = window.getSelection(); if (selection.rangeCount === 0) return null; + // Most usages of the selection only want text from a part of the page (e.g. discussion) + if (restrictToNode && !selection.containsNode(restrictToNode, true)) return null; + const documentFragment = document.createDocumentFragment(); + documentFragment.originalNodes = []; for (let i = 0; i < selection.rangeCount; i += 1) { const range = selection.getRangeAt(i); - documentFragment.appendChild(handleSelectedRange(range)); + const handledRange = handleSelectedRange(range, restrictToNode); + if (handledRange) { + documentFragment.appendChild(handledRange); + documentFragment.originalNodes.push(range.commonAncestorContainer); + } } if (documentFragment.textContent.length === 0) return null; diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index e261bd7c0ca..b3b99df5790 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -360,10 +360,6 @@ code { font-size: 95%; } -.git-revision-dropdown-toggle { - @extend .monospace; -} - .git-revision-dropdown .dropdown-content ul li a { @extend .ref-name; } diff --git a/app/assets/stylesheets/pages/editor.scss b/app/assets/stylesheets/pages/editor.scss index 04570c057d1..f46ff360496 100644 --- a/app/assets/stylesheets/pages/editor.scss +++ b/app/assets/stylesheets/pages/editor.scss @@ -32,7 +32,6 @@ .file-title { @extend .monospace; - line-height: 35px; padding-top: 7px; padding-bottom: 7px; @@ -48,13 +47,6 @@ margin-right: 10px; } - .editor-file-name { - @extend .monospace; - - float: left; - margin-right: 10px; - } - .new-file-name { display: inline-block; max-width: 420px; diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 14395cc59b0..fdd17af35fb 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -141,10 +141,6 @@ float: none; } - .api { - @extend .monospace; - } - .branch-commit { .ref-name { font-weight: $gl-font-weight-bold; diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index c7f986247bd..80ec390d18e 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -352,10 +352,6 @@ .mobile-git-clone { margin-top: $gl-padding-8; - - .dropdown-menu-inner-content { - @extend .monospace; - } } } diff --git a/app/views/peek/views/_gc.html.haml b/app/views/peek/views/_gc.html.haml index 9fc83e56ee7..2a586261ce1 100644 --- a/app/views/peek/views/_gc.html.haml +++ b/app/views/peek/views/_gc.html.haml @@ -1,7 +1,7 @@ - local_assigns.fetch(:view) %span.bold - %span{ title: 'Invoke Time', data: { defer_to: "#{view.defer_key}-gc_time" } }... + %span{ title: _('Invoke Time'), data: { defer_to: "#{view.defer_key}-gc_time" } }... \/ - %span{ title: 'Invoke Count', data: { defer_to: "#{view.defer_key}-invokes" } }... + %span{ title: _('Invoke Count'), data: { defer_to: "#{view.defer_key}-invokes" } }... gc diff --git a/app/views/projects/blob/_editor.html.haml b/app/views/projects/blob/_editor.html.haml index 24f256d083b..3c1f33ea95e 100644 --- a/app/views/projects/blob/_editor.html.haml +++ b/app/views/projects/blob/_editor.html.haml @@ -6,12 +6,12 @@ = sprite_icon('fork', size: 12) = ref - if current_action?(:edit) || current_action?(:update) - %span.editor-file-name + %span.pull-left.append-right-10 = text_field_tag 'file_path', (params[:file_path] || @path), class: 'form-control new-file-path js-file-path-name-input' - if current_action?(:new) || current_action?(:create) - %span.editor-file-name + %span.pull-left.append-right-10 \/ = text_field_tag 'file_name', params[:file_name], placeholder: "File name", required: true, class: 'form-control new-file-name js-file-path-name-input' diff --git a/app/views/projects/branches/new.html.haml b/app/views/projects/branches/new.html.haml index 500536a5dbc..8181ee9eea1 100644 --- a/app/views/projects/branches/new.html.haml +++ b/app/views/projects/branches/new.html.haml @@ -20,7 +20,7 @@ .col-sm-10.create-from .dropdown = hidden_field_tag :ref, default_ref - = button_tag type: 'button', title: default_ref, class: 'dropdown-menu-toggle wide js-branch-select git-revision-dropdown-toggle', required: true, data: { toggle: 'dropdown', selected: default_ref, field_name: 'ref' } do + = button_tag type: 'button', title: default_ref, class: 'dropdown-menu-toggle wide js-branch-select monospace', required: true, data: { toggle: 'dropdown', selected: default_ref, field_name: 'ref' } do .text-left.dropdown-toggle-text= default_ref = icon('chevron-down') = render 'shared/ref_dropdown', dropdown_class: 'wide' diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml index 0b10c66777a..44e9cb84341 100644 --- a/app/views/projects/ci/builds/_build.html.haml +++ b/app/views/projects/ci/builds/_build.html.haml @@ -60,7 +60,7 @@ - if pipeline.user = user_avatar(user: pipeline.user, size: 20) - else - %span.api API + %span.monospace API - if admin %td diff --git a/app/views/projects/compare/_form.html.haml b/app/views/projects/compare/_form.html.haml index d24ee4a3251..9744d293c8b 100644 --- a/app/views/projects/compare/_form.html.haml +++ b/app/views/projects/compare/_form.html.haml @@ -8,7 +8,7 @@ .input-group-text = s_("CompareBranches|Source") = hidden_field_tag :to, params[:to] - = button_tag type: 'button', title: params[:to], class: "form-control compare-dropdown-toggle js-compare-dropdown has-tooltip git-revision-dropdown-toggle", required: true, data: { refs_url: refs_project_path(@project), toggle: "dropdown", target: ".js-compare-to-dropdown", selected: params[:to], field_name: :to } do + = button_tag type: 'button', title: params[:to], class: "form-control compare-dropdown-toggle js-compare-dropdown has-tooltip monospace", required: true, data: { refs_url: refs_project_path(@project), toggle: "dropdown", target: ".js-compare-to-dropdown", selected: params[:to], field_name: :to } do .dropdown-toggle-text.str-truncated= params[:to] || _("Select branch/tag") = render 'shared/ref_dropdown' .compare-ellipsis.inline ... @@ -18,7 +18,7 @@ .input-group-text = s_("CompareBranches|Target") = hidden_field_tag :from, params[:from] - = button_tag type: 'button', title: params[:from], class: "form-control compare-dropdown-toggle js-compare-dropdown has-tooltip git-revision-dropdown-toggle", required: true, data: { refs_url: refs_project_path(@project), toggle: "dropdown", target: ".js-compare-from-dropdown", selected: params[:from], field_name: :from } do + = button_tag type: 'button', title: params[:from], class: "form-control compare-dropdown-toggle js-compare-dropdown has-tooltip monospace", required: true, data: { refs_url: refs_project_path(@project), toggle: "dropdown", target: ".js-compare-from-dropdown", selected: params[:from], field_name: :from } do .dropdown-toggle-text.str-truncated= params[:from] || _("Select branch/tag") = render 'shared/ref_dropdown' diff --git a/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml b/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml index 639efd34a74..7614d40ba1f 100644 --- a/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml +++ b/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml @@ -48,7 +48,7 @@ - if generic_commit_status.pipeline.user = user_avatar(user: generic_commit_status.pipeline.user, size: 20) - else - %span.api API + %span.monospace API - if admin %td diff --git a/app/views/projects/merge_requests/creations/_new_compare.html.haml b/app/views/projects/merge_requests/creations/_new_compare.html.haml index 1fd71a38472..11272a67f93 100644 --- a/app/views/projects/merge_requests/creations/_new_compare.html.haml +++ b/app/views/projects/merge_requests/creations/_new_compare.html.haml @@ -21,7 +21,7 @@ selected: f.object.source_project_id .merge-request-select.dropdown = f.hidden_field :source_branch - = dropdown_toggle f.object.source_branch || _("Select source branch"), { toggle: "dropdown", 'field-name': "#{f.object_name}[source_branch]", 'refs-url': refs_project_path(@source_project), selected: f.object.source_branch }, { toggle_class: "js-compare-dropdown js-source-branch git-revision-dropdown-toggle" } + = dropdown_toggle f.object.source_branch || _("Select source branch"), { toggle: "dropdown", 'field-name': "#{f.object_name}[source_branch]", 'refs-url': refs_project_path(@source_project), selected: f.object.source_branch }, { toggle_class: "js-compare-dropdown js-source-branch monospace" } .dropdown-menu.dropdown-menu-selectable.js-source-branch-dropdown.git-revision-dropdown = dropdown_title(_("Select source branch")) = dropdown_filter(_("Search branches")) @@ -49,7 +49,7 @@ selected: f.object.target_project_id .merge-request-select.dropdown = f.hidden_field :target_branch - = dropdown_toggle f.object.target_branch, { toggle: "dropdown", 'field-name': "#{f.object_name}[target_branch]", 'refs-url': refs_project_path(f.object.target_project), selected: f.object.target_branch }, { toggle_class: "js-compare-dropdown js-target-branch git-revision-dropdown-toggle" } + = dropdown_toggle f.object.target_branch, { toggle: "dropdown", 'field-name': "#{f.object_name}[target_branch]", 'refs-url': refs_project_path(f.object.target_project), selected: f.object.target_branch }, { toggle_class: "js-compare-dropdown js-target-branch monospace" } .dropdown-menu.dropdown-menu-selectable.js-target-branch-dropdown.git-revision-dropdown = dropdown_title(_("Select target branch")) = dropdown_filter(_("Search branches")) diff --git a/app/views/projects/pipelines/new.html.haml b/app/views/projects/pipelines/new.html.haml index 5b6823da1f6..f1cdc0a70dd 100644 --- a/app/views/projects/pipelines/new.html.haml +++ b/app/views/projects/pipelines/new.html.haml @@ -13,7 +13,7 @@ = f.label :ref, s_('Pipeline|Create for'), class: 'col-form-label' = hidden_field_tag 'pipeline[ref]', params[:ref] || @project.default_branch = dropdown_tag(params[:ref] || @project.default_branch, - options: { toggle_class: 'js-branch-select wide git-revision-dropdown-toggle', + options: { toggle_class: 'js-branch-select wide monospace', filter: true, dropdown_class: "dropdown-menu-selectable git-revision-dropdown", placeholder: s_("Pipeline|Search branches"), data: { selected: params[:ref] || @project.default_branch, field_name: 'pipeline[ref]' } }) .form-text.text-muted diff --git a/app/views/projects/protected_branches/shared/_dropdown.html.haml b/app/views/projects/protected_branches/shared/_dropdown.html.haml index b3d6068039a..67a6e8efae8 100644 --- a/app/views/projects/protected_branches/shared/_dropdown.html.haml +++ b/app/views/projects/protected_branches/shared/_dropdown.html.haml @@ -1,7 +1,7 @@ = f.hidden_field(:name) = dropdown_tag('Select branch or create wildcard', - options: { toggle_class: 'js-protected-branch-select js-filter-submit wide git-revision-dropdown-toggle qa-protected-branch-select', + options: { toggle_class: 'js-protected-branch-select js-filter-submit wide monospace qa-protected-branch-select', filter: true, dropdown_class: "dropdown-menu-selectable git-revision-dropdown qa-protected-branch-dropdown", placeholder: "Search protected branches", footer_content: true, data: { show_no: true, show_any: true, show_upcoming: true, diff --git a/app/views/projects/protected_tags/shared/_dropdown.html.haml b/app/views/projects/protected_tags/shared/_dropdown.html.haml index f0d7dcccd36..824a8604f6f 100644 --- a/app/views/projects/protected_tags/shared/_dropdown.html.haml +++ b/app/views/projects/protected_tags/shared/_dropdown.html.haml @@ -1,7 +1,7 @@ = f.hidden_field(:name) = dropdown_tag('Select tag or create wildcard', - options: { toggle_class: 'js-protected-tag-select js-filter-submit wide git-revision-dropdown-toggle', + options: { toggle_class: 'js-protected-tag-select js-filter-submit wide monospace', filter: true, dropdown_class: "dropdown-menu-selectable capitalize-header git-revision-dropdown", placeholder: "Search protected tags", footer_content: true, data: { show_no: true, show_any: true, show_upcoming: true, diff --git a/changelogs/unreleased/54032-reply-shortcut-only-discussion-text.yml b/changelogs/unreleased/54032-reply-shortcut-only-discussion-text.yml new file mode 100644 index 00000000000..5c1f6e74b39 --- /dev/null +++ b/changelogs/unreleased/54032-reply-shortcut-only-discussion-text.yml @@ -0,0 +1,5 @@ +--- +title: Make reply shortcut only quote selected discussion text +merge_request: 23096 +author: Thomas Pathier +type: fix diff --git a/changelogs/unreleased/gt-remove-instances-of-extend-monospace.yml b/changelogs/unreleased/gt-remove-instances-of-extend-monospace.yml new file mode 100644 index 00000000000..dc41de61046 --- /dev/null +++ b/changelogs/unreleased/gt-remove-instances-of-extend-monospace.yml @@ -0,0 +1,5 @@ +--- +title: Remove monospace extend +merge_request: 23089 +author: George Tsiolis +type: performance diff --git a/locale/gitlab.pot b/locale/gitlab.pot index abdb2084edd..52e15b846b7 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3490,6 +3490,12 @@ msgstr "" msgid "Invite" msgstr "" +msgid "Invoke Count" +msgstr "" + +msgid "Invoke Time" +msgstr "" + msgid "Issue" msgstr "" diff --git a/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js b/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js index bc25549cbed..b709b937180 100644 --- a/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js +++ b/spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js @@ -1,3 +1,7 @@ +/* eslint-disable + no-underscore-dangle +*/ + import $ from 'jquery'; import initCopyAsGFM from '~/behaviors/markdown/copy_as_gfm'; import ShortcutsIssuable from '~/behaviors/shortcuts/shortcuts_issuable'; @@ -27,13 +31,17 @@ describe('ShortcutsIssuable', function() { describe('replyWithSelectedText', () => { // Stub window.gl.utils.getSelectedFragment to return a node with the provided HTML. - const stubSelection = html => { - window.gl.utils.getSelectedFragment = () => { + const stubSelection = (html, invalidNode) => { + ShortcutsIssuable.__Rewire__('getSelectedFragment', () => { + const documentFragment = document.createDocumentFragment(); const node = document.createElement('div'); + node.innerHTML = html; + if (!invalidNode) node.className = 'md'; - return node; - }; + documentFragment.appendChild(node); + return documentFragment; + }); }; describe('with empty selection', () => { it('does not return an error', () => { @@ -105,5 +113,133 @@ describe('ShortcutsIssuable', function() { ); }); }); + + describe('with an invalid selection', () => { + beforeEach(() => { + stubSelection('<p>Selected text.</p>', true); + }); + + it('does not add anything to the input', () => { + ShortcutsIssuable.replyWithSelectedText(true); + + expect($(FORM_SELECTOR).val()).toBe(''); + }); + + it('triggers `focus`', () => { + const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus'); + ShortcutsIssuable.replyWithSelectedText(true); + + expect(spy).toHaveBeenCalled(); + }); + }); + + describe('with a semi-valid selection', () => { + beforeEach(() => { + stubSelection('<div class="md">Selected text.</div><p>Invalid selected text.</p>', true); + }); + + it('only adds the valid part to the input', () => { + ShortcutsIssuable.replyWithSelectedText(true); + + expect($(FORM_SELECTOR).val()).toBe('> Selected text.\n\n'); + }); + + it('triggers `focus`', () => { + const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus'); + ShortcutsIssuable.replyWithSelectedText(true); + + expect(spy).toHaveBeenCalled(); + }); + + it('triggers `input`', () => { + let triggered = false; + $(FORM_SELECTOR).on('input', () => { + triggered = true; + }); + + ShortcutsIssuable.replyWithSelectedText(true); + + expect(triggered).toBe(true); + }); + }); + + describe('with a selection in a valid block', () => { + beforeEach(() => { + ShortcutsIssuable.__Rewire__('getSelectedFragment', () => { + const documentFragment = document.createDocumentFragment(); + const node = document.createElement('div'); + const originalNode = document.createElement('body'); + originalNode.innerHTML = `<div class="issue"> + <div class="otherElem">Text...</div> + <div class="md"><p><em>Selected text.</em></p></div> + </div>`; + documentFragment.originalNodes = [originalNode.querySelector('em')]; + + node.innerHTML = '<em>Selected text.</em>'; + + documentFragment.appendChild(node); + + return documentFragment; + }); + }); + + it('adds the quoted selection to the input', () => { + ShortcutsIssuable.replyWithSelectedText(true); + + expect($(FORM_SELECTOR).val()).toBe('> _Selected text._\n\n'); + }); + + it('triggers `focus`', () => { + const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus'); + ShortcutsIssuable.replyWithSelectedText(true); + + expect(spy).toHaveBeenCalled(); + }); + + it('triggers `input`', () => { + let triggered = false; + $(FORM_SELECTOR).on('input', () => { + triggered = true; + }); + + ShortcutsIssuable.replyWithSelectedText(true); + + expect(triggered).toBe(true); + }); + }); + + describe('with a selection in an invalid block', () => { + beforeEach(() => { + ShortcutsIssuable.__Rewire__('getSelectedFragment', () => { + const documentFragment = document.createDocumentFragment(); + const node = document.createElement('div'); + const originalNode = document.createElement('body'); + originalNode.innerHTML = `<div class="issue"> + <div class="otherElem"><div><b>Selected text.</b></div></div> + <div class="md"><p><em>Valid text</em></p></div> + </div>`; + documentFragment.originalNodes = [originalNode.querySelector('b')]; + + node.innerHTML = '<b>Selected text.</b>'; + + documentFragment.appendChild(node); + + return documentFragment; + }); + }); + + it('does not add anything to the input', () => { + ShortcutsIssuable.replyWithSelectedText(true); + + expect($(FORM_SELECTOR).val()).toBe(''); + }); + + it('triggers `focus`', () => { + const spy = spyOn(document.querySelector(FORM_SELECTOR), 'focus'); + ShortcutsIssuable.replyWithSelectedText(true); + + expect(spy).toHaveBeenCalled(); + }); + }); }); }); diff --git a/spec/support/helpers/features/branches_helpers.rb b/spec/support/helpers/features/branches_helpers.rb index 3525d9a70a7..df88fd425c9 100644 --- a/spec/support/helpers/features/branches_helpers.rb +++ b/spec/support/helpers/features/branches_helpers.rb @@ -20,7 +20,7 @@ module Spec end def select_branch(branch_name) - find(".git-revision-dropdown-toggle").click + find(".js-branch-select").click page.within("#new-branch-form .dropdown-menu") do click_link(branch_name) |