fix the issue that inconsistent highlighting occurs when multiple mentions are used without spaces #594

agilejune · 2023-10-31T04:50:15Z

Fixed Issues

$ Expensify/App#21753
Proposal: Expensify/App#21753 (comment)

Tests

Go to any chat.
Type the following cases

@[email protected]@here
@[email protected][email protected]
@[email protected]@[email protected]
@here@[email protected]
@[email protected]@here
@here@[email protected]
@[email protected]@here

Observe that all the mentions are highlighted as expected.

QA

Same as tests step

Offline tests

Same as tests step

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

github-actions · 2023-10-31T04:50:31Z

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

agilejune · 2023-10-31T04:50:53Z

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

amyevans · 2023-10-31T16:54:46Z

@aimane-chnaif can you comment here so I can assign you for review please?

aimane-chnaif · 2023-10-31T16:56:58Z

reviewing

aimane-chnaif · 2023-10-31T16:57:27Z

__tests__/ExpensiMark-HTML-test.js

@@ -1298,6 +1298,36 @@ test('Test for @here mention without space or supported styling character', () =
    expect(parser.replace(testString)).toBe(resultString);
 });

+test('Test user mention and here mention, which are concatenated without space', () => {


please use general emails, not mine 😄

It's updated with general ones

Also update Step 2 in Tests as well

done as well

aimane-chnaif · 2023-11-05T09:50:49Z

@agilejune

agilejune · 2023-11-06T08:03:00Z

@aimane-chnaif good catch of edge case
I will fix to cover all those cases

agilejune · 2023-11-07T09:03:00Z

agilejune · 2023-11-07T09:03:16Z

@aimane-chnaif I fixed and added auto test cases, please check again

puneetlath · 2023-11-10T20:45:04Z

@agilejune looks like you have some failing tests.

aimane-chnaif · 2023-11-10T20:46:49Z

@agilejune please merge main and fix javascript test failing

agilejune · 2023-11-13T07:24:14Z

@puneetlath @aimane-chnaif
all done

aimane-chnaif

What code change you done to fix js lint?

aimane-chnaif · 2023-11-13T16:29:36Z

lib/ExpensiMark.js

@@ -202,7 +208,7 @@ export default class ExpensiMark {
                    return this.modifyTextForQuote(regex, textToProcess, replacement);
                },
                replacement: (g1) => {
-                    const replacedText = this.replace(g1, {filterRules: ['heading1'], shouldEscapeText: false});
+                    const replacedText = this.replace(g1, { filterRules: ['heading1'], shouldEscapeText: false });


remove all irrelevant changes like this. Keep original code formatting

oops, mistake of auto formatting
all restored
@aimane-chnaif

aimane-chnaif · 2023-11-14T16:21:15Z

lib/str.js

@@ -944,7 +944,8 @@ const Str = {
     */
    isValidMention(mention) {
        // Mentions can start @ proceeded by a space, eg "ping @[email protected]"
-        if (/[\s@]/g.test(mention.charAt(0))) {
+        // or by a @here, eg "@here@[email protected]"
+        if (/[\s@]/g.test(mention.charAt(0)) || /^@here.*/g.test(mention)) {


Is this change needed? I don't see issue without adding this condition

If this fixes something, add more test case affected by this change

@aimane-chnaif you're right. it's needless
I removed it

aimane-chnaif · 2023-11-15T09:43:01Z

Not sure this should be fixed here. I think it's expected.

aimane-chnaif · 2023-11-15T10:15:53Z

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

aimane-chnaif · 2023-11-15T10:17:10Z

@puneetlath Is this known bug? style broken on iOS native

aimane-chnaif

LGTM 🎉
@puneetlath all yours

puneetlath · 2023-11-17T22:32:44Z

Sorry for the delay. Code looks good to me, but tests are failing.

agilejune · 2023-11-18T09:24:16Z

@puneetlath It's fixed

fix-#21753

6c0760a

agilejune requested a review from a team as a code owner October 31, 2023 04:50

melvin-bot bot requested review from amyevans and removed request for a team October 31, 2023 04:50

agilejune mentioned this pull request Oct 31, 2023

[$2000] Web - Inconsistently highlighting occurs when multiple mentions are used without spaces. Expensify/App#21753

Closed

6 tasks

amyevans removed their request for review October 31, 2023 16:55

aimane-chnaif reviewed Oct 31, 2023

View reviewed changes

update test case

cd50eb3

agilejune and others added 2 commits November 7, 2023 07:19

fix 21753 - edge cases of multiple mention

77c08a9

Merge branch 'main' into fix-#21753

378d8f0

Merge branch 'Expensify:main' into fix-#21753

2b7a23a

aimane-chnaif reviewed Nov 13, 2023

View reviewed changes

fix lint

b5707f8

agilejune force-pushed the fix-#21753 branch from 0c9f1f8 to b5707f8 Compare November 13, 2023 22:38

aimane-chnaif reviewed Nov 14, 2023

View reviewed changes

agilejune and others added 2 commits November 14, 2023 22:54

remove condition

5d40ac3

Merge branch 'main' into fix-#21753

1d6b926

aimane-chnaif approved these changes Nov 15, 2023

View reviewed changes

puneetlath previously approved these changes Nov 17, 2023

View reviewed changes

agilejune and others added 2 commits November 18, 2023 01:22

Merge branch 'Expensify:main' into fix-#21753

d5d7fcd

fix here mention

bdbef09

agilejune dismissed puneetlath’s stale review via bdbef09 November 18, 2023 09:23

puneetlath approved these changes Nov 19, 2023

View reviewed changes

puneetlath merged commit d24d6fa into Expensify:main Nov 19, 2023
5 checks passed

Victor-Nyagudi mentioned this pull request Nov 22, 2023

[HOLD for payment 2023-12-28] [$500] Mentioning an email with double @ sign results in an invalid email mention Expensify/App#28946

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix the issue that inconsistent highlighting occurs when multiple mentions are used without spaces #594

fix the issue that inconsistent highlighting occurs when multiple mentions are used without spaces #594

agilejune commented Oct 31, 2023 •

edited

Loading

github-actions bot commented Oct 31, 2023 •

edited

Loading

agilejune commented Oct 31, 2023

amyevans commented Oct 31, 2023

aimane-chnaif commented Oct 31, 2023

aimane-chnaif Oct 31, 2023

agilejune Nov 1, 2023

aimane-chnaif Nov 1, 2023

agilejune Nov 1, 2023

aimane-chnaif commented Nov 5, 2023

agilejune commented Nov 6, 2023

agilejune commented Nov 7, 2023

agilejune commented Nov 7, 2023 •

edited

Loading

puneetlath commented Nov 10, 2023

aimane-chnaif commented Nov 10, 2023

agilejune commented Nov 13, 2023

aimane-chnaif left a comment

aimane-chnaif Nov 13, 2023

agilejune Nov 13, 2023 •

edited

Loading

aimane-chnaif Nov 14, 2023

aimane-chnaif Nov 14, 2023

agilejune Nov 14, 2023

aimane-chnaif commented Nov 15, 2023 •

edited

Loading

aimane-chnaif commented Nov 15, 2023

aimane-chnaif commented Nov 15, 2023

aimane-chnaif left a comment

puneetlath commented Nov 17, 2023

agilejune commented Nov 18, 2023

fix the issue that inconsistent highlighting occurs when multiple mentions are used without spaces #594

fix the issue that inconsistent highlighting occurs when multiple mentions are used without spaces #594

Conversation

agilejune commented Oct 31, 2023 • edited Loading

Fixed Issues

Tests

QA

Offline tests

Screenshots/Videos

github-actions bot commented Oct 31, 2023 • edited Loading

agilejune commented Oct 31, 2023

amyevans commented Oct 31, 2023

aimane-chnaif commented Oct 31, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aimane-chnaif commented Nov 5, 2023

agilejune commented Nov 6, 2023

agilejune commented Nov 7, 2023

agilejune commented Nov 7, 2023 • edited Loading

puneetlath commented Nov 10, 2023

aimane-chnaif commented Nov 10, 2023

agilejune commented Nov 13, 2023

aimane-chnaif left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agilejune Nov 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aimane-chnaif commented Nov 15, 2023 • edited Loading

aimane-chnaif commented Nov 15, 2023

Screenshots/Videos

aimane-chnaif commented Nov 15, 2023

aimane-chnaif left a comment

Choose a reason for hiding this comment

puneetlath commented Nov 17, 2023

agilejune commented Nov 18, 2023

agilejune commented Oct 31, 2023 •

edited

Loading

github-actions bot commented Oct 31, 2023 •

edited

Loading

agilejune commented Nov 7, 2023 •

edited

Loading

agilejune Nov 13, 2023 •

edited

Loading

aimane-chnaif commented Nov 15, 2023 •

edited

Loading