-
Notifications
You must be signed in to change notification settings - Fork 984
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: avatar alignment #19073
fix: avatar alignment #19073
Conversation
Jenkins BuildsClick to see older builds (49)
|
41ab0e8
to
754f6df
Compare
754f6df
to
3beb4b2
Compare
71% of end-end tests have passed
Failed tests (13)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (34)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestDeepLinksOneDevice:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
|
e8061f2
to
38d6481
Compare
94% of end-end tests have passed
Failed tests (2)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityMultipleDeviceMerged:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (45)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
|
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.
Thank you for implementing this animation.
- Avatar is overlapping Name
- Probably we also want to decrease/animate gap between pins banner and top bar
Please request design review before merging.
output-2024-03-16_18.53.41.mp4
1e6c8e4
to
60d178f
Compare
:number-of-lines 1} | ||
display-name] | ||
[contact-icon contact theme]] | ||
[:f> f-username |
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.
Do we still need :f>
?
From ui-guidelines
:f>
not needed anymore, all components are functional by default
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 :f>
is not needed anymore
@@ -133,6 +132,28 @@ | |||
:profile-picture profile-picture | |||
:size :big}]])) | |||
|
|||
(defn f-username |
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.
Since all components are functional by default, the f-
naming convention doesn't make sense
cc: @flexsurfer
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.
yes thanks
60d178f
to
aff946e
Compare
92% of end-end tests have passed
Failed tests (3)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (44)Click to expandClass TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestDeepLinksOneDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePRTwo:
|
aff946e
to
021060d
Compare
Hi @BalogunofAfrica! Thanks for your implementation! So will adress it to the design team. |
@BalogunofAfrica please tag me when I can retest 🙏 |
fa87a5d
to
c2bc334
Compare
Hi @mariia-skrypnyk You can re test. Android Jank is related to #19387. |
Hi @Francesca-G, I have addressed all comments save the dot size. For that, the identicon and dot are generated as one single image, which is what we can animate to scale. We can not animate the dot separately. |
can we create a separate issue to find a solution to this? 🙏 |
we already have a similar open issue for indicator size #18257 |
Thanks @mohsen-ghafouri 🙏 cc: @Francesca-G |
c2bc334
to
879bc1e
Compare
@mariia-skrypnyk Can this be merged? |
@BalogunofAfrica yes, please! |
879bc1e
to
25c1325
Compare
Co-authored-by: balogunofafrica <balogunakanbi.k@gmail.com>
Fixes #18827
Summary
Handles the limit event when creating a new group chat
Platforms
Functional
Steps to test
status: ready