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

Fix text copied in reverse order on inverted flatlist #5561

Closed
wants to merge 5 commits into from

Conversation

azimgd
Copy link
Contributor

@azimgd azimgd commented Sep 28, 2021

@thienlnam
This is a draft in attempt to move progress on #3381 a bit further. Please note that this is by no means a final PR, this is simply a demo to let QA team validate there are no regressions by using this approach.
I used patched version of [email protected] with fixes for copying text into buffer in reversed order;

React native web diff: https://github.com/azimgd/react-native-web/pull/1/files

Details

I have removed flatlist inversion (by inverted=true prop) which used [transform: translate] css hack and instead adding flatlist array items in reverse order [unshift instead of push]; It also reverses ScrollView event response received from browser [e.nativeEvent.configOffset *= -1];

Fixed Issues

#3381

Tests

  1. run npm install which will patch react-native-web package
  2. npm run web
  3. load a chat with 200 messages
  4. scrolling up and down (with debugger enabled) works fine
  5. copying text into buffer preserves order

QA Steps

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Screen.Recording.2021-08-09.at.01.16.16.mov

Mobile Web

Desktop

iOS

Android

…xt into buffer in reversed order; I have removed flatlist inversion (by inverted=true prop) which used [transform: translate] css hack and instead adding flatlist array items in reverse order [unshift instead of push]; It also reverses ScrollView event response received from browser [e.nativeEvent.configOffset *= -1];
@azimgd azimgd requested a review from a team as a code owner September 28, 2021 17:32
@MelvinBot MelvinBot requested review from thienlnam and removed request for a team September 28, 2021 17:32
@github-actions
Copy link
Contributor

github-actions bot commented Sep 28, 2021

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

@azimgd
Copy link
Contributor Author

azimgd commented Sep 28, 2021

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

@thienlnam
Copy link
Contributor

thienlnam commented Oct 22, 2021

Hey @azimgd, I've checked out this branch and tested the changes and it doesn't appear that that issues are fixed in the github issue #3381. The order of chats is the same but the symptoms of a css inverted flat list are still occurring.

  1. Copying is still sporadic and jumps around
  2. Pasting is still in the wrong order. If you send multiple messages
    1
    2
    3
    4
    5

and copy it, pasting it should not result in 54321 but 12345

@thienlnam
Copy link
Contributor

thienlnam commented Oct 22, 2021

Hmm, I see that in the video that is the behavior that we would expect but I don't seem to be getting the same results on your branch - is there something I'm missing?

I've double checked that I'm on your branch and have run npm i
Screen Shot 2021-10-22 at 4 43 27 PM

However, here are my results on localhost
https://user-images.githubusercontent.com/30609178/138533847-68205480-7bcf-459d-aa95-ca2b9743754b.mp4

Add postinstall script with patch-package execution, to update react-native-web repo inside node_modules which fixes inverted FlatList
@azimgd
Copy link
Contributor Author

azimgd commented Oct 25, 2021

@thienlnam patch-package was probably not applied by npm. Added a postinstall script to fix that.
note: actual PR will be against expensify/react-native-web once the green light is received.

@thienlnam
Copy link
Contributor

This is working pretty well - only thing I'm noticing right now is that the scroll event is opposite - can we have the page scroll in the same direction (as whatever the user set as default?). The scrolling is opposite of what is happening in the current app

Remove obsolete scroll wheel inversion (w/ invertedWheelEvent) as FlatList supports native list inversion now;
@azimgd
Copy link
Contributor Author

azimgd commented Oct 26, 2021

@thienlnam Removed obsolete scroll wheel inversion (w/ invertedWheelEvent) as FlatList supports native list inversion now.
ead40c6

@thienlnam
Copy link
Contributor

Awesome, this tests well!

@azimgd
Copy link
Contributor Author

azimgd commented Nov 9, 2021

closing in favor of #6268

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.

2 participants