-
Notifications
You must be signed in to change notification settings - Fork 985
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
Implement gradient cover
component
#16778
Conversation
Jenkins BuildsClick to see older builds (20)
|
src/status_im2/contexts/quo_preview/gradient/gradient_cover.cljs
Outdated
Show resolved
Hide resolved
src/status_im2/contexts/quo_preview/gradient/gradient_cover.cljs
Outdated
Show resolved
Hide resolved
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.
nice one @smohamedjavid! :)
183665d
to
5104106
Compare
@Francesca-G - Can you help us do a design review on this PR? |
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.
🚀
5104106
to
d46b048
Compare
@Francesca-G - Appreciate your review of this component. |
Hello, sorry for the late reply. Due to issues related to some PRs I'm not able to review this for the time being, the PR instantly crashes after being installed. They're investigating to solve it and as soon as I'm able to view it correctly I will review this :) |
Interesting! 🤔 @Francesca-G - Can you try build |
it works now, thank you 🙏 |
yeah its my mistake actually, in PR #16803 (comment) |
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.
Everything looks good about the component itself.
The Yin Yan style (black and white) is missing from the color options, I don't know if you're planning on implementing that.
Also, I noticed this happening on Android: a half circle with opacity on the selected color (this happened with every color).
Full screenshot:
Hope this helps :)
Thanks a lot for the quick review @Francesca-G Duly appreciate it!
Last month, we temporarily removed the Yin Yan (black and white) color as it caused some issues with syncing between Desktop and Mobile. We will add that color back into the customization color palette once the issue is resolved.
The Can you check the |
@Francesca-G - Can you help us check the |
I was only able to check this PR #16781 (comment) and this one also has the same issue with the color picker. Unfortunately I don't have any other build ready for design review at the moment. |
Thanks, @Francesca-G for checking other builds. It looks like this component is not behaving as expected in your device. Please help us raise a bug as this issue is more of device-specific. Additionally, add a description regarding your phone model. This will help us debug the issue. I hope the PR can be merged. If yes, approval would be much appreciated. |
4b88f84
to
d4ece43
Compare
90% of end-end tests have passed
Failed tests (4)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Passed tests (36)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePR:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
|
Signed-off-by: Mohamed Javid <[email protected]>
Signed-off-by: Mohamed Javid <[email protected]>
Signed-off-by: Mohamed Javid <[email protected]>
Signed-off-by: Mohamed Javid <[email protected]>
Signed-off-by: Mohamed Javid <[email protected]>
Signed-off-by: Mohamed Javid <[email protected]>
Signed-off-by: Mohamed Javid <[email protected]>
Signed-off-by: Mohamed Javid <[email protected]>
Signed-off-by: Mohamed Javid <[email protected]>
d4ece43
to
7f089ea
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.
Created a separate issue #16815 for the color picker bug on Android
Thanks @Francesca-G! |
fixes #16607
Summary
This PR implements the
gradient cover
component which is needed for wallet screen development.Design
Link to Figma
Platforms
Steps to test
Quo2 Preview > gradient > gradient cover
status: ready