Use new logic to check bold text

[ShridharGoel]

Use new logic to check bold text: Expensify/App#41109 (comment)

Fixed Issues

$ Expensify/App#41109

Tests and QA steps:

1. Type anything on a Google document.
2. Copy it.
3. Paste it in Expensify.
4. Check if the font is still normal (it shouldn't be bold).
5. Now, type something in bold text on a Google document.
6. Copy/paste to Expensify.
7. It should show in bold.

Screen.Recording.2024-06-28.at.1.56.15.PM.mov

Screen.Recording.2024-06-28.at.8.06.32.PM.mov

[github-actions]

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

[ShridharGoel]

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

[jasperhuangg]

Please show testing steps and screenshots of this change working with the App PR you're going to submit.

Please also link the App PR in the description.

JS Linter is also failing, try running npm run prettier

[jasperhuangg]

Hey @ShridharGoel any updates on this?

[ShridharGoel]

Will add screenshots in some time.

[jasperhuangg]

@ShridharGoel Please prioritize this when you can

[ShridharGoel]

This was updated few days back.

[ShridharGoel]

Testing steps and videos were added.

[jasperhuangg]

@ShridharGoel the tests are failing, please fix them.

For the linter tests, just run npm run prettier and commit the changes.

Please find the failing JS tests here: https://github.com/Expensify/expensify-common/actions/runs/9379944077/job/25851454328?pr=710

[jasperhuangg]

Bump @ShridharGoel

[jasperhuangg]

@ShridharGoel Did you run npm run prettier locally? I just ran it and there's still a diff, all you need to do is run it and commit the difference. I can't do so because I'm a reviewer.

╰─ git diff
diff --git a/lib/ExpensiMark.js b/lib/ExpensiMark.js
index 2c51e80..917606f 100644
--- a/lib/ExpensiMark.js
+++ b/lib/ExpensiMark.js
@@ -411,9 +411,8 @@ export default class ExpensiMark {
                 replacement: (match, tagContent, innerContent) => {
                     const fontWeightMatch = innerContent.match(/style="(.*?)"/);
                     const isFontWeightBold = fontWeightMatch
-                    ? fontWeightMatch[1].replaceAll(/\s/g, "").includes("font-weight:bold;") ||
-                      fontWeightMatch[1].replaceAll(/\s/g, "").includes("font-weight:700;")
-                    : false;
+                        ? fontWeightMatch[1].replaceAll(/\s/g, '').includes('font-weight:bold;') || fontWeightMatch[1].replaceAll(/\s/g, '').includes('font-weight:700;')
+                        : false;
                     const isBold = fontWeightMatch ? isFontWeightBold : true;
                     return isBold ? `*${innerContent}*` : innerContent;
                 },

[ShridharGoel]

@jasperhuangg Should be fixed now, thanks.

[jasperhuangg]

@ShridharGoel Still have linter errors, why aren't you running npm run prettier? Either do that or just correctly apply the diff that I'm showing you 😆

╰─ git diff
diff --git a/lib/ExpensiMark.js b/lib/ExpensiMark.js
index fd54708..917606f 100644
--- a/lib/ExpensiMark.js
+++ b/lib/ExpensiMark.js
@@ -411,7 +411,7 @@ export default class ExpensiMark {
                 replacement: (match, tagContent, innerContent) => {
                     const fontWeightMatch = innerContent.match(/style="(.*?)"/);
                     const isFontWeightBold = fontWeightMatch
-                        ? fontWeightMatch[1].replaceAll(/\s/g, "").includes("font-weight:bold;") || fontWeightMatch[1].replaceAll(/\s/g, "").includes("font-weight:700;")
+                        ? fontWeightMatch[1].replaceAll(/\s/g, '').includes('font-weight:bold;') || fontWeightMatch[1].replaceAll(/\s/g, '').includes('font-weight:700;')
                         : false;
                     const isBold = fontWeightMatch ? isFontWeightBold : true;
                     return isBold ? `*${innerContent}*` : innerContent

[ShridharGoel]

I had used it locally, not sure what went wrong (some changes were applied but double quotes didn't change to single quotes). But it should work now.

[jasperhuangg]

@ShridharGoel it looks like your commits aren't signed, can you look into this?

The base branch requires all commits to be signed

[alitoshmatov]

Can you add comments to explain what's happening it became pretty unclear what's going on.

[alitoshmatov]

I think we also need unit tests to cover this cases. @ShridharGoel Can you work on that

[jasperhuangg]

@ShridharGoel bump!

[jasperhuangg]

Gonna go ahead and stick this back on draft while the unit tests are worked out.

[ShridharGoel]

Will update this soon.

[alitoshmatov]

Thank you @jasperhuangg looks like we need to revisit our solution on this one. @ShridharGoel Can have a look at this Expensify/App#41109 (comment)

[ShridharGoel]

Testing the mentioned scenarios now.

[jasperhuangg]

@ShridharGoel Do you mind also updating your screenshots to handle the new cases outlined in Expensify/App#41109 (comment)

[ShridharGoel]

@jasperhuangg Will update. Also, just to note: Expensify/App#41109 (comment)

Edit: Updated the videos in OP.

[jasperhuangg]

LGTM @alitoshmatov all yours

[alitoshmatov]

Looks good to me to, I think we can merge this.

[github-actions]

🚀Published to npm in v2.0.40