Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Make text on native platforms non-selectable #2342

Closed
wants to merge 10 commits into from

Conversation

roryabraham
Copy link
Contributor

@roryabraham roryabraham commented Apr 10, 2021

Details

  • Extracts two new components, ReportActionItemCommentFragment and ReportActionItemTextFragment, which are the same on web/desktop and native, with one difference. Native text is not selectable, while web/desktop text is selectable. This fixes the bug linked in the description, without breaking native copy/paste functionality on web/desktop, where most users would use a mouse instead of the context menu to copy/paste text anyways.
  • There was one other common bug that occurred where, when the keyboard was open, the first tap would close the keyboard and the second longPress would open the ReportActionContextMenu. I was able to fix this so that the first longPress closes the keyboard and opens the ReportActionContextMenu in one go, using the keyboardShouldPersistTaps prop of FlatList (inherited from ScrollView I believe).

Fixed Issues

Fixes #2279

Tests / QA Steps (Web/Desktop)

  1. Make sure you can highlight text and copy/paste like normal.
  2. Make sure that right-clicking a chat opens the ReportActionContextMenu.
  3. Make sure that clicking anywhere outside the ReportActionContextMenu closes it.

Tests / QA Steps (iOS/Android)

  1. Open a report. If necessary click on the chat composer to open the keyboard.
  2. Tap above the keyboard - make sure the keyboard is dismissed.
  3. Open the keyboard again.
  4. Swipe the keyboard down - make sure you can dismiss it by swiping.
  5. Long-press a chat, make sure the ReportActionContextMenu opens
  6. Tap outside or swipe to close the ReportActionContextMenu
  7. Open the keyboard again.
  8. Long-press a chat, verify that the keyboard closes and the ReportActionContextMenu opens. You should not see the native platform's "copy" popup, as shown in the linked issue.
  9. Copy the message.
  10. Paste the message somewhere to verify that the correct message was copied to the clipboard.
  11. Open the keyboard again.
  12. Long-press a user's name in the chat. Verify that the keyboard closes and the ReportActionContextMenu opens. You should not see the native platform's "copy" popup, as shown in the linked issue.

Tests / QA Steps (mWeb)

On mobile web, this PR should cause no notable difference.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

image

Mobile Web

Desktop

image

iOS

Android

@roryabraham roryabraham requested a review from a team April 10, 2021 08:54
@roryabraham roryabraham self-assigned this Apr 10, 2021
@MelvinBot MelvinBot requested review from Gonals and removed request for a team April 10, 2021 08:54
@roryabraham roryabraham changed the title Make text on native platforms non-selectable [WIP] Make text on native platforms non-selectable Apr 10, 2021
@roryabraham
Copy link
Contributor Author

Somehow this is breaking fonts (on web at least, probably other platforms too):
image

@roryabraham roryabraham requested a review from a team as a code owner April 16, 2021 22:14
@MelvinBot MelvinBot requested review from nkuoch and removed request for a team April 16, 2021 22:14
@roryabraham
Copy link
Contributor Author

Okay, fixed the font error ... I was doing import {Text} from 'react-native' instead of import Text from '../../components/Text' 🤦🏼‍♂️

@roryabraham
Copy link
Contributor Author

Losing my mind trying to figure out how to prevent icons from being selectable:

image

@stitesExpensify stitesExpensify requested review from a team and removed request for nkuoch April 20, 2021 19:23
@MelvinBot MelvinBot requested review from joelbettner and removed request for a team April 20, 2021 19:24
@joelbettner
Copy link
Contributor

Is this still being worked on? Should I review it even though it is a WIP?

@roryabraham
Copy link
Contributor Author

Is this still being worked on

Honestly, this is pretty decayed at this point and never really worked on mWeb. Since this can be worked on externally, I'm actually going to close it out and we can get the original issue exported to Upwork. Hopefully the work in this PR can still be a reference for the contributor that's hired.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

iOS/Android - Message - Copy message menu is not displayed when pressing on the message
2 participants