Rich text editor: Text is not converted to a reference automatically after pressing spacebar
Problem
Typing a username then pressing spacebar or clicking somewhere else in the Rich Text Editor textarea, isn't converted into a user mention. Similar issue exists for other references, as outlined in the table below.
Example video
Background
In rich text editor, we currently support autocompleting 8 kinds of references:
Reference Type | Example | Autocomplete dropdown supported | Autocomplete after space supported |
---|---|---|---|
Issue | #1 (closed) | Yes | Yes |
Merge Request | !1 (merged) | Yes | Yes |
Epic | &1 | Yes | Yes |
Snippet | $2490900 | Yes | No |
Milestone | %16.6 | Yes | No |
User | @himkp | Yes | No |
Label | frontend | Yes | No |
Vulnerability | [vulnerability:99022839] | Yes | No |
Support for autocomplete after space was added in !117749 (merged), but that MR only added it for issues, merge requests and epics. We now need to complete the functionality for all the remaining autocomplete reference types.
Implementation Guide
Code
The code exists in the following files (similar to the MR mentioned above):
- app/assets/javascripts/content_editor/extensions/reference.js
- app/assets/javascripts/content_editor/services/asset_resolver.js
Tests need to be updated in:
- spec/frontend/content_editor/extensions/reference_spec.js
- spec/frontend/content_editor/services/asset_resolver_spec.js
Patch
Below patch can help resolve this issue and add test cases, but still would need a final check to find any unforeseen bugs:
View patch
diff --git a/app/assets/javascripts/content_editor/extensions/reference.js b/app/assets/javascripts/content_editor/extensions/reference.js
index fd248709b5ab..0c385481ac52 100644
--- a/app/assets/javascripts/content_editor/extensions/reference.js
+++ b/app/assets/javascripts/content_editor/extensions/reference.js
@@ -72,11 +72,20 @@ export default Node.create({
addInputRules() {
const { editor } = this;
const { assetResolver } = this.options;
- const referenceInputRegex = /(?:^|\s)([\w/]*([!&#])\d+(\+?s?))(?:\s|\n)/m;
+ const referenceInputRegex = /(?:^|\s)([\w/]*([#!&%$@~]|\[vulnerability:)[\w.]+(\+?s?\]?))(?:\s|\n)/m;
const referenceTypes = {
'#': 'issue',
'!': 'merge_request',
'&': 'epic',
+ '%': 'milestone',
+ $: 'snippet',
+ '@': 'user',
+ '~': 'label',
+ '[vulnerability:': 'vulnerability',
+ };
+ const nodeTypes = {
+ label: editor.schema.nodes.referenceLabel,
+ default: editor.schema.nodes.reference,
};
return [
@@ -91,22 +100,26 @@ export default Node.create({
text,
expandedText,
fullyExpandedText,
+ backgroundColor,
} = await assetResolver.resolveReference(referenceId);
if (!text) return;
let referenceText = text;
- if (expansionType === '+') referenceText = expandedText;
- if (expansionType === '+s') referenceText = fullyExpandedText;
+ if (expansionType === '+') referenceText = expandedText || text;
+ if (expansionType === '+s') referenceText = fullyExpandedText || text;
const position = findReference(editor, referenceId);
if (!position) return;
+ const nodeType = nodeTypes[referenceType] || nodeTypes.default;
+
editor.view.dispatch(
editor.state.tr.replaceWith(position, position + referenceId.length, [
- this.type.create({
+ nodeType.create({
referenceType,
originalText: referenceId,
+ color: backgroundColor,
href,
text: referenceText,
}),
diff --git a/app/assets/javascripts/content_editor/services/asset_resolver.js b/app/assets/javascripts/content_editor/services/asset_resolver.js
index 0d4396fc176c..07a69db74284 100644
--- a/app/assets/javascripts/content_editor/services/asset_resolver.js
+++ b/app/assets/javascripts/content_editor/services/asset_resolver.js
@@ -27,10 +27,11 @@ export default class AssetResolver {
if (!a.length) return {};
return {
- href: a[0].getAttribute('href'),
- text: a[0].textContent,
- expandedText: a[1].textContent,
- fullyExpandedText: a[2].textContent,
+ href: a[0]?.getAttribute('href'),
+ text: a[0]?.textContent,
+ expandedText: a[1]?.textContent,
+ fullyExpandedText: a[2]?.textContent,
+ backgroundColor: a[0]?.firstElementChild?.style.backgroundColor,
};
});
diff --git a/spec/frontend/content_editor/extensions/reference_spec.js b/spec/frontend/content_editor/extensions/reference_spec.js
index c25c7c41d755..d4b07d5127e8 100644
--- a/spec/frontend/content_editor/extensions/reference_spec.js
+++ b/spec/frontend/content_editor/extensions/reference_spec.js
@@ -1,9 +1,15 @@
import Reference from '~/content_editor/extensions/reference';
+import ReferenceLabel from '~/content_editor/extensions/reference_label';
import AssetResolver from '~/content_editor/services/asset_resolver';
import {
RESOLVED_ISSUE_HTML,
RESOLVED_MERGE_REQUEST_HTML,
RESOLVED_EPIC_HTML,
+ RESOLVED_LABEL_HTML,
+ RESOLVED_SNIPPET_HTML,
+ RESOLVED_MILESTONE_HTML,
+ RESOLVED_USER_HTML,
+ RESOLVED_VULNERABILITY_HTML,
} from '../test_constants';
import {
createTestEditor,
@@ -17,6 +23,7 @@ describe('content_editor/extensions/reference', () => {
let doc;
let p;
let reference;
+ let referenceLabel;
let renderMarkdown;
let assetResolver;
@@ -25,33 +32,54 @@ describe('content_editor/extensions/reference', () => {
assetResolver = new AssetResolver({ renderMarkdown });
tiptapEditor = createTestEditor({
- extensions: [Reference.configure({ assetResolver })],
+ extensions: [Reference.configure({ assetResolver }), ReferenceLabel],
});
({
- builders: { doc, p, reference },
+ builders: { doc, p, reference, referenceLabel },
} = createDocBuilder({
tiptapEditor,
names: {
reference: { nodeType: Reference.name },
+ referenceLabel: { nodeType: ReferenceLabel.name },
},
}));
});
describe('when typing a valid reference input rule', () => {
- const buildExpectedDoc = (href, originalText, referenceType, text) =>
+ const buildExpectedDoc = (href, originalText, referenceType, text = originalText) =>
doc(p(reference({ className: null, href, originalText, referenceType, text }), ' '));
+ const buildExpectedDocForLabel = (href, originalText, text, color) =>
+ doc(
+ p(
+ referenceLabel({
+ className: null,
+ referenceType: 'label',
+ href,
+ originalText,
+ text,
+ color,
+ }),
+ ' ',
+ ),
+ );
+
it.each`
- inputRuleText | mockReferenceHtml | expectedDoc
- ${'#1 '} | ${RESOLVED_ISSUE_HTML} | ${() => buildExpectedDoc('/gitlab-org/gitlab/-/issues/1', '#1', 'issue', '#1 (closed)')}
- ${'#1+ '} | ${RESOLVED_ISSUE_HTML} | ${() => buildExpectedDoc('/gitlab-org/gitlab/-/issues/1', '#1+', 'issue', '500 error on MR approvers edit page (#1 - closed)')}
- ${'#1+s '} | ${RESOLVED_ISSUE_HTML} | ${() => buildExpectedDoc('/gitlab-org/gitlab/-/issues/1', '#1+s', 'issue', '500 error on MR approvers edit page (#1 - closed) • Unassigned')}
- ${'!1 '} | ${RESOLVED_MERGE_REQUEST_HTML} | ${() => buildExpectedDoc('/gitlab-org/gitlab/-/merge_requests/1', '!1', 'merge_request', '!1 (merged)')}
- ${'!1+ '} | ${RESOLVED_MERGE_REQUEST_HTML} | ${() => buildExpectedDoc('/gitlab-org/gitlab/-/merge_requests/1', '!1+', 'merge_request', 'Enhance the LDAP group synchronization (!1 - merged)')}
- ${'!1+s '} | ${RESOLVED_MERGE_REQUEST_HTML} | ${() => buildExpectedDoc('/gitlab-org/gitlab/-/merge_requests/1', '!1+s', 'merge_request', 'Enhance the LDAP group synchronization (!1 - merged) • John Doe')}
- ${'&1 '} | ${RESOLVED_EPIC_HTML} | ${() => buildExpectedDoc('/groups/gitlab-org/-/epics/1', '&1', 'epic', '&1')}
- ${'&1+ '} | ${RESOLVED_EPIC_HTML} | ${() => buildExpectedDoc('/groups/gitlab-org/-/epics/1', '&1+', 'epic', 'Approvals in merge request list (&1)')}
+ inputRuleText | mockReferenceHtml | expectedDoc
+ ${'#1'} | ${RESOLVED_ISSUE_HTML} | ${() => buildExpectedDoc('/gitlab-org/gitlab/-/issues/1', '#1', 'issue', '#1 (closed)')}
+ ${'#1+'} | ${RESOLVED_ISSUE_HTML} | ${() => buildExpectedDoc('/gitlab-org/gitlab/-/issues/1', '#1+', 'issue', '500 error on MR approvers edit page (#1 - closed)')}
+ ${'#1+s'} | ${RESOLVED_ISSUE_HTML} | ${() => buildExpectedDoc('/gitlab-org/gitlab/-/issues/1', '#1+s', 'issue', '500 error on MR approvers edit page (#1 - closed) • Unassigned')}
+ ${'!1'} | ${RESOLVED_MERGE_REQUEST_HTML} | ${() => buildExpectedDoc('/gitlab-org/gitlab/-/merge_requests/1', '!1', 'merge_request', '!1 (merged)')}
+ ${'!1+'} | ${RESOLVED_MERGE_REQUEST_HTML} | ${() => buildExpectedDoc('/gitlab-org/gitlab/-/merge_requests/1', '!1+', 'merge_request', 'Enhance the LDAP group synchronization (!1 - merged)')}
+ ${'!1+s'} | ${RESOLVED_MERGE_REQUEST_HTML} | ${() => buildExpectedDoc('/gitlab-org/gitlab/-/merge_requests/1', '!1+s', 'merge_request', 'Enhance the LDAP group synchronization (!1 - merged) • John Doe')}
+ ${'&1'} | ${RESOLVED_EPIC_HTML} | ${() => buildExpectedDoc('/groups/gitlab-org/-/epics/1', '&1', 'epic', '&1')}
+ ${'&1+'} | ${RESOLVED_EPIC_HTML} | ${() => buildExpectedDoc('/groups/gitlab-org/-/epics/1', '&1+', 'epic', 'Approvals in merge request list (&1)')}
+ ${'@root'} | ${RESOLVED_USER_HTML} | ${() => buildExpectedDoc('/root', '@root', 'user')}
+ ${'~Aquanix'} | ${RESOLVED_LABEL_HTML} | ${() => buildExpectedDocForLabel('/gitlab-org/gitlab-shell/-/issues?label_name=Aquanix', '~Aquanix', 'Aquanix', 'rgb(230, 84, 49)')}
+ ${'%v4.0'} | ${RESOLVED_MILESTONE_HTML} | ${() => buildExpectedDoc('/gitlab-org/gitlab-shell/-/milestones/5', '%v4.0', 'milestone')}
+ ${'$25'} | ${RESOLVED_SNIPPET_HTML} | ${() => buildExpectedDoc('/gitlab-org/gitlab-shell/-/snippets/25', '$25', 'snippet')}
+ ${'[vulnerability:1]'} | ${RESOLVED_VULNERABILITY_HTML} | ${() => buildExpectedDoc('/gitlab-org/gitlab-shell/-/security/vulnerabilities/1', '[vulnerability:1]', 'vulnerability')}
`(
'replaces the input rule ($inputRuleText) with a reference node',
async ({ inputRuleText, mockReferenceHtml, expectedDoc }) => {
@@ -61,8 +89,8 @@ describe('content_editor/extensions/reference', () => {
action() {
renderMarkdown.mockResolvedValueOnce(mockReferenceHtml);
- tiptapEditor.commands.insertContent({ type: 'text', text: inputRuleText });
- triggerNodeInputRule({ tiptapEditor, inputRuleText });
+ tiptapEditor.commands.insertContent({ type: 'text', text: `${inputRuleText} ` });
+ triggerNodeInputRule({ tiptapEditor, inputRuleText: `${inputRuleText} ` });
},
});
diff --git a/spec/frontend/content_editor/services/asset_resolver_spec.js b/spec/frontend/content_editor/services/asset_resolver_spec.js
index 292eec6db77b..b0135a6bc9f3 100644
--- a/spec/frontend/content_editor/services/asset_resolver_spec.js
+++ b/spec/frontend/content_editor/services/asset_resolver_spec.js
@@ -3,6 +3,11 @@ import {
RESOLVED_ISSUE_HTML,
RESOLVED_MERGE_REQUEST_HTML,
RESOLVED_EPIC_HTML,
+ RESOLVED_LABEL_HTML,
+ RESOLVED_SNIPPET_HTML,
+ RESOLVED_MILESTONE_HTML,
+ RESOLVED_USER_HTML,
+ RESOLVED_VULNERABILITY_HTML,
} from '../test_constants';
describe('content_editor/services/asset_resolver', () => {
@@ -48,6 +53,32 @@ describe('content_editor/services/asset_resolver', () => {
text: '!1 (merged)',
};
+ const resolvedLabel = {
+ backgroundColor: 'rgb(230, 84, 49)',
+ href: '/gitlab-org/gitlab-shell/-/issues?label_name=Aquanix',
+ text: 'Aquanix',
+ };
+
+ const resolvedSnippet = {
+ href: '/gitlab-org/gitlab-shell/-/snippets/25',
+ text: '$25',
+ };
+
+ const resolvedMilestone = {
+ href: '/gitlab-org/gitlab-shell/-/milestones/5',
+ text: '%v4.0',
+ };
+
+ const resolvedUser = {
+ href: '/root',
+ text: '@root',
+ };
+
+ const resolvedVulnerability = {
+ href: '/gitlab-org/gitlab-shell/-/security/vulnerabilities/1',
+ text: '[vulnerability:1]',
+ };
+
describe.each`
referenceType | referenceId | sentMarkdown | returnedHtml | resolvedReference
${'issue'} | ${'#1'} | ${'#1 #1+ #1+s'} | ${RESOLVED_ISSUE_HTML} | ${resolvedIssue}
@@ -59,7 +90,9 @@ describe('content_editor/services/asset_resolver', () => {
it(`resolves ${referenceType} reference to href, text, title and summary`, async () => {
renderMarkdown.mockResolvedValue(returnedHtml);
- expect(await assetResolver.resolveReference(referenceId)).toEqual(resolvedReference);
+ expect(await assetResolver.resolveReference(referenceId)).toMatchObject(
+ resolvedReference,
+ );
});
it.each`
@@ -74,6 +107,26 @@ describe('content_editor/services/asset_resolver', () => {
},
);
+ describe.each`
+ referenceType | referenceId | returnedHtml | resolvedReference
+ ${'label'} | ${'~Aquanix'} | ${RESOLVED_LABEL_HTML} | ${resolvedLabel}
+ ${'snippet'} | ${'$25'} | ${RESOLVED_SNIPPET_HTML} | ${resolvedSnippet}
+ ${'milestone'} | ${'%v4.0'} | ${RESOLVED_MILESTONE_HTML} | ${resolvedMilestone}
+ ${'user'} | ${'@root'} | ${RESOLVED_USER_HTML} | ${resolvedUser}
+ ${'vulnerability'} | ${'[vulnerability:1]'} | ${RESOLVED_VULNERABILITY_HTML} | ${resolvedVulnerability}
+ `(
+ 'for reference type $referenceType',
+ ({ referenceType, referenceId, returnedHtml, resolvedReference }) => {
+ it(`resolves ${referenceType} reference to href, text and additional props (if any)`, async () => {
+ renderMarkdown.mockResolvedValue(returnedHtml);
+
+ expect(await assetResolver.resolveReference(referenceId)).toMatchObject(
+ resolvedReference,
+ );
+ });
+ },
+ );
+
it.each`
case | sentMarkdown | returnedHtml
${'no html is returned'} | ${''} | ${''}
diff --git a/spec/frontend/content_editor/test_constants.js b/spec/frontend/content_editor/test_constants.js
index cbd4f555e97f..255a7104eafa 100644
--- a/spec/frontend/content_editor/test_constants.js
+++ b/spec/frontend/content_editor/test_constants.js
@@ -44,3 +44,18 @@ export const RESOLVED_MERGE_REQUEST_HTML =
export const RESOLVED_EPIC_HTML =
'<p data-sourcepos="1:1-1:11" dir="auto"><a href="/groups/gitlab-org/-/epics/1" data-reference-type="epic" data-original="&amp;1" data-link="false" data-link-reference="false" data-group="9970" data-epic="1" data-container="body" data-placement="top" title="Approvals in merge request list" class="gfm gfm-epic has-tooltip">&1</a> <a href="/groups/gitlab-org/-/epics/1" data-reference-type="epic" data-original="&amp;1+" data-link="false" data-link-reference="false" data-group="9970" data-epic="1" data-reference-format="+" data-container="body" data-placement="top" title="Approvals in merge request list" class="gfm gfm-epic has-tooltip">Approvals in merge request list (&1)</a> <a href="/groups/gitlab-org/-/epics/1" data-reference-type="epic" data-original="&amp;1+s" data-link="false" data-link-reference="false" data-group="9970" data-epic="1" data-reference-format="+s" data-container="body" data-placement="top" title="Approvals in merge request list" class="gfm gfm-epic has-tooltip">Approvals in merge request list (&1)</a></p>';
+
+export const RESOLVED_LABEL_HTML =
+ '<p data-sourcepos="1:1-1:29" dir="auto"><span class="gl-label gl-label-sm"><a href="/gitlab-org/gitlab-shell/-/issues?label_name=Aquanix" data-reference-type="label" data-original="~Aquanix" data-link="false" data-link-reference="false" data-project="2" data-label="5" data-container="body" data-placement="top" title="" class="gfm gfm-label has-tooltip gl-link gl-label-link"><span class="gl-label-text gl-label-text-light" data-container="body" data-html="true" style="background-color: #e65431">Aquanix</span></a></span> <span class="gl-label gl-label-sm"><a href="/gitlab-org/gitlab-shell/-/issues?label_name=Aquanix" data-reference-type="label" data-original="~Aquanix" data-link="false" data-link-reference="false" data-project="2" data-label="5" data-container="body" data-placement="top" title="" class="gfm gfm-label has-tooltip gl-link gl-label-link"><span class="gl-label-text gl-label-text-light" data-container="body" data-html="true" style="background-color: #e65431">Aquanix</span></a></span>+ <span class="gl-label gl-label-sm"><a href="/gitlab-org/gitlab-shell/-/issues?label_name=Aquanix" data-reference-type="label" data-original="~Aquanix" data-link="false" data-link-reference="false" data-project="2" data-label="5" data-container="body" data-placement="top" title="" class="gfm gfm-label has-tooltip gl-link gl-label-link"><span class="gl-label-text gl-label-text-light" data-container="body" data-html="true" style="background-color: #e65431">Aquanix</span></a></span>+s</p>';
+
+export const RESOLVED_SNIPPET_HTML =
+ '<p data-sourcepos="1:1-1:14" dir="auto"><a href="/gitlab-org/gitlab-shell/-/snippets/25" data-reference-type="snippet" data-original="$25" data-link="false" data-link-reference="false" data-project="2" data-snippet="25" data-container="body" data-placement="top" title="test" class="gfm gfm-snippet has-tooltip">$25</a> <a href="/gitlab-org/gitlab-shell/-/snippets/25" data-reference-type="snippet" data-original="$25" data-link="false" data-link-reference="false" data-project="2" data-snippet="25" data-container="body" data-placement="top" title="test" class="gfm gfm-snippet has-tooltip">$25</a>+ <a href="/gitlab-org/gitlab-shell/-/snippets/25" data-reference-type="snippet" data-original="$25" data-link="false" data-link-reference="false" data-project="2" data-snippet="25" data-container="body" data-placement="top" title="test" class="gfm gfm-snippet has-tooltip">$25</a>+s</p>';
+
+export const RESOLVED_MILESTONE_HTML =
+ '<p data-sourcepos="1:1-1:20" dir="auto"><a href="/gitlab-org/gitlab-shell/-/milestones/5" data-reference-type="milestone" data-original="%v4.0" data-link="false" data-link-reference="false" data-project="2" data-milestone="10" data-container="body" data-placement="top" title="" class="gfm gfm-milestone has-tooltip">%v4.0</a> <a href="/gitlab-org/gitlab-shell/-/milestones/5" data-reference-type="milestone" data-original="%v4.0" data-link="false" data-link-reference="false" data-project="2" data-milestone="10" data-container="body" data-placement="top" title="" class="gfm gfm-milestone has-tooltip">%v4.0</a>+ %v4.0+s</p>';
+
+export const RESOLVED_USER_HTML =
+ '<p data-sourcepos="1:1-1:20" dir="auto"><a href="/root" data-reference-type="user" data-user="1" data-container="body" data-placement="top" class="gfm gfm-project_member js-user-link" title="Administrator">@root</a> <a href="/root" data-reference-type="user" data-user="1" data-container="body" data-placement="top" class="gfm gfm-project_member js-user-link" title="Administrator">@root</a>+ <a href="/root" data-reference-type="user" data-user="1" data-container="body" data-placement="top" class="gfm gfm-project_member js-user-link" title="Administrator">@root</a>+s</p>';
+
+export const RESOLVED_VULNERABILITY_HTML =
+ '<p data-sourcepos="1:1-1:56" dir="auto"><a href="/gitlab-org/gitlab-shell/-/security/vulnerabilities/1" data-reference-type="vulnerability" data-original="[vulnerability:1]" data-link="false" data-link-reference="false" data-project="2" data-vulnerability="1" data-container="body" data-placement="top" title="oh no!" class="gfm gfm-vulnerability has-tooltip">[vulnerability:1]</a> <a href="/gitlab-org/gitlab-shell/-/security/vulnerabilities/1" data-reference-type="vulnerability" data-original="[vulnerability:1]" data-link="false" data-link-reference="false" data-project="2" data-vulnerability="1" data-container="body" data-placement="top" title="oh no!" class="gfm gfm-vulnerability has-tooltip">[vulnerability:1]</a>+ <a href="/gitlab-org/gitlab-shell/-/security/vulnerabilities/1" data-reference-type="vulnerability" data-original="[vulnerability:1]" data-link="false" data-link-reference="false" data-project="2" data-vulnerability="1" data-container="body" data-placement="top" title="oh no!" class="gfm gfm-vulnerability has-tooltip">[vulnerability:1]</a>+s</p>';
Edited by Himanshu Kapoor