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
Optimized ScrollView instead of FlatList for desktop chat #8164
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good overall
Jenkins BuildsClick to see older builds (21)
|
da93449
to
ccc9f93
Compare
Pull Request Checklist
|
@vkjr what about other places where flatlist is used? tbh i thought that we make changes in flatlist component itself? |
i mean something like
|
@flexsurfer, flatlist shows bad performance when there are a lot of elements and a lot of dynamic loads\unloads needed (lot of scrolling). So far it looks like in other places except chat it doesn't affect user experience. If you think that it would be a more clean solution - I can substitute it everywhere but I also think that it would be good to reduce the surface where mobile and desktop code differs as much as possible. |
@flexsurfer @vkjr in this discussion I would accept @vkjr's side, taking into accoun that that is a temporary measure and we dont want to spend too much time fixing potential regressions by replacing |
ok then :) |
98% of end-end tests have passed
Failed tests (1)Click to expand
Passed tests (48)Click to expand
|
@vkjr 1) Input is not erased after sending messagesSteps:
2)Can't fetch the previous historySteps:
3)Share my code is not visibleSteps:
Didn't notice issues with mobile / regular desktop builds. |
Also in mobile UI you can notice that text input doesn't reset to blank if message sent with send icon. It is expected behavior for now. In main branch of |
@vkjr Can't get build |
Some problem on last |
@jakubgs, could you please take a look? I see |
@vkjr can you rebase please? |
Please rabase on |
a13ccdb
to
a6669da
Compare
Done |
98% of end-end tests have passed
Failed tests (1)Click to expand
Passed tests (48)Click to expand |
Tested briefly (creating accounts, chatting) on Linux Ubuntu 18.04, Windows 10 Home, Mac OSx (normal desktop builds) and on IOS 11.4.1 (IPhone 7), Android 8 (LG V20) - no regression is found. |
100% of end-end tests have passed
Passed tests (1) |
Signed-off-by: Andrey Shovkoplyas <motor4ik@gmail.com>
a6669da
to
add7597
Compare
fixes #8163
Summary
FlatList implementation in react-native-desktop works slow and makes chat in mobile ui for desktop unusable
As a workaround, until FlatList implementation is buggy we can use ScrollView component that has optimization on Qt side of react-native-desktop.
Review notes
ScrollView
that used instead ofFlatList
taken from current desktop UI and works the same wayTesting notes
Currently in new mobile UI you can't open user profile by clicking on his avatar. This is a known issue. I have a solution for it in
react-native-desktop
but this solution causes bug in old desktop UI which is default now (only part of avatar image responds to click.)My suggestion is to leave this bug for now and fix it before we fully switch to new UI
Platforms
Areas that maybe impacted
Only chat area should be impacted
Functional
Steps to test
MOBILE_UI_FOR_DESKTOP=1
in.env
filestatus: ready