Add handling short mentions tags to ExpensiMark #824

Kicu · 2024-12-21T14:27:29Z

Details

This is part of allowing to style and highlight short-mentions inside live-markdown when user is writing/editing their message. More details here: Expensify/App#38025 (comment)

The purpose of this PR is to add handling of (possible) short mention tags to ExpensiMark.
In this PR we define "short-mention" as anything after @ character that would be a valid email prefix.
Examples:

valid short mentions: @mateusz.titz, @test.com, @john.doe, @jane123456
invalid short mentions: @mateusz.titz@swmansion.com (<-- standard mention), @+12345

Why "possible" short mentions

The only way for us to actually render short-mentions inside react-native-live-markdown (and other code using ExpensiMark) is to somehow have the list of all possible user logins, and then checking whether the user typed mention matches any of them.

However such list should not and cannot be shared in any way to ExpensiMark, as this is just a simple parser. But we still need to some kind of output from ExpensiMark, to understand that there are mentions inside parsed text, so that we can later try and match them to user onyx data.

This creates some changes in how ExpensiMark interprets text, because something that in the past would be considered just normal text, might now be a short mention.

Example of parser output that changed (based on tests)

test@gmail.com@gmail.com
- prev: <a href=\"mailto:test@gmail.com\">test@gmail.com</a>@gmail.com
- now: <a href=\"mailto:test@gmail.com\">test@gmail.com</a><mention-short>@gmail.com</mention-short> - @gmail.com is a correct mention

In total I had to edit ~5 tests, all the other ones are passing correctly.

CC @puneetlath @tomekzaw @BartoszGrajdek

Fixed Issues

$ Expensify/App#38025

Tests

I made sure all existing unit tests pass, and added specific tests for short mentions

QA

What does QA need to do to validate your changes?
What areas to they need to test for regressions?

github-actions · 2024-12-21T14:27:43Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

Kicu · 2024-12-21T14:34:10Z

I have read the CLA Document and I hereby sign the CLA

shubham1206agra · 2025-01-08T18:34:22Z

Commenting here for assignment.

shubham1206agra · 2025-01-08T18:38:00Z

__tests__/ExpensiMark-HTML-test.js

 test('Test for email with test+1@gmail.com@gmail.com', () => {
    const testString = 'test+1@gmail.com@gmail.com';
-    const resultString = '<a href="mailto:test+1@gmail.com">test+1@gmail.com</a>@gmail.com';
+    const resultString = '<a href="mailto:test+1@gmail.com">test+1@gmail.com</a><mention-short>@gmail.com</mention-short>';
    expect(parser.replace(testString)).toBe(resultString);
 });

 test('Test for email with test@gmail.com@gmail.com', () => {
    const testString = 'test@gmail.com@gmail.com';
-    const resultString = '<a href="mailto:test@gmail.com">test@gmail.com</a>@gmail.com';
+    const resultString = '<a href="mailto:test@gmail.com">test@gmail.com</a><mention-short>@gmail.com</mention-short>';
    expect(parser.replace(testString)).toBe(resultString);
 });


This might be invalid emails after all. See https://stackoverflow.com/questions/12355858/how-many-symbol-can-be-in-an-email-address

yeah indeed test+1@gmail.com@gmail.com looks like invalid email.
But I thought the reasoning was that the parser should treat it as a separate email (1st part) and then just text @gmail.com which now becomes a short mention.

Do you want me to change anything related to this?

shubham1206agra · 2025-01-27T08:28:19Z

@Kicu I was trying this PR along with Expensify/App#55509 and Expensify/App#54037. But RN Live Markdown does not seem to be highlighting short mentions. Is there some additional logic missing which I needed to make this work correctly?

shubham1206agra · 2025-01-27T08:36:27Z

Kicu · 2025-01-28T09:03:50Z

hey @shubham1206agra I will take another look at this, because some time has passed and perhaps things broke.
I will provide a more detailed description of testing this in a few days.

I'm currently busy working on the bugs on navigation updates which take priority over this. Once navi (Expensify/App#49539) is in a better spot I will come back to this one.
Sorry for any confusion :)

CC @puneetlath

Kicu · 2025-02-06T11:04:10Z

@shubham1206agra update with description how to test this here: Expensify/App#38025 (comment)

shubham1206agra · 2025-02-18T08:41:22Z

lib/ExpensiMark.ts

+            {
+                name: 'hereMentionAfterShortMentions',
+                regex: /(<\/mention-short>)(@here)(?=\b)/gm,
+                replacement: '$1<mention-here>$2</mention-here>',
+            },


@Kicu Is this still needed? Since patch does not have this code and it works fine for me.

yes I verified this is still needed.
Without it such string don't work: @mateusz.titz@here

It's a mistake I didn't add it to the patch, sorry.

Without this:

With this:

you should be able to test by yourself, you can just copy-paste this block and add it to the same file that patch modifies

@Kicu One improvement note: Add some border radius so that we know these are separate mentions in RN Live Markdown.

Adding rounded borders is quite a complicated feature on which we are already working, and for now we cannot implement it for native, only for web.
Please refer to this issue: Expensify/App#56560 which adds it only for web.
And this an even older issue related to rounded corners: Expensify/App#19299

In short: for now we can have it only on web, and a colleague is working on this. So I won't be touching styles in this PR

shubham1206agra · 2025-02-21T13:42:57Z

@puneetlath All yours.

Kicu mentioned this pull request Dec 21, 2024

Render shortened mentions with blue/green outline in live markdown preview Expensify/App#38025

Open

Kicu force-pushed the kicu/38025-short-mentions branch from 275cc20 to c9e41a1 Compare December 30, 2024 09:44

CLABotify added a commit to Expensify/CLA that referenced this pull request Dec 30, 2024

@Kicu has signed the CLA in Expensify/expensify-common#824

128f595

Kicu added 2 commits January 7, 2025 15:17

Add handling possible short mentions tags to ExpensiMark

3c69bbe

Cleanup and fix short-mentions tests

bb4cc97

Kicu force-pushed the kicu/38025-short-mentions branch from c9e41a1 to bb4cc97 Compare January 7, 2025 14:17

Kicu mentioned this pull request Jan 8, 2025

Add support for short mentions from ExpensiMark Expensify/react-native-live-markdown#592

Merged

Kicu force-pushed the kicu/38025-short-mentions branch from e7b887c to 206d18c Compare January 8, 2025 12:59

Kicu changed the title ~~[WIP] Add handling possible short mentions tags to ExpensiMark~~ Add handling possible short mentions tags to ExpensiMark Jan 8, 2025

Kicu changed the title ~~Add handling possible short mentions tags to ExpensiMark~~ Add handling short mentions tags to ExpensiMark Jan 8, 2025

Improve short mentions regex and make parser tests pass

f7e27c7

Kicu force-pushed the kicu/38025-short-mentions branch from 206d18c to f7e27c7 Compare January 8, 2025 13:13

Kicu marked this pull request as ready for review January 8, 2025 13:14

Kicu requested a review from a team as a code owner January 8, 2025 13:14

melvin-bot bot requested review from arosiclair and removed request for a team January 8, 2025 13:14

arosiclair requested review from puneetlath and thienlnam and removed request for arosiclair January 8, 2025 16:10

shubham1206agra reviewed Jan 8, 2025

View reviewed changes

thienlnam requested a review from shubham1206agra January 8, 2025 19:09

Kicu mentioned this pull request Jan 9, 2025

Show short mentions and current user mention in MarkdownTextInput with styling Expensify/App#54037

Open

48 tasks

shubham1206agra reviewed Feb 18, 2025

View reviewed changes

shubham1206agra mentioned this pull request Feb 19, 2025

[$500] Mention disappears when including 's Expensify/App#50724

Open

6 tasks

shubham1206agra approved these changes Feb 21, 2025

View reviewed changes

puneetlath approved these changes Feb 24, 2025

View reviewed changes

puneetlath merged commit c73d705 into Expensify:main Feb 24, 2025
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add handling short mentions tags to ExpensiMark #824

Add handling short mentions tags to ExpensiMark #824

Kicu commented Dec 21, 2024 •

edited

Loading

github-actions bot commented Dec 21, 2024 •

edited

Loading

Kicu commented Dec 21, 2024

shubham1206agra commented Jan 8, 2025

shubham1206agra Jan 8, 2025

Kicu Jan 9, 2025

shubham1206agra Jan 9, 2025

shubham1206agra commented Jan 27, 2025 •

edited

Loading

shubham1206agra commented Jan 27, 2025

Kicu commented Jan 28, 2025

Kicu commented Feb 6, 2025

shubham1206agra Feb 18, 2025

Kicu Feb 21, 2025 •

edited

Loading

Kicu Feb 21, 2025

shubham1206agra Feb 21, 2025

Kicu Feb 24, 2025

shubham1206agra commented Feb 21, 2025

Add handling short mentions tags to ExpensiMark #824

Add handling short mentions tags to ExpensiMark #824

Conversation

Kicu commented Dec 21, 2024 • edited Loading

Details

Why "possible" short mentions

Example of parser output that changed (based on tests)

Fixed Issues

Tests

QA

github-actions bot commented Dec 21, 2024 • edited Loading

Kicu commented Dec 21, 2024

shubham1206agra commented Jan 8, 2025

shubham1206agra Jan 8, 2025

Choose a reason for hiding this comment

Kicu Jan 9, 2025

Choose a reason for hiding this comment

shubham1206agra Jan 9, 2025

Choose a reason for hiding this comment

shubham1206agra commented Jan 27, 2025 • edited Loading

shubham1206agra commented Jan 27, 2025

Kicu commented Jan 28, 2025

Kicu commented Feb 6, 2025

shubham1206agra Feb 18, 2025

Choose a reason for hiding this comment

Kicu Feb 21, 2025 • edited Loading

Choose a reason for hiding this comment

Kicu Feb 21, 2025

Choose a reason for hiding this comment

shubham1206agra Feb 21, 2025

Choose a reason for hiding this comment

Kicu Feb 24, 2025

Choose a reason for hiding this comment

shubham1206agra commented Feb 21, 2025

Kicu commented Dec 21, 2024 •

edited

Loading

github-actions bot commented Dec 21, 2024 •

edited

Loading

shubham1206agra commented Jan 27, 2025 •

edited

Loading

Kicu Feb 21, 2025 •

edited

Loading