bitshares / bitshares-ui

Fully featured Graphical User Interface / Reference Wallet for the BitShares Blockchain
https://wallet.bitshares.org
MIT License
518 stars 569 forks source link

[96][charyury][3][gibbsfromncis] Update Sign Up UX #1832

Closed gibbsfromncis closed 5 years ago

gibbsfromncis commented 6 years ago

After this issue #1808 I suggested that misunderstanding of wallet types is on Sign Up page. When user sees Sign Up for the first time he can't make conscious choice between wallet types because he has no idea that we support 2 different wallet types. When user opens Sign Up the sees only that he can create account using advanced form.

Also my friend had some another problems on sign up like no clear validation errors etc.

The result of this issue must be a new, clear UX of Sign Up.

froooze commented 6 years ago

I like the layout from Openledger, where we have two columns with a list to compare each other.

Ask Openledger for a back port to the Bitshares client?

startailcoon commented 6 years ago

The two types are very similar for users when they sign up, as well as sign in, nowdays indeed. Nothing really speaks about the difference.

OoenLedger has made it much more clear and could be a good place for inspiration on this.

gibbsfromncis commented 6 years ago

@froooze,@startailcoon, yep, that's what I mean exactly.

@startailcoon I'll prepare the list of requirements & try to estimate it.

sschiessl-bcp commented 6 years ago

Ask Openledger for a back port to the Bitshares client?

I asked what there standpoint on that is

gibbsfromncis commented 6 years ago

@startailcoon @sschiessl-bcp

I suppose we can keep Sign Up components we already have but provide additional step when user should decide about account type.

I don't think we should wait for somebody's response or ask for a back port.

Estimate for UX is 3h and 3h for development.

In result it will be additional page which user will see when press "Create account" (step before registration forms). On this page user should get clear info about wallet types and decide which one he wants.

happyconcepts commented 6 years ago

This is pointless when there are more serious ui deficiencies that should be the focus of this worker.

This worker was to improve the UI and not be a Slush fund for a small group to enrich themselves at the expense of the other participants in the chain.

sschiessl-bcp commented 6 years ago

This is pointless when there are more serious ui deficiencies that should be the focus of this worker.

Please open issues for those if not already existent.

This worker was to improve the UI and not be a Slush fund for a small group to enrich themselves at the expense of the other participants in the chain.

Can you give more detail in your accusation here?

happyconcepts commented 6 years ago

No. It would just get me banned here as in telegram, for telling the truth to those who want to hide it.

iIRC It's simple math to add up who has been enriched on the UI worker.

froooze commented 6 years ago

The registration process is critical and not very helpful atm.

@gibbsfromncis: We don't need another page, we just need to redesign the actual one like open ledger did it.

@happyconcepts: You should open some tickets and post on other platforms things you don't like.

sschiessl-bcp commented 6 years ago

OpenLedger signaled that they would be willing to backport there approach. Do we like the UX?

abitmore commented 6 years ago

@happyconcepts please stay in topic. I know you're a talent and have a desire to improve things and have done some, but some other things you've done have led to the opposite. BTW I guess admins can ban you here as well if you're identified as "not-welcome".

abitmore commented 6 years ago

@sschiessl-bcp I'd expect that OpenLedger will estimate much more hours. See discussion here https://github.com/bitshares/bitshares-core/issues/1210#issuecomment-418860702.

gibbsfromncis commented 6 years ago

@sschiessl-bcp Yep.

@sschiessl-bcp if @abitmore right and backport from OpenLedger will take more time than requested hours from me I see no sense to ask them about their help but I definitely like their solution

froooze commented 6 years ago

With backport I thought of copying the code from Openledger to the bitshares-ui? Where is the extra work?

gibbsfromncis commented 6 years ago

@froooze it may be not so easy to just copy-paste code. Also their bureaucracy and management take time too. Lets wait for OpenLedger team estimate first

froooze commented 6 years ago

Why do we need OpenLedger, when we are allowed to backport it?

sschiessl-bcp commented 6 years ago

Why do we need OpenLedger, when we are allowed to backport it?

OpenLedger is closed-source afaik

gibbsfromncis commented 6 years ago

@froooze they can do it more cheap than I requested.

@sschiessl-bcp btw how long should we wait for their response? Can we make some deadline if they don't response I can start to work by myself?

startailcoon commented 6 years ago

As I find some things on OpenLedger pretty nice, I still think we shouldn't backport their style directly.

What I think we need

Today our UX is confusing at best when you sign up, with options to type in a form, or restore, or use an advanced from. The advanced form links to another set of form, or you can store from backup or local brain key. This is to many steps without any easy explanations.

What OpenLedger has solved is to put a simple side-by-side comparison, and this is what we should work towards. But I don't agree that we should use their UX!

I think we should allow a set of UX/UI hours to making a good implementation of our own.

I support @gibbsfromncis proposal of estimated hours for this task.

froooze commented 6 years ago

@startailcoon: The OpenLedger is a good base, but needs further tweak and we should discuss some improvements.

charyury commented 6 years ago

Hello friends! I will provide detailed information about the effort we spent for sign up, login and logout screens, waiting for details from the developers. Anyhow, this will be definitely far from 6 hrs mentioned above, so if we somehow block the progress, please go ahead. Still we would love to be 100% transparent and make sure we compare apples to apples when talking about estimation.

froooze commented 5 years ago

Before working on the signup window and integrate it, a draft is needed for discussion.

charyury commented 5 years ago

Hello friends! Sorry for the delay, below are efforts we spent for our implementation. As expected, those are much bigger than estimated by your team, still I am willing to share, this might help to understand if there is something wrong with our process, or maybe your team is underestimating, or maybe we are comparing completely different solutions.

Here we go:

Registration

Selection screen Page 1: Components creation, components structure - 4h Registration selection wallet panel markup - 4h Registration selection account panel markup - 4h New layout styles - 4h Code review - 2h Bug fixes, code review comments fixes - 2h

Registration wallet page 2.1 screen: Registration form - 4h Validations - 2h New layout stylings - 8h Code review, fixes - 2h

Registration wallet page 2.2 screen: Confirmation form - 4h New layout stylings - 4h

Registration account page 3.1 screen: Registration form - 4h Validations - 2h New layout stylings - 8h Code review, fixes - 2h

Registration account page 3.2 screen Confirmation form - 4h New layout stylings - 4h

Login:

Plate with headers: 12h Account login: 12h Wallet login: 24h New tooltips: 6h Mobile version: 8h Refactoring, small fixes: 10h

Total (registration+login):

FE - 140h QA - 20h UX - 20h PM- 12h

Logout

FE - 20h QA - 4h PM - 2h UX - 2h

As we did this for our solution, we are ready to share the code at 0.5 of the costs of the initial implementation.
Any feedback is much welcomed and appreciated!

wmbutler commented 5 years ago

@charyury Thanks for this. I received your email and we are excited to begin collaborating with Openledger and all of the gateway providers who have created added value for their own gateways and are interested in contributing back to core.

I reviewed your signup procedure and only have a few comments:

Which Sprint should I place this issue into in order to manage the expected timeline for delivery?

froooze commented 5 years ago

I thought @startailcoon just wanted to replace the Selection screen Page 1 and add a row of extra information?

gibbsfromncis commented 5 years ago

I've unassigned myself from this issue because now OpenLedger will be responsible for this one and will work on it by themself

froooze commented 5 years ago

@charyury

@wmbutler

charyury commented 5 years ago

@wmbutler

I reviewed your signup procedure and only have a few comments:

  • 7 character password minimums on "Account Model", (Cloud Wallet) should be brought up to 16 to ensure brute force attacks on username/password are unlikely.
  • We should keep the Local Wallet / Cloud Wallet terminology for the reference wallet so we don't have to re-educate.
  • Since we are going to ANT, let's use ANT components. I don't think this materially affects your implementation.
  • Ensure fonts are consistent with reference wallet

We will implement all these requirements. This will not add any additional cost.

Which Sprint should I place this issue into in order to manage the expected timeline for delivery?

We are ready to start early next week and deliver by the end of the week.

@froooze, thanks a lot for your comment, we will update accordingly.

wmbutler commented 5 years ago

@charyury since this issue is just for sign up, let's only include estimates for that portion. We can make a separate issue for the sign out.

FE - 140h QA - 20h UX - 20h PM- 12h

Total (@50%) = 96 hours

Since we are effectively helping compensate OL for their dev work, I would ask you to agree to work with us diligently as we fine-tune various aspects of the design related to the ANT framework. @startailcoon I will allocate hours to both you and to @gibbsfromncis for your time as it relates to building this with ANT in mind. It's important that we do not build this and then have to build it a second time to conform.

This item is in the 181017 sprint.

sschiessl-bcp commented 5 years ago

One thing came to mind to prepare for the future: Beet will be integrated sooner or later, which means a third way of signing up. The new UX should be prepared for that.

The requirements for Beet are quite simply: On sign up you only provide a username without any credentials, as every signature request will be forwarded to Beet

charyury commented 5 years ago

@wmbutler , got it! I'm transferring this to my dev team now, will introduce them. @sschiessl-bcp, we are going to introduce Beet integration in our UI as well. Let me check with my team if we have any questions for now.

clockworkgr commented 5 years ago

@sschiessl-bcp & @wmbutler & @charyury

In fact , as far as Beet is concerned, the way I envision it is you simply get a "Use with Beet" button.

On click, the UI authenticates itself to beet and receives 1 or more account ids. From then on it works exactly the same as if having opened a wallet, only signing is done over on beet rather than in the UI

gibbsfromncis commented 5 years ago

@wmbutler @charyury can you please clarify what exactly we are going to do? I do not understand my and OL duties in case of this task.

abitmore commented 5 years ago

(Cloud Wallet) should be brought up to 16 to ensure brute force attacks on username/password are unlikely.

Please be aware that a simple 16-character password is NOT secure. We don't have any 2FA method so far, nor any password recovery method. We did decide to not allow users to choose their own passwords for a reason. Earlier discussion is here https://github.com/bitshares/bitshares-ui/issues/177.

sschiessl-bcp commented 5 years ago

I just noticed that the hours assigned are set to include login functionality. The OpenLedger login method is fundamentally different.

Reference Login is available everywhere via a Popup, which allows to login with local wallet or cloud login non-intrusively while staying on the currently open tab (after login nothing is reloaded, simply the Popup disappears). image The reference wallet does not hide any features when not logged in / unlocked. For example, you can browse the exchange, enter what you want to Buy/Sell and login when needed.

OpenLedger If there is no account recognized, Login is available via a seperate route. In that route you can decide to login via local wallet or cloud login. image After this initial login, there is a popup that then either asks for local wallet password, or cloud login password, but it does not allow to switch, see below for the exchange image Furthermore, if no initial login is found, the controls of the exchange and other features are simple locked / hidden, see below for the exchange image

Suggestion If Login is to be included with this issue, I suggest that the Login route from OpenLedger is taken for initial login, and the PopUp from the Reference Wallet is also kept (which means it allows switching). The PopUp could also benefit from a facelift to match the new UX. The basic logged-in / logged-out / locked / unlocked behavior in terms of hiding and/or locking features in the Reference UI remains as is.

What are your thoughts on this?

wmbutler commented 5 years ago

(Cloud Wallet) should be brought up to 16 to ensure brute force attacks on username/password are unlikely.

Please be aware that a simple 16-character password is NOT secure. We don't have any 2FA method so far, nor any password recovery method. We did decide to not allow users to choose their own passwords for a reason. Earlier discussion is here #177.

@charyury please note this as well. I think for the cloud wallet, it's ok for you to use our existing method of providing a very long complex password. Just use the existing method to prefill that for the user while asking them to save and confirm it. We've been through quite a bit of iterations on that and what we have now seems to work well.

wmbutler commented 5 years ago

@wmbutler @charyury can you please clarify what exactly we are going to do? I do not understand my and OL duties in case of this task.

@gibbsfromncis I'd like for you to assist with the ANTification of this so we don't have duplication of effort. I've allocated some time for you to do this. in the issue.

wmbutler commented 5 years ago

@charyury please be sure to submit UX for the various scenarios, particularly for those where more complex interactions occur (a user placing an order while not logged in which results in a modal to gain authentication).

OpenLedgerApp commented 5 years ago

@clockworkgr, @sschiessl-bcp, we are planning to introduce Beet integration. It can be delivered when it is ready. @wmbutler, the general flow can be the same - we can share our estimate and if it is confirmed - we will provide our solution.

@abitmore, @wmbutler we can implement your scenario for the cloud wallet with the providing a long complex password if it is required, however, it was not in our estimate. @wmbutler, should we provide our estimate for this feature? It will be a minor effort and maybe we can even provide it free of charge after sync with technical team, however, IHO we don't see any value to generate such complex password, first, it is stored without any encoding locally, second, as was rightly mentioned in the #177 - it is unreal to remember such password for the user, third, in our approach we have strict rules for the password (uppercase, special symbol, digit, etc.) - as the result, it is very complex to hack it.

@sschiessl-bcp, thanks for your notes about UX and the detailed description! Our suggestion is below: 1) The initial login will be the OpendLedger route. We are on the same page here.

2) When the user already logged in, for locking/unlocking functionality we are thinking our UX behavior is more suitable for the best practices. If you have Cloud wallet - you should enter only the Cloud credentials. If you have the Local wallet, you should enter only the Local password. If a user wants to change an account, he can use our feature Log Out that we are going to provide in the bounds of the separate task. @wmbutler, could you please create a separate task for the Log Out functionality? In the current Reference UI - it is not obvious how a user can change the account. We think it is better to introduce "Switch account" feature in the nearest future. @wmbutler, if you agree, we can provide our timelines and detailed estimate for this. For the current task, let's keep our approach with Log Out button. Do you agree?

3) @sschiessl-bcp, we really like an idea is to allow users to have all functionality without login. However, it was not in our estimate and the current version in the Reference UI has bugs. For example, see below. When an anonymous user browses the exchange, the initial balances look incorrect: photo_2018-09-28_16-57-40 When he clicks buy/sell, the system doesn't check his balance after login, there is a transaction confirmation window: photo_2018-09-28_16-57-41 After the clicking "Confirm", system checks the actual balance and gives the error: photo_2018-09-28_16-57-41 2 And, Et... voilà, only now we can see the correct balance for the logged user. You can see all previous steps are surplus: photo_2018-09-28_16-57-41 3 We think, the anonymous functionality should be developed properly in the bounds of a separate task. Here, we prefer Binance UX: image - the user should sign up or log in before any transaction. @wmbutler, please let us know if you interested in such feature. Our tech guys can develop it accordingly. Thanks!

froooze commented 5 years ago

@charyury: Activation of check boxes at the end of the registration process is not possible, without resizing the text on an mobile phone, but than the text is not readable anymore. A smaller max-width with an larger clickable area could do the trick.

wmbutler commented 5 years ago

@abitmore, @wmbutler we can implement your scenario for the cloud wallet with the providing a long complex password if it is required, however, it was not in our estimate. @wmbutler, should we provide our estimate for this feature?

@charyury I would hope you would not charge us for inserting the password into the field since it's (a) already part of our code (b) we are actively subsidizing work that you have completed already. It was my position that we shouldn't sweat the small details since we have entered into this partnership in good faith.

it is unreal to remember such password for the user, third, in our approach we have strict rules for the password (uppercase, special symbol, digit, etc.) - as the result, it is very complex to hack it.

I'd rather not get into a debate about this here, but it's very easy to brute force attack an 8 character password if the username is known. It would take about 9 months to go through all combinations. I am speaking only from the cloud wallet perspective here.

could you please create a separate task for the Log Out functionality?

Let's get this issue completed before we add the next. I think we need to make sure that we all meet expectations before adding more issues if that is acceptable.

@sschiessl-bcp, we really like an idea is to allow users to have all functionality without login. However, it was not in our estimate and the current version in the Reference UI has bugs. For example, see below.

@sschiessl-bcp I would like to see your response to this. It is an interesting problem. Seems to me that if a user is not logged in (cloud wallet), we really have no way of knowing which account they are trying to use and therefore don't know their balances. On the other hand, we should be able to perform other operations like proposing transactions, etc.

wmbutler commented 5 years ago

@wmbutler, please let us know if you interested in such feature. Our tech guys can develop it accordingly.

This will need to wait for the exchange redesign UX. Once it's in production, I'd like to collaborate with you guys on this.

OpenLedgerApp commented 5 years ago

@charyury I would hope you would not charge us for inserting the password into the field since it's (a) already part of our code (b) we are actively subsidizing work that you have completed already. It was my position that we shouldn't sweat the small details since we have entered into this partnership in good faith.

@wmbutler, our partnership is very important to us and our primary goal is to bring value to all community. We will do this task for free.

Let's get this issue completed before we add the next.

@wmbutler, it sounds reasonable! Let's finalize and complete this task first.

This will need to wait for the exchange redesign UX. Once it's in production, I'd like to collaborate with >you guys on this. @wmbutler, We are ready to embed our sign up/login to your current UX flow in the bounds of this task.

  1. The initial login will be the OpendLedger route.
  2. Lock/Unlock functionality will be inlined for the currently logged-in user. If a user wants to change the account, Log Out function will be used.
  3. For anonymous mode, we can save your current UX behavior. We will only put sign up/login screens accordingly.

@wmbutler, if we are on the same page now, let's move forward with this task and keep in mind further improvements: Log Out, Switch User, Binance anonymous behavior, Beet integration.

wmbutler commented 5 years ago

Yep. I just want to restate that it's my expectation that we will have time to make tweaks as we get closer to completion even if they are not explicitly stated here. I think everyone involved can appreciate that we are committing enough funds to allow for some negotiation if there are unanticipated details that need to be worked through.

OpenLedgerApp commented 5 years ago

Okay. We have the same vision. We will sync with @gibbsfromncis and we will start our activities here.

sschiessl-bcp commented 5 years ago

The old local wallet create screen has a password strength indicator, can that be copied over?

image

sschiessl-bcp commented 5 years ago

If the IP restriction from the backend fires, the button should say "Try again"

image

OpenLedgerApp commented 5 years ago

@sschiessl-bcp Thanks for your comments! We will check and save these features.

gibbsfromncis commented 5 years ago

I updated hours because of additional time I spent for testing & reviewing styles and UX

sschiessl-bcp commented 5 years ago

Is this issue considered to be done from your side with the login PR from yesterday? Then I will review the general flow one more time.