-
Notifications
You must be signed in to change notification settings - Fork 992
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
feat: contact CustomizationColor #19087
Conversation
Jenkins BuildsClick to see older builds (151)
|
2b3f0b6
to
995aaa0
Compare
995aaa0
to
5822151
Compare
8ac7afd
to
c909009
Compare
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.
Hey looks good! Thank you 🙌
I left a few small comments, lmk what you think plz 🙏
:params [(or preferred-name display-name name) ""] | ||
:params [(or preferred-name display-name name) "" ""] |
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.
What's the meaning of passing this empty string?
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.
it just means do not update profile customization color
profile color is handled by wakuext_setCustomizationColor
status-go-version.json
Outdated
"version": "v0.176.6", | ||
"commit-sha1": "200d5f054096518848adabca82d56105af97086f", | ||
"src-sha256": "02c04j7dd29wd0sxbk97yfwq8gj3hwjvlrj07b188cqkq08p920f" | ||
"version": "feat/contact-customization-color", | ||
"commit-sha1": "4569864def2153b5609e9548607afcbb631a7ce0", | ||
"src-sha256": "0n8fz0mj2q90wqnijskiavmln7sayvsg13k3mz3l3kbl96kkfc6z" |
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.
Should this version number change after the PR for status-go is merged?
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.
no, I'll merge the status-go PR first after this mobile PR got approved, create a tag in status-go repo and update this version file to that tag
:params [(or preferred-name display-name name) ""] | ||
:params [(or preferred-name display-name name) "" ""] |
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.
Similar question:
What's the meaning of the empty string?
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.
Same thing, I guess wakuext_sendContactUpdates
is actually used as set ens name? not sure about it
c909009
to
c0ce6db
Compare
0fe7ff7
to
292f793
Compare
94% of end-end tests have passed
Failed tests (2)Click to expandClass TestCommunityMultipleDeviceMerged:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (45)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityOneDeviceMerged:
|
Hi @yqrashawn Thank you for PR. Take a look the found issues: ISSUE 2: "jump to" button on the profile page Inherits contact's color Instead of the owner'sSteps:
Actual result:"jump to" buttons inherit the color of Expected result:All buttons should inherit the color of the owner, |
ISSUE 3: The colors of elements in 1-1 chat are not dependent on the color preference of the receiving userSteps:
Actual result:
Expected result:
Refferencе |
ISSUE 4: Avatar color not Immediately updated when selecting color in profile editingSteps to Reproduce:
Actual Result:The avatar color is not changed immediately after selecting a color. avatar.mp4Expected Result:The avatar color should change immediately after selecting a color avatar_nightly.mp4 |
0db1657
to
352797b
Compare
Thanks for the review @VolodLytvynenko, ISSUE 2, 3, 4 are fixed |
56% of end-end tests have passed
Failed tests (20)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (27)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityOneDeviceMerged:
Class TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
|
94% of end-end tests have passed
Failed tests (2)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (45)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestDeepLinksOneDevice:
Class TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
|
hi @yqrashawn Thank you for the PR. After the discussion with design it just turned out that all elements in the opened chat should also inherit the contact accent color, but this can be a follow-up. Let me know if you'll address it in this PR so I can retest. Otherwise, the PR can be merged, and I'll create a follow-up. ISSUE 5: Chat elements do not Inherit contact's colorSteps:
Actual result:
Expected result:Elements in the chat should inherit colour of the contact
Discussed with UI team https://discord.com/channels/624634427930312714/1224719697958670460/1225032265768374352 |
let's merge it first, I got another question about pinned message icon color https://discord.com/channels/624634427930312714/1224719697958670460/1225090831971520522 |
03e8b55
to
ba80cc0
Compare
status-im/status-go@894eb57...0db27a8 Signed-off-by: yqrashawn <[email protected]>
status-im/status-go@00f50a4...96f0490 Signed-off-by: yqrashawn <[email protected]>
for one-to-one chat, use contact color for group chat, use chat color
ba80cc0
to
1040e15
Compare
status-go: status-im/status-go#4869
fixes #18733
Summary
:customization-color
:background-color
with:customization-color
in image serverget-initials-avatar-uri
wakuext_sendContactUpdates
CleanShot.2024-03-12.at.21.33.27.mp4
Review notes
Testing notes
Platforms
Areas that maybe impacted
Steps to test
status: ready