Web - Chat - Pasted message not always displays no hyperlink format w… by prakashbask · Pull Request #55932 · Expensify/App

prakashbask · 2025-01-29T09:38:02Z

Explanation of Change

Removed AnchorTag regex check in ContextMenuActions.tsx so that markdown is copied to Clipboard. This will solve the problem in another issue
Update logic for parsing markdown and convert to plain text in the sequence markdown -> html -> plain text. This change is done in useHtmlPaste/index.ts

Fixed Issues

$ #53718 , #53832
PROPOSAL: #53718 (comment) , #53832 (comment)

Tests

Test Scenario 1

At any chat, type hello and send message
Click three dots of message action menu
Click "Copy to clipboard"
At compose box, right click mouse and select "Paste as plain text"
Send message
Repeat 3 times from step 2 to step 6
All steps should paste the content in plain text format

Test Scenario 2

At any chat, type # Heading1 or ~~strike~~ or italic or bold and send message
Click "Copy to clipboard"
Go to Android Hybrid App
At any chat in compose box, paste from clipboard
Markdown should be preserved in the pasted content

Verify that no errors appear in the JS console

Offline tests

NA

QA Steps

Same as Tests

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

androidHybrid.mov

Android: mWeb Chrome

androidChrome.mp4

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

macOSSafari.mp4

MacOS: Desktop

macOSDesktop.mp4

melvin-bot · 2025-01-29T09:38:07Z

@Pujan92 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

Pujan92 · 2025-01-31T08:00:20Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android: Native
- Android: mWeb Chrome
- iOS: Native
- iOS: mWeb Safari
- MacOS: Chrome / Safari
- MacOS: Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label and/or tagged @Expensify/design so the design team can review the changes.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native

ma1.webm

Android: mWeb Chrome

ma1.webm

iOS: Native

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2025-02-06.at.11.43.39.mp4

iOS: mWeb Safari

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2025-02-06.at.11.43.39.mp4

MacOS: Chrome / Safari

Screen.Recording.2025-02-03.at.17.25.03.mov

MacOS: Desktop

Pujan92 · 2025-01-31T08:26:12Z

@prakashbask Please merge branch with the latest main and consider checking on this once.

prakashbask · 2025-01-31T08:35:49Z

@prakashbask Please merge branch with the latest main and consider checking on this once.

@Pujan92 I don't have access to Slack as it has not been provided yet.

Pujan92 · 2025-01-31T08:37:41Z

Ok, let's merge main and if we found those bad commits then follow this slack thread.

Thanks for your patience everyone! We can resume merging E/App PRs now. :greenlight:
Furthermore, we had to do a force-push on main to get rid of a bad commit. This means that many of you may experience that your main appears to be ahead of the remote by 67 commits. Should you encounter that, please address it by running:
git fetch origin main
git reset --hard origin/main

# note that git pull will not work. You need to do the reset --hard, or delete the branch and recreate it from the remote
If you have a feature branch that was built on top of the deleted version of main, you'll also see those 67 commits appear in your PR. It is CRITICAL that even if there's no errant diff in your PR that these commits ARE NOT MERGED.
If you find yourself in this situation, you have a few options to address it. One approach is to:
Create a temporary branch off of main
git checkout main
git pull
git checkout -b temp
Cherry-pick the commits that you actually wrote in your feature branch to your temporary branch
git cherry-pick <your_commit_hash>

# you can pass just one commit hash,
# or <your_commit_hash>^5 for your hash and the 5 preceeding it
# or <start_commit_hash>...<end_commit_hash> for a range of commits
# (any valid ref works)
Resolve any conflicts if you have to, using git cherry-pick --continue as needed
hard-reset your feature branch to the temp branch and force-push
git checkout my_feat
git reset --hard temp
git push --force origin my_feat
git branch -D temp
If all that sounds like more git than you care to use, you can always just create a fresh branch off of an up-to-date main, rewrite your code, and discard your old branch.
Apologies again for the inconvenience. If you are confused or stuck, I'm happy to help :slightly_smiling_face: (edited)

…hen paste as plain text

prakashbask · 2025-01-31T09:18:35Z

@Pujan92 Ok. I was able to sync and pull to my main without any issues as my main was behind. Hence, I have rebased my feature branch and pushed it again.

Pujan92 · 2025-02-03T13:49:05Z

@prakashbask I see an issue with copying from native and pasting it to a web(bcoz it will fall into handlePastePlainText case).

issue.webm

prakashbask · 2025-02-04T15:51:25Z

@prakashbask I see an issue with copying from native and pasting it to a web(bcoz it will fall into handlePastePlainText case).

issue.webm

@Pujan92 Yes. This is an issue from native -> web pasting. We may have to set a context here to preserve the original text.

App/src/libs/Clipboard/index.native.ts

Line 8 in bfb97c5

Clipboard.setString(text);

    const textToCopy = {
         preserveOriginal: true,
         content: text
    }
   Clipboard.setString(JSON.stringify(textToCopy));

This preserveOriginal flag will then be used in handlePastePlainText to determine if conversion to plain text is required or fallback to default case.

Any other suggestions?

Pujan92 · 2025-02-05T08:59:29Z

This preserveOriginal flag will then be used in handlePastePlainText to determine if conversion to plain text is required or fallback to default case.

I am not getting how that context will help here. Bcoz it depends on what paste option the user has selected.

prakashbask · 2025-02-05T15:14:55Z

This preserveOriginal flag will then be used in handlePastePlainText to determine if conversion to plain text is required or fallback to default case.

I am not getting how that context will help here. Bcoz it depends on what paste option the user has selected.

react-native-clipboard/clipboard supports setting the string in text/plain option. There is no text/html option available. Hence, handlePastePlainText function will be called irrespective of the paste option the user has selected.
But I feel this approach of preserveOriginal will not be a good idea as paste outside of the Expensify/App will not work properly.
So my proposal will be to document native -> web paste as a known limitation and look for other clipboard options like expo-clipboard as a separate feature.

Pujan92 · 2025-02-05T16:29:41Z

-                paste(plainText);
+            const markdownText = event.clipboardData?.getData('text/plain');
+            if (markdownText) {
+                paste(Parser.htmlToText(Parser.replace(markdownText)));


Suggested change

paste(Parser.htmlToText(Parser.replace(markdownText)));

if(isMobile) {

paste(markdownText);

} else {

paste(Parser.htmlToText(Parser.replace(markdownText)));

}

One option is to check for isMobile, if it is a mobile then don't convert it to the plainText by considering we don't have an option "Paste as plain text" in mobile.

Pujan92 · 2025-02-06T04:59:46Z

 import {useCallback, useEffect} from 'react';
 import Parser from '@libs/Parser';
 import CONST from '@src/CONST';
+import isMobile from '@src/utils/isMobile';


Suggested change

import isMobile from '@src/utils/isMobile';

import isMobile from '@libs/Browser';

We can use isMobile from @libs/browser

Pujan92 · 2025-02-06T05:03:56Z

-                paste(plainText);
+            const markdownText = event.clipboardData?.getData('text/plain');
+            if (markdownText) {
+                const parsedText = isMobile() ? markdownText : Parser.htmlToText(Parser.replace(markdownText));


Please add a comment about why we need to do this and also provide a link to the issue in the comment

Added comment and mentioned issue number.

Pujan92

LGTM!

@jasperhuangg plz consider checking the change we need to make to fulfill all platforms.

OSBotify · 2025-02-06T17:14:49Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2025-02-06T22:01:46Z

🚀 Deployed to staging by https://github.com/jasperhuangg in version: 9.0.95-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅
🤖🔄 android HybridApp 🤖🔄	success ✅
🍎🔄 iOS HybridApp 🍎🔄	success ✅

kavimuru · 2025-02-07T02:53:56Z

Failing in mweb with original bug id #53718
https://platform.applause.com/services/links/v1/external/0472be27f7c042284379f4412bc08f7e9bea9e4e8d478f4c298e56b3bfa1f4fe

This reverts commit 4987f62, reversing changes made to beafd48.

[CP Staging] Revert "Merge pull request #55932 from prakashbask/fix/53718"

[CP Staging] Revert "Merge pull request #55932 from prakashbask/fix/53718" (cherry picked from commit c912900) (CP triggered by mountiny)

github-actions · 2025-02-11T02:13:05Z

🚀 Deployed to production by https://github.com/Julesssss in version: 9.0.95-6 🚀

platform	result
🤖 android 🤖	true ❌
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅
🤖🔄 android HybridApp 🤖🔄	failure ❌
🍎🔄 iOS HybridApp 🍎🔄	failure ❌

github-actions · 2025-02-11T02:38:32Z

🚀 Deployed to staging by https://github.com/jasperhuangg in version: 9.0.96-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅
🤖🔄 android HybridApp 🤖🔄	success ✅
🍎🔄 iOS HybridApp 🍎🔄	success ✅

izarutskaya · 2025-02-11T10:01:43Z

@prakashbask Failing with original bug id #53718 on Web and mWeb

https://platform.applause.com/services/links/v1/external/6d9c1ab1bc0f8a6ef8b8b200d817a55971a3af484e67bebf17033b38dcb8d402

github-actions · 2025-02-12T01:44:03Z

🚀 Deployed to production by https://github.com/Julesssss in version: 9.0.96-1 🚀

platform	result
🤖 android 🤖	true ❌
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅
🤖🔄 android HybridApp 🤖🔄	failure ❌
🍎🔄 iOS HybridApp 🍎🔄	failure ❌

prakashbask requested a review from a team as a code owner January 29, 2025 09:38

melvin-bot Bot requested a review from Pujan92 January 29, 2025 09:38

melvin-bot Bot removed the request for review from a team January 29, 2025 09:38

prakashbask mentioned this pull request Jan 29, 2025

[Due for payment 2025-03-31] [$250] Web - Chat - Pasted message not always displays no hyperlink format when paste as plain text #53718

Closed

8 tasks

Web - Chat - Pasted message not always displays no hyperlink format w…

4886fb1

…hen paste as plain text

prakashbask force-pushed the fix/53718 branch from ab990de to 4886fb1 Compare January 31, 2025 08:56

This comment was marked as outdated.

Sign in to view

Pujan92 reviewed Feb 5, 2025

View reviewed changes

Added isMobile check when paste as plain text

668e6ae

Pujan92 reviewed Feb 6, 2025

View reviewed changes

prakashbask added 2 commits February 6, 2025 10:39

Removed redundant isMobile util

c97af19

Fixed commit and added comment to explain the changes

9ed76b2

Pujan92 approved these changes Feb 6, 2025

View reviewed changes

melvin-bot Bot requested a review from jasperhuangg February 6, 2025 06:20

jasperhuangg approved these changes Feb 6, 2025

View reviewed changes

jasperhuangg merged commit 4987f62 into Expensify:main Feb 6, 2025

github-actions Bot mentioned this pull request Feb 6, 2025

Deploy Checklist: New Expensify 2025-02-06 #56482

Closed

amyevans mentioned this pull request Feb 6, 2025

[Due for payment 2025-02-19] [Due for payment 2025-02-18] Mark down - Text with mark down is pasted without mark down #56484

Closed

8 tasks

amyevans added a commit that referenced this pull request Feb 7, 2025

Revert "Merge pull request #55932 from prakashbask/fix/53718"

ed18bfd

This reverts commit 4987f62, reversing changes made to beafd48.

amyevans mentioned this pull request Feb 7, 2025

[CP Staging] Revert "Merge pull request #55932 from prakashbask/fix/53718" #56535

Merged

50 tasks

amyevans added a commit that referenced this pull request Feb 7, 2025

Merge pull request #56535 from Expensify/amy-revert-55932

c912900

[CP Staging] Revert "Merge pull request #55932 from prakashbask/fix/53718"

OSBotify pushed a commit that referenced this pull request Feb 7, 2025

Merge pull request #56535 from Expensify/amy-revert-55932

34a17ed

[CP Staging] Revert "Merge pull request #55932 from prakashbask/fix/53718" (cherry picked from commit c912900) (CP triggered by mountiny)

github-actions Bot mentioned this pull request Feb 11, 2025

Deploy Checklist: New Expensify 2025-02-11 #56625

Closed

69 tasks

-                paste(Parser.htmlToText(Parser.replace(markdownText)));
+  if(isMobile) {
+      paste(markdownText);
+  } else {
+      paste(Parser.htmlToText(Parser.replace(markdownText)));
+  }

	import isMobile from '@src/utils/isMobile';
	import isMobile from '@libs/Browser';

Conversation

prakashbask commented Jan 29, 2025

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

melvin-bot Bot commented Jan 29, 2025

Uh oh!

Pujan92 commented Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer Checklist

Screenshots/Videos

Uh oh!

Pujan92 commented Jan 31, 2025

Uh oh!

prakashbask commented Jan 31, 2025

Uh oh!

Pujan92 commented Jan 31, 2025

Uh oh!

prakashbask commented Jan 31, 2025

Uh oh!

Pujan92 commented Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prakashbask commented Feb 4, 2025

Uh oh!

Pujan92 commented Feb 5, 2025

Uh oh!

prakashbask commented Feb 5, 2025

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

Pujan92 Feb 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pujan92 Feb 6, 2025

Choose a reason for hiding this comment

Uh oh!

Pujan92 Feb 6, 2025

Choose a reason for hiding this comment

Uh oh!

prakashbask Feb 6, 2025

Choose a reason for hiding this comment

Uh oh!

Pujan92 Feb 6, 2025

Choose a reason for hiding this comment

Uh oh!

Pujan92 left a comment

Choose a reason for hiding this comment

Uh oh!

OSBotify commented Feb 6, 2025

Uh oh!

github-actions Bot commented Feb 6, 2025

Uh oh!

kavimuru commented Feb 7, 2025

Uh oh!

github-actions Bot commented Feb 11, 2025

Uh oh!

github-actions Bot commented Feb 11, 2025

Uh oh!

izarutskaya commented Feb 11, 2025

Uh oh!

github-actions Bot commented Feb 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Pujan92 commented Jan 31, 2025 •

edited

Loading

Pujan92 commented Feb 3, 2025 •

edited

Loading

Pujan92 Feb 5, 2025 •

edited

Loading