summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/assets/javascripts/behaviors/shortcuts/shortcuts_issuable.js31
-rw-r--r--app/assets/javascripts/lib/utils/common_utils.js24
-rw-r--r--changelogs/unreleased/54032-reply-shortcut-only-discussion-text.yml5
-rw-r--r--spec/javascripts/behaviors/shortcuts/shortcuts_issuable_spec.js144
4 files changed, 195 insertions, 9 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/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/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();
+ });
+ });
});
});