diff --git a/src/sidebar/components/MarkdownEditor.tsx b/src/sidebar/components/MarkdownEditor.tsx index efb7f15dfbf..c00a91a9b2a 100644 --- a/src/sidebar/components/MarkdownEditor.tsx +++ b/src/sidebar/components/MarkdownEditor.tsx @@ -585,8 +585,8 @@ export default function MarkdownEditor({ const input = useRef(null); const textWithoutMentionTags = useMemo( - () => unwrapMentions(text, mentionMode), - [mentionMode, text], + () => unwrapMentions({ text, mentionMode, mentions }), + [mentionMode, mentions, text], ); useEffect(() => { diff --git a/src/sidebar/helpers/mentions.ts b/src/sidebar/helpers/mentions.ts index 4b11496e85c..f7f6c712eef 100644 --- a/src/sidebar/helpers/mentions.ts +++ b/src/sidebar/helpers/mentions.ts @@ -96,27 +96,56 @@ export function wrapDisplayNameMentions( // some assumptions about the tag content in order to be able to get away // with it. Specifically we assume that these tags will not contain `<` or // `>` in attribute values or the HTML content of the tag. -const MENTION_TAG_RE = /]\bdata-hyp-mention\b[^>]*>([^<]+)<\/a>/g; +const MENTION_TAG_RE = /]\bdata-hyp-mention\b[^>]*>@([^<]+)<\/a>/g; + +export type UnwrapMentionsOptions = { + text: string; + mentionMode: MentionMode; + mentions?: Mention[]; +}; /** * Replace all mentions wrapped in the special `` tag with * their plain-text representation. + * * The plain-text representation depends on the mention mode: * - `username`: @username * - `display-name`: @[Display Name] + * + * If a list of mentions is provided, the tag's userid will be matched against + * it, so that the plain-text version uses the most recent username or display + * name, in case they have changed since the mention was created. + * If a list is not provided or the mention is not found, the tag's content will + * be used as username or display name. */ -export function unwrapMentions(text: string, mentionMode: MentionMode) { +export function unwrapMentions({ + text, + mentionMode, + mentions = [], +}: UnwrapMentionsOptions) { // Use a regex rather than HTML parser to replace the mentions in order // to avoid modifying any of the content outside of the replaced tags. This // includes avoiding modifications such as encoding characters that will // happen when parsing and re-serializing HTML via eg. `innerHTML`. - return text.replace(MENTION_TAG_RE, (match, mention) => { - if (mentionMode === 'username') { - return mention; - } + return text.replace(MENTION_TAG_RE, (match, tagContent) => { + // Even though we need to capture the tag content via a regex for the + // reasons explained above, we can use regular DOM APIs to get the userid + const tempElement = document.createElement('div'); + tempElement.innerHTML = match; + const userid = tempElement + .querySelector('a[data-hyp-mention]') + ?.getAttribute('data-userid'); + + const mention = mentions.find( + ({ original_userid }) => original_userid === userid, + ); - const [atChar, ...rest] = mention; - return `${atChar}[${rest.join('')}]`; + return mentionMode === 'username' + ? `@${mention?.username ?? tagContent}` + : // Using || rather than ?? for the display name, to avoid setting an + // empty string if the user used to have a display name and has been + // removed since the mention was created + `@[${mention?.display_name || tagContent}]`; }); } diff --git a/src/sidebar/helpers/test/mentions-test.js b/src/sidebar/helpers/test/mentions-test.js index 591e465f5ef..3abb14db8e5 100644 --- a/src/sidebar/helpers/test/mentions-test.js +++ b/src/sidebar/helpers/test/mentions-test.js @@ -134,7 +134,10 @@ Hello ${mentionTag('jane.doe', 'example.com')}.`, describe('unwrapMentions - `username` mode', () => { it('removes wrapping mention tags', () => { - assert.equal(unwrapMentions(textWithTags, 'username'), text); + assert.equal( + unwrapMentions({ text: textWithTags, mentionMode: 'username' }), + text, + ); }); }); }); @@ -211,7 +214,88 @@ Hello ${mentionTag('jane.doe', 'example.com')}.`, describe('unwrapMentions - `display-name` mode', () => { it('removes wrapping mention tags', () => { - assert.equal(unwrapMentions(textWithTags, 'display-name'), text); + assert.equal( + unwrapMentions({ text: textWithTags, mentionMode: 'display-name' }), + text, + ); + }); + }); +}); + +describe('unwrapMentions', () => { + [ + // Mention not found. Tag content kept + { + mentionMode: 'username', + mentions: [], + text: `Hello ${mentionTag('jane_doe')}`, + expectedResult: 'Hello @jane_doe', + }, + { + mentionMode: 'display-name', + mentions: [], + text: `Hello ${displayNameMentionTag('Jane Doe', 'jane_doe')}`, + expectedResult: 'Hello @[Jane Doe]', + }, + + // Mention found with a new username + { + mentionMode: 'username', + mentions: [ + { + original_userid: 'acct:jane_doe@hypothes.is', + username: 'jane_edited', + }, + ], + text: `Hello ${mentionTag('jane_doe')}`, + expectedResult: 'Hello @jane_edited', + }, + + // Mention found with a new display name + { + mentionMode: 'display-name', + mentions: [ + { + original_userid: 'acct:jane_doe@hypothes.is', + display_name: 'My new name', + }, + ], + text: `Hello ${displayNameMentionTag('Jane Doe', 'jane_doe')}`, + expectedResult: 'Hello @[My new name]', + }, + + // Mention found with a now empty display name + { + mentionMode: 'display-name', + mentions: [ + { + original_userid: 'acct:jane_doe@hypothes.is', + display_name: '', + }, + ], + text: `Hello ${displayNameMentionTag('Jane Doe', 'jane_doe')}`, + expectedResult: 'Hello @[Jane Doe]', + }, + + // data-userid not present + { + mentionMode: 'username', + mentions: [], + text: 'Hello @user_id_missing', + expectedResult: 'Hello @user_id_missing', + }, + { + mentionMode: 'display-name', + mentions: [], + text: 'Hello @User ID Missing', + expectedResult: 'Hello @[User ID Missing]', + }, + ].forEach(({ text, mentionMode, mentions, expectedResult }) => { + it('replaces mention tag with current username or display name', () => { + assert.equal( + unwrapMentions({ text, mentionMode, mentions }), + expectedResult, + ); }); }); });