-
Notifications
You must be signed in to change notification settings - Fork 987
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
[WIP] Share Tab (Shell Profile Only) #15528
Conversation
5e0bba0
to
6b00bd1
Compare
Jenkins BuildsClick to see older builds (130)
|
8812845
to
3d1b070
Compare
324831b
to
0975786
Compare
a87fc62
to
e52e150
Compare
(defn screen-container | ||
[window-width top bottom] | ||
{:flex 1 | ||
:width window-width |
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.
:width window-width
why this needed?
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.
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.
seems this use of window-width is still here? this should be adjusted too if possible 👍
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.
This is just for the wallet-tab at the moment, it will go away once that comes in
6dd9897
to
5111d2f
Compare
8c7d057
to
fc677da
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.
aside from the open discussion looks good to me 👍
one other thing I noticed when I checked out your branch is that the background color is a solid color and in designs it should be a blurred overlay.. although perhaps this is beyond the scope of this pr? 🤔
It is a blurred overlay currently, It may look like a solid background because of the dark starting screen, try checking the share view on some other screen, maybe messaging screen, it looks like this : |
ffa4412
to
95cb4fb
Compare
hey @siddarthkay i see its approved and tested, so it's not a draft? |
I found a critical issue and hence moved this PR to draft since it is work in progress. |
…tatus-im/status-mobile into 13439-implement-share-in-shell
…ub.com/status-im/status-mobile into 13439-implement-share-in-shell" This reverts commit caf53a2, reversing changes made to ffa4412.
closing in favour of : #15782 |
fixes : #13439
note : only profile tab, it makes sense to introduce wallet tab once the wallet redesign is done, I am sure a lot of things will change post wallet redesign.
Screen record of how it work :
Screen.Recording.2023-03-31.at.1.40.53.PM.mov
status: ready