[#17507] fix: image placeholder issue in create-profile #17645
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
mohsen-ghafouri
force-pushed
the
fix/image-placeholder-profile
branch
from
84952c7
to
7b5fd2d
Compare
mohsen-ghafouri
force-pushed
the
fix/image-placeholder-profile
branch
from
7b5fd2d
to
2f3e62d
Compare
Jenkins BuildsClick to see older builds (4)
|
J-Son89
reviewed
Oct 16, 2023
| (rf/sub | ||
| [:profile/onboarding-placeholder-avatar | ||
| @profile-pic])) | ||
| :image-picker-props {:profile-picture @profile-pic |
There was a problem hiding this comment.
hmm, is this right? the sub is there because we want to hydrate the data if the user navigates backwards.
e.g user sets profile picture, then user navigates forward to create password, then user navigates back to create profile page ->
expected the profile picture should be set as before 👍
There was a problem hiding this comment.
Thanks @J-Son89 as I checked if you go to next screen (set password) and the click on back button you still can see the selected avatar photo. please let me know if i should check any other scenario
Simulator.Screen.Recording.-.iPhone.11.Pro.-.2023-10-16.at.13.05.26.mp4
There was a problem hiding this comment.
hi, the or logic here is for rendering initial avatars when no image set
I believe this will break initial avatar
There was a problem hiding this comment.
The reason for the issue might be (just a guess) image server port changed when back to foreground. media server before login is started at https://github.com/status-im/status-go/blob/21ddaa4b9f57288ed99fed04db528f3790b7bd2f/node/get_status_node.go#L210
There was a problem hiding this comment.
@yqrashawn yes as i mentioned the issue is related to the port but i can't see any issue with this changes as user-avatar already handled initial avatar according to full-name.
(if (and full-name (not (or profile-picture-fn profile-picture)))
[initials-avatar props]
...
2f3e62d
to
91188a3
Compare
yqrashawn
approved these changes
Oct 17, 2023
mohsen-ghafouri
force-pushed
the
fix/image-placeholder-profile
branch
from
91188a3
to
6999ee6
Compare
|
Hey @VolodLytvynenko i moved this PR to E2E test step but it didn't run them, JFYI |
Hi @mohsen-ghafouri The e2e run still in process. Sometimes it happens if a few PRs are moved to E2E at the same time which might cause the queue. |
53% of end-end tests have passedFailed tests (20)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Passed tests (23)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
|
|
@mohsen-ghafouri thank you for your work. Issues in e2e are not related to the current PR. Ready to be merged |
J-Son89
approved these changes
Oct 17, 2023
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 #17507
Reproduction
it was hard to reproduce but now it should be fine, i noticed sometime the port has been missed for mediaserver ,so i try to avoid calling mediaserver when user doesn't have any avatar there
status: ready