Don't load chat view until chat is fetched #19988
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Parveshdhull
commented
May 12, 2024
| @@ -114,20 +114,20 @@ | |||
|
|
|||
| (defn list-footer-avatar | |||
| [{:keys [distance-from-list-top display-name online? profile-picture theme group-chat color | |||
| emoji chat-type chat-name last-message]}] | |||
There was a problem hiding this comment.
For chat screen we have a special subscription :chats/current-chat-chat-view which returns selected keys only to avoid unnecessary re-renders.
But in the sub we are also returning :last-message, which I think is one the most active key in chat object.
Jenkins BuildsClick to see older builds (12)
|
seanstrom
approved these changes
May 13, 2024
flexsurfer
approved these changes
May 13, 2024
|
can you include a video/ some screenshots of how this looks? |
J-Son89
reviewed
May 13, 2024
| (let [scale (reanimated/interpolate | ||
| distance-from-list-top | ||
| [0 (if (seq last-message) messages.constants/header-container-top-margin 0)] | ||
| [0 (if empty-chat? 0 messages.constants/header-container-top-margin)] |
There was a problem hiding this comment.
no strong opinions but maybe it's better to abstract to a helper function?
J-Son89
reviewed
May 13, 2024
85% of end-end tests have passedFailed tests (6)Click to expandClass TestDeepLinksOneDevice:
Class TestWalletMultipleDevice:
Class TestWalletOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Expected to fail tests (2)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Passed tests (44)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestWalletOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
|
Parveshdhull
force-pushed
the
fix/channel-fetching
branch
from
48df59e
to
11c3b50
Compare
|
Thank you @ilmotta and @J-Son89 for reviewing PR. It seems status-mobile/src/status_im/subs/chats.cljs Line 134 in d658fcf So updated the subscription to
Updated the PR description with the video. thank you |
Parveshdhull
commented
May 14, 2024
Comment on lines
6
to
5
| ["@walletconnect/web3wallet" :refer [Web3Wallet]] | ||
| (:require ["@walletconnect/core" :refer [Core]] | ||
| ["@walletconnect/react-native-compat"] |
There was a problem hiding this comment.
File updated by make lint-fix
77% of end-end tests have passedFailed tests (10)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestDeepLinksOneDevice:
Class TestWalletMultipleDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Expected to fail tests (2)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Passed tests (40)Click to expandClass TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestWalletOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
ilmotta
approved these changes
May 14, 2024
|
@Parveshdhull thank you for the PR! It looks good to me. The only problem I am thinking about is that there is no way to close this screen on IOS while cannel is in loading state. On Android we can only go back by using system back button. Maybe we still need to show close button in top left corner? WDYT?
|
yes, that sounds good. Pushing changes with close button |
Parveshdhull
force-pushed
the
fix/channel-fetching
branch
from
11c3b50
to
9888844
Compare
done, please let me if looks ok |
Thanks @Parveshdhull! Looks good to me. Please, take a look at another issue ISSUE 1 Skeleton remains after channel has been fetched, channel icon, header are wrongSteps:
Actual result: Skeleton remains after channel has been fetched, channel icon is wrong
telegram-cloud-document-2-5345893055427790036.mp4 |
Parveshdhull
force-pushed
the
fix/channel-fetching
branch
from
9888844
to
0b593a4
Compare
|
Thanks for the PR @Parveshdhull! It is ready for merge now. ISSUE 1 is not related to this PR and is fixed by @Parveshdhull here #20093 |
Parveshdhull
force-pushed
the
fix/channel-fetching
branch
from
0b593a4
to
bc5c172
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #19659
Video
output-2024-05-14_17.13.34.mp4
Testing
Issue 1, 2, 3 should be fixed
status: ready