Copy email url #8195

mateusbra · 2022-03-17T02:20:49Z

Details

Right clicking email address shows pop up that says ~~Copy URL to clipboard~~ Copy e-mail to clipboard.

This PR will change the message we should show when right clicking e-mail to "Copy e-mail to clipboard".

Fixed Issues

$ #8006

Tests

Log in on New Expensify.
Open a report.
Type an URL and an e-mail on chat.
Verify that now e-mails shows "Copy e-mail to clipboard" when you try to copy it.

PR Review Checklist

Contributor (PR Author) Checklist

PR Reviewer Checklist

QA Steps

Web,Desktop:

Log in on New Expensify.
Open a report.
Type an URL and an e-mail on chat.
Right Click an e-mail.
Verify that now e-mails shows "Copy e-mail to clipboard" when you try to copy it.
Right Click an URL.
Verify that URLs still shows "Copy URL to clipboard" when you try to copy it.

Android,iOS,Mobile Web:

Log in on New Expensify.
Open a report.
Type an URL and an e-mail on chat.
Long press an e-mail.
Verify that now e-mails shows "Copy e-mail to clipboard" when you try to copy it.
Long press an URL.
Verify that URLs still shows "Copy URL to clipboard" when you try to copy it.

Screenshots

Web

Windows-Chrome.mp4

MacOs-Chrome.mov

Mobile Web

Android-Chrome.mp4

iOS-Safari.mp4

Desktop

MacOs-Desktop.mov

iOS

iOS-Native.mov

Android

Android-Native.mp4

mateusbra · 2022-03-17T03:32:00Z

As @Santhosh-Sellavel said here, we still should rename fileName prop to fit all its purposes.
So @marcochavezf @Santhosh-Sellavel what name you think we should use to rename it?
Also, do you guys think it should be done in a separatelly PR or it could be made here?

mateusbra · 2022-03-17T03:39:13Z

On Mobile Web when we longpress the URL/e-mail it doesn't show the pop-up(I think it expect user to right click in order to show it, as expected on Web). I reported it on slack here.

Santhosh-Sellavel · 2022-03-17T15:59:01Z

@marcochavezf

My suggestions for prop name instead of fileName is anchorText or displayName or displayLabel. Please post your thoughts on this!

Santhosh-Sellavel · 2022-03-17T16:41:02Z

@mateusbra
Add the new type here

App/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js

Line 104 in 31492d7

* @param {string} type - context menu type [LINK, REPORT_ACTION]

and here

App/src/pages/home/report/ContextMenu/ReportActionContextMenu.js

Line 8 in 31492d7

* @param {string} type - the context menu type to display [LINK, REPORT_ACTION]

src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly/index.js

src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly/index.native.js

src/languages/es.js

marcochavezf · 2022-03-17T17:58:02Z

@marcochavezf

My suggestions for prop name instead of fileName is anchorText or displayName or displayLabel. Please post your thoughts on this!

Hi guys, I see that we're currently using fileName for attachments, and for this case, I think displayName makes more sense. We also need to update AnchorForCommentsOnly (which I think is the other component that uses anchorForCommentsOnlyPropTypes too)

mateusbra · 2022-03-17T17:59:45Z

@mateusbra
Add the new type here

Sorry, I missed this one. Already commited it!

mateusbra · 2022-03-17T18:02:02Z

Hi guys, I see that we're currently using fileName for attachments, and for this case, I think displayName makes more sense. We also need to update AnchorForCommentsOnly (which I think is the other component that uses anchorForCommentsOnlyPropTypes too)

Hi Marco, thanks for the feedback, working on it!

src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly/index.js

src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly/index.native.js

src/components/AnchorForCommentsOnly/anchorForCommentsOnlyPropTypes.js

Santhosh-Sellavel · 2022-03-22T19:19:00Z

Sorry for the delay here.

Santhosh-Sellavel · 2022-03-22T19:22:34Z

PR Reviewer Checklist

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 verified tests pass on all platforms & I tested again on:
- iOS / native
- Android / native
- iOS / Safari
- Android / Chrome
- MacOS / Chrome
- MacOS / Desktop
I verified there are no console errors (if there’s a console error not related to the PR, report it or open an issue for it to be fixed)
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 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 was added in all src/languages/* files
- I verified any copy / text that was added to the app is correct english and approved by marketing by tagging the marketing team on the original GH to get the correct copy.
- 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 are not impacted by changes in this PR (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
I verified all code is DRY
I verified any variables that can be defined as constants (ie. in CONST.js) 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 */
- Any functional components have the displayName property
- 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
- Any internal methods are bound to “this” properly so there are no scoping issues
- Any internal methods bound to “this” are necessary to be bound
- 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 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(themeColors.componentBG)

Santhosh-Sellavel · 2022-03-22T19:24:53Z

@mateusbra Update same QA Steps to Tests also. So it would be easier to understand!

Santhosh-Sellavel · 2022-03-22T19:27:40Z

@marcochavezf

I verified any copy / text that was added to the app is correct english and approved by marketing by tagging the marketing team on the original GH to get the correct copy.

Do we need opinion from the marketing team?

Santhosh-Sellavel

LGTM! Tests well.

@marcochavezf

Seems mWeb is already broken not introduced in this PR. Except for mWeb Everything else tests well!

mateusbra · 2022-03-22T20:49:55Z

@mateusbra Update same QA Steps to Tests also. So it would be easier to understand!

Changed the Tests section, thanks for the review guys!

marcochavezf · 2022-03-23T21:34:02Z

Do we need opinion from the marketing team?

Good question, for this case is fine because we're just changing a word from an existing text (in fact I think we should move this to an interpolated text like Copy ${value} to clipboard if we add another type of value).

marcochavezf

Thank you guys for your effort!

OSBotify · 2022-03-23T21:44:08Z

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

OSBotify · 2022-03-24T21:00:35Z

🚀 Deployed to staging by @marcochavezf in version: 1.1.46-0 🚀

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

michaelhaxhiu · 2022-03-28T19:50:51Z

~~Am I correct in understanding that #8311 is a regression?~~

michaelhaxhiu · 2022-03-28T19:51:33Z

Wait no, this isn't a regression. It's a bug discovery?

Santhosh-Sellavel · 2022-03-28T19:55:48Z

Not it's not a regression, @michaelhaxhiu!

michaelhaxhiu · 2022-03-28T19:58:00Z

yep got it - was just confused it didn't have the "@ - reported by" part in the title. I see what's going on, and was just trying to get an update for the main GH.

OSBotify · 2022-03-30T19:26:53Z

🚀 Deployed to production by @timszot in version: 1.1.46-3 🚀

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

fedirjh · 2023-06-08T23:53:41Z

src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly/index.js

@@ -61,7 +62,7 @@ class BaseAnchorForCommentsOnly extends React.Component {
                        onSecondaryInteraction={
                        (event) => {
                            ReportActionContextMenu.showContextMenu(
-                                ContextMenuActions.CONTEXT_MENU_TYPES.LINK,
+                                Str.isValidEmail(this.props.displayName) ? ContextMenuActions.CONTEXT_MENU_TYPES.EMAIL : ContextMenuActions.CONTEXT_MENU_TYPES.LINK,


This line introduced a regression in #18912 , When an email is linked to a url link. The message display "Copy Email to Clipboard" however the copied text is the url link.

mateusbra added 2 commits March 16, 2022 18:27

fix: show Copy e-mail instead of Copy URL when anchor is an e-mail

989d283

add import Str to native applications

3670301

mateusbra requested a review from a team as a code owner March 17, 2022 02:20

MelvinBot requested review from marcochavezf and Santhosh-Sellavel and removed request for a team March 17, 2022 02:20

Fix ESlint problems

29b0701

Santhosh-Sellavel suggested changes Mar 17, 2022

View reviewed changes

src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly/index.js Outdated Show resolved Hide resolved

src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly/index.native.js Outdated Show resolved Hide resolved

src/languages/es.js Outdated Show resolved Hide resolved

add EMAIL type to @param

8956ffa

mateusbra added 2 commits March 17, 2022 16:09

rename fileName to displayName

4076f1b

change e-mail to email on translations

0b75ee7

mateusbra requested a review from Santhosh-Sellavel March 18, 2022 14:28

Santhosh-Sellavel suggested changes Mar 18, 2022

View reviewed changes

mateusbra added 2 commits March 18, 2022 13:20

revert displayName -> fileName in processDownload function

b6a4fd3

Change comment

6018858

mateusbra requested a review from Santhosh-Sellavel March 19, 2022 00:51

Santhosh-Sellavel approved these changes Mar 22, 2022

View reviewed changes

marcochavezf approved these changes Mar 23, 2022

View reviewed changes

marcochavezf merged commit 9beba91 into Expensify:main Mar 23, 2022

OSBotify mentioned this pull request Mar 24, 2022

Deploy Checklist: New Expensify 2022-03-24 #8305

Closed

69 tasks

kavimuru mentioned this pull request Mar 24, 2022

[HOLD for payment 2023-01-23] [$4000] mWeb - Copy to email/URL is not working correctly - reported by @mateusbra #8311

Closed

mateusbra mentioned this pull request Apr 5, 2022

[$500] Right clicking email address shows pop up that says Copy URL to clipboard #8006

Closed

fedirjh reviewed Jun 8, 2023

View reviewed changes

fedirjh mentioned this pull request Jun 9, 2023

[$1000] Misleading right-click on email linked to URL #18912

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Copy email url #8195

Copy email url #8195

mateusbra commented Mar 17, 2022

mateusbra commented Mar 17, 2022 •

edited

mateusbra commented Mar 17, 2022 •

edited

Santhosh-Sellavel commented Mar 17, 2022

Santhosh-Sellavel commented Mar 17, 2022 •

edited

marcochavezf commented Mar 17, 2022

mateusbra commented Mar 17, 2022

mateusbra commented Mar 17, 2022

Santhosh-Sellavel commented Mar 22, 2022

Santhosh-Sellavel commented Mar 22, 2022 •

edited

Santhosh-Sellavel commented Mar 22, 2022

Santhosh-Sellavel commented Mar 22, 2022

Santhosh-Sellavel left a comment

mateusbra commented Mar 22, 2022

marcochavezf commented Mar 23, 2022

marcochavezf left a comment

OSBotify commented Mar 23, 2022

OSBotify commented Mar 24, 2022

michaelhaxhiu commented Mar 28, 2022 •

edited

michaelhaxhiu commented Mar 28, 2022

Santhosh-Sellavel commented Mar 28, 2022

michaelhaxhiu commented Mar 28, 2022

OSBotify commented Mar 30, 2022

fedirjh Jun 8, 2023

Copy email url #8195

Copy email url #8195

Conversation

mateusbra commented Mar 17, 2022

Details

Fixed Issues

Tests

PR Review Checklist

Contributor (PR Author) Checklist

PR Reviewer Checklist

QA Steps

Screenshots

Web

Mobile Web

Desktop

iOS

Android

mateusbra commented Mar 17, 2022 • edited

mateusbra commented Mar 17, 2022 • edited

Santhosh-Sellavel commented Mar 17, 2022

Santhosh-Sellavel commented Mar 17, 2022 • edited

marcochavezf commented Mar 17, 2022

mateusbra commented Mar 17, 2022

mateusbra commented Mar 17, 2022

Santhosh-Sellavel commented Mar 22, 2022

Santhosh-Sellavel commented Mar 22, 2022 • edited

PR Reviewer Checklist

Santhosh-Sellavel commented Mar 22, 2022

Santhosh-Sellavel commented Mar 22, 2022

Santhosh-Sellavel left a comment

Choose a reason for hiding this comment

mateusbra commented Mar 22, 2022

marcochavezf commented Mar 23, 2022

marcochavezf left a comment

Choose a reason for hiding this comment

OSBotify commented Mar 23, 2022

OSBotify commented Mar 24, 2022

michaelhaxhiu commented Mar 28, 2022 • edited

michaelhaxhiu commented Mar 28, 2022

Santhosh-Sellavel commented Mar 28, 2022

michaelhaxhiu commented Mar 28, 2022

OSBotify commented Mar 30, 2022

fedirjh Jun 8, 2023

Choose a reason for hiding this comment

mateusbra commented Mar 17, 2022 •

edited

mateusbra commented Mar 17, 2022 •

edited

Santhosh-Sellavel commented Mar 17, 2022 •

edited

Santhosh-Sellavel commented Mar 22, 2022 •

edited

michaelhaxhiu commented Mar 28, 2022 •

edited