Skip to content
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

[Multi-Account] Design touch-ups for account explorer + wallet UI #8447

Closed
rachelhamlin opened this issue Jun 20, 2019 · 37 comments
Closed

[Multi-Account] Design touch-ups for account explorer + wallet UI #8447

rachelhamlin opened this issue Jun 20, 2019 · 37 comments
Assignees

Comments

@rachelhamlin
Copy link
Contributor

rachelhamlin commented Jun 20, 2019

Description

Type: Feature

Summary: There are some to-dos remaining from PR 8425:

  • The correct on scroll behaviour of the navigation bar is not implemented yet N/A now
  • The horizontal scroll container is clipping its content on the left side N/A now
  • Pinterest style transition - SEPARATE ISSUE: Pinterest style transitions for account explorer  #8896
  • The single card should have the right padding at 16 - DONE
  • Monospace (supported after onboarding PR merge) - DONE
  • Update manage assets icon
  • Copy updates: Add a watch account > Add a watch address - DONE
@rachelhamlin rachelhamlin changed the title [Wallet] Design touch-ups for account explorer + wallet UI [Multi-Account] Design touch-ups for account explorer + wallet UI Jun 20, 2019
@rachelhamlin rachelhamlin added v1 and removed v1 audit labels Jul 23, 2019
@hesterbruikman
Copy link
Contributor

@andmironov for the above items: can you please check if there's more detailed specification to be added?

@andmironov
Copy link

andmironov commented Jul 26, 2019

I'd be happy to provide the details if needed, but for now, every item of the list above seems straightforward to me.

The only thing is

Pinterest style transition

I think it might better be a separate issue since it's not super important and it (at least to me) seems to require a lot of implementation effort? Correct me if I am wrong.

anyway, @errorists can you share the link to the prototype you've made?

@errorists
Copy link
Contributor

andrey
andrey2

@rachelhamlin
Copy link
Contributor Author

After the discussion on this post and in Status, I'm going to remove the Pinterest style transition from this issue and make it its own. Not sure it is possible with React Native.

@errorists
Copy link
Contributor

@bitsikka hey, so a couple pointers, for the scroll transition you can recycle the toolbar you did for the profile. I might have used a different behaviour when prototyping this (the text interpolates its size based on scroll position) but that's not really required, it can use the same go under the toolbar, appear smaller above it like you did for profile, consistency and all :)
also I just got from vacation so if you need help/feedback with any of your awesome prior work, lmk.

as for the pinteresty transition, I'm 100% sure it's possible, it's a custom React Navigation transition

@bitsikka
Copy link
Contributor

bitsikka commented Sep 4, 2019

@bitsikka hey, so a couple pointers, for the scroll transition you can recycle the toolbar you did for the profile. I might have used a different behaviour when prototyping this (the text interpolates its size based on scroll position) but that's not really required, it can use the same go under the toolbar, appear smaller above it like you did for profile, consistency and all :)
also I just got from vacation so if you need help/feedback with any of your awesome prior work, lmk.

wonderful! :) appreciate your thoughtfulness. Although I've been studying and I am positive, Text scaling transform/interpolation is possible, it will be wise to maintain consistency.

as for the pinteresty transition, I'm 100% sure it's possible, it's a custom React Navigation transition

Thank you for the awesome resource. I will master it and apply it here. I knew you wouldn't prototype something not possible :). After all, engineering is also art/design, and you are yourself excellent at it. I figure, were it not for the implementation language, you would've long jumped in and fixed it :) Will do my best to do justice to spec, meanwhile being mindfully reasonable about performance, as well as time.

I also had a hunch that there is untapped creative potential in React Navigation that will be useful here(and other places). On to study.

Let me just take this opportunity to say, what I've been meaning to say, that, your and design/ux team work is simply top notch, to the extent of every sub-pixel and their movement being accounted for, with concern for overall UX; And I am glad to have the opportunity to work on implementing them. The magic of "minor" details like alignment/typography/interaction-feedback when it all comes together, is simply very pleasing, satisfying and amazing UX - in other words subconsciously mind-blowing! :)

I will ping you in status or here when I have questions.

@gitcoinbot
Copy link

@bitsikka Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@bitsikka
Copy link
Contributor

bitsikka commented Sep 8, 2019

@bitsikka Hello from Gitcoin Core - are you still working on this issue?
@gitcoinbot I would be if not for a hiccup. Dev build is not launching in iOS emulator. Too slow in Android Avd. Seems to be a known problem, waiting for it to be fixed.

@bitsikka
Copy link
Contributor

bitsikka commented Sep 9, 2019

Help!

It seems that I am the only one having this problem.

  • dev build for iOS builds successfully, but then launches and gets stuck of status logo screen.
  • I am on MacOS Mojave Version 10.14.6
  • XCode is version 10.3
  • Simulator is 12.3
  • I am rebased to latest develop
  • Checked out remove-real-final branch - same thing :(

Have not been able to move forward an inch since Thursday

@flexsurfer
Copy link
Member

@bitsikka there should be an error in the logs, could you check it in safari dev console?

@bitsikka
Copy link
Contributor

bitsikka commented Sep 9, 2019

@bitsikka there should be an error in the logs, could you check it in safari dev console?
@flexsurfer
Not familiar with that, never used it. Please tell me how.
I went to re-frisk at http://localhost:4567/ - nothing there just nil

@flexsurfer
Copy link
Member

@bitsikka https://facebook.github.io/react-native/docs/debugging#safari-developer-tools
You can use Safari to debug the iOS version of your app without having to enable "Debug JS Remotely".

Enable Develop menu in Safari: Preferences → Advanced → Select "Show Develop menu in menu bar"
Select your app's JSContext: Develop → Simulator → JSContext
Safari's Web Inspector should open which has a Console and a Debugger

@bitsikka
Copy link
Contributor

bitsikka commented Sep 9, 2019

oh.. ddg'd and found that doc :) doing it now.. hold on

@bitsikka
Copy link
Contributor

bitsikka commented Sep 9, 2019

got this

[Warning] Possible Unhandled Promise Rejection (id: 0): (index.bundle, line 903)
TypeError: undefined is not an object (evaluating 'status_im.utils.handlers.register_handler_fx')
eval code
eval@[native code]
asyncImportScripts$@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:200728:21
tryCatch@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:26567:23
invoke@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:26742:32
tryCatch@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:26567:23
invoke@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:26643:30
http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:26653:21
tryCallOne@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:3725:16
http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:3826:27
_callTimer@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:28405:17
_callImmediatesPass@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:28441:19
callImmediates@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:28660:33
callImmediates@[native code]
__callImmediates@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:3155:35
http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:2976:34
__guard@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:3138:15
flushedQueue@http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false:2975:21
flushedQueue@[native code]
invokeCallbackAndReturnFlushedQueue@[native code]

@bitsikka
Copy link
Contributor

bitsikka commented Sep 9, 2019

maybe this will help too

Screen Shot 35

@flexsurfer
Copy link
Member

have you tried to clean project and rebuild again?

@flexsurfer
Copy link
Member

@bitsikka i have the same error in develop, on it, hold on

@bitsikka
Copy link
Contributor

bitsikka commented Sep 9, 2019

have you tried to clean project and rebuild again?

I have, multiple times. Did make nix-clean + make clean.

@vitvly
Copy link
Contributor

vitvly commented Sep 9, 2019

Just to confirm I'm facing the same issue after rebase with latest develop.

@vitvly
Copy link
Contributor

vitvly commented Sep 9, 2019

@bitsikka you can also install https://www.npmjs.com/package/react-native-log-ios and just run react-native-log-ios StatusIm and it'll give you the logs in the terminal.

@bitsikka
Copy link
Contributor

bitsikka commented Sep 9, 2019

@bitsikka you can also install https://www.npmjs.com/package/react-native-log-ios and just run react-native-log-ios StatusIm and it'll give you the logs in the terminal.

Good to know. Thanks! @siphiuel
Very handy

@bitsikka
Copy link
Contributor

bitsikka commented Sep 9, 2019

I'm good to go 😄
#8953 fixed it
Thanks! @flexsurfer 🚀

@flexsurfer
Copy link
Member

@bitsikka please rebase , should work

@bitsikka
Copy link
Contributor

Best to wait for #8776 to be merged, - it has improved animation/component-structure for large-header component useful for main part of this issue.

Meanwhile,

@gitcoinbot
Copy link

@bitsikka Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@bitsikka
Copy link
Contributor

@gitcoinbot I am working indirectly on this. large-header is as much a part of this, as it is part of #8981, which I'm working on. It is also better to finish the general case of it, in #8981 before this. Plus waiting on soon to be merged PR that will be useful for both.

@rachelhamlin
Copy link
Contributor Author

Rock on @bitsikka. If you're planning to finalize #8981 first, I'll consider that to be in progress without making it Gitcoin official & will pay for you everything in this one via tip. :)

@bitsikka
Copy link
Contributor

Rock on @bitsikka. If you're planning to finalize #8981 first, I'll consider that to be in progress without making it Gitcoin official & will pay for you everything in this one via tip. :)

works for me 👍
I just had a major distraction out of the way, so moving on with everything peacefully now

@gitcoinbot
Copy link

@bitsikka Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@bitsikka
Copy link
Contributor

@gitcoinbot
I'm finding fair amount of intertwines on the PRs I'm working on, easy to mess up if not careful.
Slowly and surely though, I'm working on them, ensuring as much as possible smooth transition to the new.

@gitcoinbot
Copy link

@bitsikka Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


@bitsikka due to inactivity, we have escalated this issue to Gitcoin's moderation team. Let us know if you believe this has been done in error!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

hesterbruikman added a commit that referenced this issue Oct 2, 2019
Fixes copy update in #8447

In the UI we want to refer to a watch only address as `address` instead of `account`. Interactions with an address are limited compared to an account in wallet. e.g. there is no option to send assets from this account as Status wallet does not have the key to sign for transactions.
flexsurfer pushed a commit that referenced this issue Oct 7, 2019
Fixes copy update in #8447

In the UI we want to refer to a watch only address as `address` instead of `account`. Interactions with an address are limited compared to an account in wallet. e.g. there is no option to send assets from this account as Status wallet does not have the key to sign for transactions.

Signed-off-by: Andrey Shovkoplyas <motor4ik@gmail.com>
@hesterbruikman
Copy link
Contributor

@bitsikka catching up here. Do I understand correctly that this issue is on pause while you are working on Topbar #8981 and were waiting for #8776 to be merged?

Do you have an ETA on when you might get around to this issue? If it's plus 2 weeks we might see if someone else can pick this one up, but obviously not if you're 90% there.

@bitsikka
Copy link
Contributor

@bitsikka catching up here. Do I understand correctly that this issue is on pause while you are working on Topbar #8981 and were waiting for #8776 to be merged?

Do you have an ETA on when you might get around to this issue? If it's plus 2 weeks we might see if someone else can pick this one up, but obviously not if you're 90% there.

@hesterbruikman yes we are in total sync

  • This one is on pause while I work on Topbar
  • Positive that Topbar will take < 2 weeks (only ~30% there so far)
  • I am 50% present till 9 more days, and more-or-less 100% present till New Year

@andmironov
Copy link

andmironov commented Jan 16, 2020

hey everyone! seems like this issue has grown too big and is on a long pause and has changed it's initial subject over time :-) . Does anyone think it would be helpful opening a new topbar-specific issue (and maybe another one about the account card transition from @errorists 's prototype) and closing this one?

These two un-crossed tasks from the initial issue description are no more relevant

  • The horizontal scroll container is clipping its content on the left side
  • Update manage assets icon

@rachelhamlin
Copy link
Contributor Author

rachelhamlin commented Jan 17, 2020

Yup, sounds good. I think this one covers account card transition @andmironov? #8896

And as for the topbar, we have this issue, which is still open—although there is a first iteration complete.

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Cancelled


The funding of 60.0 SAI (60.0 USD @ $1.0/SAI) attached to this issue has been cancelled by the bounty submitter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants