search

status-im / status-mobile

a free (libre) open source, mobile OS for Ethereum
https://status.app
Mozilla Public License 2.0
3.9k stars 987 forks source link

[Profile] My profile edit and share screens #8069

Closed flexsurfer closed 5 years ago

flexsurfer commented 5 years ago

Implement new UI for user name

image

image

Implement share icon and screen image

Acceptance criteria:

1) pixel perfect 2) new name UI and icons should be introduced 3) it should be possible to share 4) it should be possible to edit a picture

Figma: https://www.figma.com/file/TNCyHKtR3sx5EL6YznFWUa4O/Profile?node-id=510%3A3116

errorists commented 5 years ago

@flexsurfer if you need help visualising the top bar's scroll behaviour or the animation for sharing, this prototype can help 👉 https://framer.cloud/AvjzY/

flexsurfer commented 5 years ago

nice, thanks!

StatusWrike commented 5 years ago

➤ Julien Eluard commented:

Estimated to 2 days of work

gitcoinbot commented 5 years ago

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

This issue now has a funding of 300.0 DAI (300.0 USD @ $1.0/DAI) attached to it.

gitcoinbot commented 5 years ago

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

Workers have applied to start work.

These users each claimed they can complete the work by 12 months from now. Please review their action plans below:

1) x5engine has applied to start work _(Funders only: approve worker | reject worker)_.

I would love to build this UI screens and complete this before 2 days.

Some guidance over which file and folder to work on.

thanks

Learn more on the Gitcoin Issue Details page.

gitcoinbot commented 5 years ago

@tbenr 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!

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

gitcoinbot commented 5 years ago

@tbenr 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!

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

tbenr commented 5 years ago

@gitcoinbot still finishing #8071 then i'll move on this

gitcoinbot commented 5 years ago

@tbenr 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!

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

gitcoinbot commented 5 years ago

@tbenr 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!

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

gitcoinbot commented 5 years ago

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

@tbenr 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!

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

gitcoinbot commented 5 years ago

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

@tbenr 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!

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

rachelhamlin commented 5 years ago

FYI to anyone checking in: we'll have @bitsikka take over on this issue while @tbenr is away. Looking for @StatusSceptre's help in reassigning it on gitcoin, but the issue is claimed now.

tbenr commented 5 years ago

@rachelhamlin I released the issue in gitcoin. I think @bitsikka can tale over now

gitcoinbot commented 5 years ago

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

Work has been started.

These users each claimed they can complete the work by 10 months, 2 weeks from now. Please review their action plans below:

1) bitsikka has been approved to start work.

I think top bar's scroll behavior can be achieved using onViewableItemsChanged prop other challenges I'm sure I can figure out

Learn more on the Gitcoin Issue Details page.

bitsikka commented 5 years ago

@rachelhamlin I released the issue in gitcoin. I think @bitsikka can tale over now

done!

thanks! @tbenr

bitsikka commented 5 years ago

I had my build system broken for several days and fixed it yesterday.

Starting with this one (as opposed to #8070) because both are profile related issues, and we might need a fundamental change(replace) the use of scrollView to flatList to get onViewableItemsChanged(it fires with list items coming in and going out of viewable window - perfect) to achieve this effect https://framer.cloud/AvjzY/; And that change would affect implementation for #8070.

rachelhamlin commented 5 years ago

Sounds good to me @bitsikka, thanks!

bitsikka commented 5 years ago

I had a slight interruption but I'm back on this.

bitsikka commented 5 years ago

On that note, in the existing implementation of sharing chat key as modal screen, there is show-tribute-to-talk-warning? condition for visibility of said warning; But, new design in figma does not reflect that part. I am assuming that will come later and leaving it out as I port modal to popover design.

Also of note is that Figma design asks for tap on list item to copy, whereas existing functionality, and simple existing-native-support for the functionality, works with lang tap to copy only. Going with simple and lean as it was before.

Except that popover behaves differently than modal in that it can be dismissed much easily - tapping area for dismissal of popover is like 99% of the screen - as opposed to practically opposite in modal.

All that makes it hard/maybe-frustrating for user to get long-press to copy 🤔

rachelhamlin commented 5 years ago

@bitsikka ah yes - that would be correct. Please leave out the TtT bit.

Hmmm cc @errorists so he's aware of tap vs. long press. Was tap on list item to copy a request for a new behavior?

errorists commented 5 years ago

@bitsikka hey, what exactly is stopping us from evoking the Copy menu via a regular tap? it's about discoverability, most of the time you'd try to tap first rather than long-pressing onto something. You mention that 99% of the screen area can be used to dismiss a popover, surely that's an exaggeration, it's specifically tapping outside of it and with these, the popover takes more than half easily :)

bitsikka commented 5 years ago

Thanks @rachelhamlin for making the key concern clear :)

@errorists there is no api for text or text input component to trigger the copy context menu programmatically in react-native that I know of. We'd have to fork the component out of react-native and implement it natively. There are some third party components that could be employed but maybe overkill.

I think what can be done is that we show a tooltip(the kind used to show error message elsewhere in text-input component) instead, as a visual cue, to inform user that the key has been copied to clipboard.

Re: 99% area for dismissing popover - I was simply copying the example of sharing wallet-address in current nightly/develop where this is true(please try it out) - in which case this would be a bug that needs fixing for wallet-address share(or maybe/probably popover implementation in general).

bitsikka commented 5 years ago

I think what can be done is that we show a tooltip(the kind used to show error message elsewhere in text-input component) instead as a visual cue to inform user that the key has been copied to clipboard.

I mean for this - the functionality itself "copy on tap" is not the problem - hardcoded behavior of copy context-menu upon long-press is the problem to replicate "on single tap"

errorists commented 5 years ago

Ok, let's do it via long press. I've seen wallets made with React Native (Rainbow) where regular press -> context menu works, so I never thought this might require going through so many hoops. And good lord, how did that wallet popover get through QA 😬 it definitely shouldn't be the case, tapping inside should be ignored. Thanks for mentioning that!

bitsikka commented 5 years ago

I've seen wallets made with React Native (Rainbow) where regular press -> context menu works, so I never thought this might require going through so many hoops.

Interesting! I will check out how rainbow does it and try to replicate it - having to long press actually bothered me very much in the first place to bring it up

bitsikka commented 5 years ago

@errorists turns out Rainbow which is iOS only app, uses react-native-tooltip - a third party component which is also iOS only :(

leaning towards using react-native-tooltip for iOS, with react-native built-in long-press version for Android for now, but not sure.

bitsikka commented 5 years ago

Now I'm thinking, it might be good idea and less over-kill to replicate platform specific fake copy context-menu/tool-tip with a custom non-native component. I don't think something "so simple" has to be natively implemented.

errorists commented 5 years ago

bitsikka

ok how about something simple like showing a tooltip for a second after tap? for animated values are opacity 0 to 1 to 0 and translateY 10 to 0 to 10, duration 140ms ease.

bitsikka commented 5 years ago

ok how about something simple like showing a tooltip for a second after tap? for animated values are opacity 0 to 1 to 0 and translateY 10 to 0 to 10, duration 140ms ease.

@errorists Sounds good! - that idea did cross my mind

Then, I also found https://anchorchat.github.io/anchor-ui-native/#/context-menu which is implemented as a regular(non-native) component just like I suspected there would be. Existing tooltip, this component, and the idea you just suggested are/can-be implemented simply without native coding.

This idea is actually less step, clean, and simple; therefore better ux - so going with it!

bitsikka commented 5 years ago

I found a bug in ENS registration where it checks for already owned name. I needed ENS name to test the name UI part and tried to get already owned name registered in the app - but it failed to recognize I am the owner of the name even though I did own the name.

Turns out comparision of resolved name to wallet address in app failed because of difference in eip55 checksum. I got it to recognize my name as owned, by adding eip55/address->checksum on top of ethereum/normalized-address during comparision.

Also of note is that [:app-db :multiaccounts :address] stored is not eip55 checksummed address rather than the address resolved by the contract.

This is probably already known, or, there are improvements to ENS registration process coming, in which it will be resolved. Otherwise, as it exists, why would I have to do the registration process for already owned name and pay(or seem to pay) for gas? 🤔 among other problems.

errorists commented 5 years ago

Cool! just fyi, we will need a robust tooltip component in the future once we'll finally start working on stuff like message reactions

reactions

bitsikka commented 5 years ago

Cool! just fyi, we will need a robust tooltip component in the future once we'll finally start working on stuff like message reactions

Cool! I remember seeing those 😄

in which case a native implementation would probably be the way to go. But I'll keep that in mind. Tooltip already exists for error messages - will take a look at its implementation too. I think better to call this context-menu.

rachelhamlin commented 5 years ago

Oh dear. Glad you caught that ENS bug and the source of it too @bitsikka. I'll capture it in an issue--it didn't come up during QA of ENS name registration & verification.

rachelhamlin commented 5 years ago

Hey @bitsikka! Doing some bounty management - you're on this one + #8070. Should we expect to review anything this week?

bitsikka commented 5 years ago

@rachelhamlin I'm close to getting this one done to the point of being reviewed.

Planning on moving on to #8070 soon.

rachelhamlin commented 5 years ago

Fab. Thanks for the update! Keep up the great work.

gitcoinbot commented 5 years ago

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

Work for 300.0 DAI (300.0 USD @ $1.0/DAI) has been submitted by:

  1. @bitsikka

@StatusSceptre please take a look at the submitted work:

gitcoinbot commented 5 years ago

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

The funding of 300.0 DAI (300.0 USD @ $1.0/DAI) attached to this issue has been approved & issued to @bitsikka.