fix user select style on anchor tag #8757

mollfpr · 2022-04-23T07:35:16Z

Details

adding style userSelect: 'text' to BaseHTMLEngineProvider and BaseAnchorForCommentsOnly.

Fixed Issues

$ #8421

Tests

Send someone a text below
There are a bunch of wizard type tools online you can use to create the SPF and DMARC records:
https://www.spf-record.com/generator
https://dmarcian.com/dmarc-record-wizard/
Try to select & highlight the sent message
Verify able to select & highlight the sent message

Verify that no errors appear in the JS console

PR Review Checklist

Contributor (PR Author) Checklist

PR Reviewer Checklist

QA Steps

Send someone a text below
There are a bunch of wizard type tools online you can use to create the SPF and DMARC records:
https://www.spf-record.com/generator
https://dmarcian.com/dmarc-record-wizard/
Try to select & highlight the sent message
Verify able to select & highlight the sent message

Verify that no errors appear in the JS console

Screenshots

Web

Screen.Recording.2022-04-23.at.13.58.47.mov

Mobile Web

Screen.Recording.2022-04-23.at.14.15.03.mov

Screen.Recording.2022-04-23.at.14.22.23.mov

Desktop

Screen.Recording.2022-04-23.at.14.07.29.mov

iOS

Screen.Recording.2022-04-23.at.14.14.26.mov

Android

Screen.Recording.2022-04-23.at.14.20.30.mov

Santhosh-Sellavel · 2022-04-25T18:15:05Z

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 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 */
- 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
- 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 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)
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.

Santhosh-Sellavel · 2022-04-25T18:20:11Z

@mollfpr

Block/highlight the content message

Verified that the link is got blocked/highlighted on WEB and Desktop

It could be better,

Try to select & highlight the sent message
Verify able to select & highlight the sent message

Santhosh-Sellavel

Tests well requested few changes.

cc: @deetergp

Santhosh-Sellavel · 2022-04-25T19:09:14Z

src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js

@@ -36,7 +36,7 @@ const customHTMLElementModels = {
    }),
 };

-const defaultViewProps = {style: {alignItems: 'flex-start'}};
+const defaultViewProps = {style: {alignItems: 'flex-start', userSelect: 'text'}};


We could refactor this to use the newly added style, instead of hardcoding here.

Possibly refactor both , use Styles.alignItemsStart instead of alignItems: 'flex-start'

mollfpr · 2022-04-26T01:16:17Z

updated and pulled from main

cc @Santhosh-Sellavel @deetergp

deetergp

Looks good to me. @Santhosh-Sellavel, since you requested changes, I'll leave it for you to give your seal of approval.

Santhosh-Sellavel

LGTM, tests well!

Santhosh-Sellavel · 2022-04-28T21:33:30Z

@deetergp

OSBotify · 2022-04-28T23:50:27Z

✋ 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-05-09T20:04:21Z

🚀 Cherry-picked to staging by @sketchydroide in version: 1.1.57-8 🚀

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

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

OSBotify · 2022-05-13T16:13:17Z

🚀 Deployed to production by @chiragsalian in version: 1.1.57-17 🚀

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

fix user select style on anchor tag

639e61f

mollfpr requested a review from a team as a code owner April 23, 2022 07:35

melvin-bot bot requested review from deetergp and Santhosh-Sellavel and removed request for a team April 23, 2022 07:35

Santhosh-Sellavel suggested changes Apr 25, 2022

View reviewed changes

mollfpr added 2 commits April 26, 2022 08:04

Merge branch 'main' of https://github.com/mollfpr/App into fix-8421

2256f91

refactor the styles

e709343

deetergp approved these changes Apr 27, 2022

View reviewed changes

mollfpr requested a review from Santhosh-Sellavel April 28, 2022 02:46

Santhosh-Sellavel approved these changes Apr 28, 2022

View reviewed changes

deetergp merged commit d1f3643 into Expensify:main Apr 28, 2022

OSBotify mentioned this pull request May 10, 2022

Deploy Checklist: New Expensify 2022-04-25 #8777

Closed

This was referenced May 12, 2022

[med] Safari + VoiceOver: Entered value on the text field is not announced. #8619

Closed

[med] Chrome + Jaws: Success message after uploading image is not announced. #8624

Closed

melvin-bot bot mentioned this pull request May 13, 2022

[HOLD for payment 2022-05-20] [$500] when selecting multiple lines on a message, sometimes the lines that aren't the first aren't highlighted #8421

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix user select style on anchor tag #8757

fix user select style on anchor tag #8757

mollfpr commented Apr 23, 2022 •

edited

Loading

Santhosh-Sellavel commented Apr 25, 2022 •

edited

Loading

Santhosh-Sellavel commented Apr 25, 2022

Santhosh-Sellavel left a comment •

edited

Loading

Santhosh-Sellavel Apr 25, 2022

Santhosh-Sellavel Apr 25, 2022

mollfpr commented Apr 26, 2022 •

edited

Loading

deetergp left a comment

Santhosh-Sellavel left a comment

Santhosh-Sellavel commented Apr 28, 2022

OSBotify commented Apr 28, 2022

OSBotify commented May 9, 2022

OSBotify commented May 13, 2022

fix user select style on anchor tag #8757

fix user select style on anchor tag #8757

Conversation

mollfpr commented Apr 23, 2022 • edited Loading

Details

Fixed Issues

Tests

PR Review Checklist

Contributor (PR Author) Checklist

PR Reviewer Checklist

QA Steps

Screenshots

Web

Mobile Web

Desktop

iOS

Android

Santhosh-Sellavel commented Apr 25, 2022 • edited Loading

PR Reviewer Checklist

Santhosh-Sellavel commented Apr 25, 2022

Santhosh-Sellavel left a comment • edited Loading

Choose a reason for hiding this comment

Santhosh-Sellavel Apr 25, 2022

Choose a reason for hiding this comment

Santhosh-Sellavel Apr 25, 2022

Choose a reason for hiding this comment

mollfpr commented Apr 26, 2022 • edited Loading

deetergp left a comment

Choose a reason for hiding this comment

Santhosh-Sellavel left a comment

Choose a reason for hiding this comment

Santhosh-Sellavel commented Apr 28, 2022

OSBotify commented Apr 28, 2022

OSBotify commented May 9, 2022

OSBotify commented May 13, 2022

mollfpr commented Apr 23, 2022 •

edited

Loading

Santhosh-Sellavel commented Apr 25, 2022 •

edited

Loading

Santhosh-Sellavel left a comment •

edited

Loading

mollfpr commented Apr 26, 2022 •

edited

Loading