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

Beos dialog #2935

Open
wants to merge 74 commits into
base: develop
Choose a base branch
from
Open

Beos dialog #2935

wants to merge 74 commits into from

Conversation

szechem
Copy link
Contributor

@szechem szechem commented Jul 12, 2019

General

Implementation of a new EOS Bitshares section in Deposit / Withdrawal. It contains a description of the token distribution and functionality of the opening dialog, in which it is possible to create BEOS accounts and transfer tokens between the Bitshares and BEOS networks.

Code Preparation

Please review all your changes one last time before committing

  • Check for unused code
  • No unrelated changes are included
  • None of the changed files are reformatting only
  • Code is self explanatory or documented
  • All written text is properly translated (english language)

Testing

The branch has been tested on the following browsers (desktop and mobile view)

  • Chrome
  • Opera
  • Firefox
  • Safari

User interface changes

Delete this section if there weren't any UI changes. Please make sure you tested your changes in all themes

  • Dark
  • Light
  • Midnight

Please provide screenshots/licecap of your changes below

@szechem
Copy link
Contributor Author

szechem commented Jul 12, 2019

beos_a
beos_b

@abitmore
Copy link
Member

My own opinion: the "BitShares EOS" text is confusing, better replace all with "BEOS".

@sschiessl-bcp
Copy link
Contributor

sschiessl-bcp commented Jul 15, 2019

This essentially opens the BTS withdraw functionality and include external custodians to be listed in a similar way like gateways are.

This opens up couple points for discussion:

  1. do we want to include those in the wallet? I feel like yes, since people would have a better UX if they can select to deposit e.g. into binance. Question remains where to get the needed information.
  2. if 1 is yes, which external custodians should we include? opt-in like gateways? community selected?
  3. the legacy Deposit/Withdraw is deprecated and will be removed soon (I just removed the legacy Send last week), adding this now seems not feasible. This needs proper UX and be included in the Deposit/Withdraw modal.

It's a significant strategic change, thus it needs an issue for discussion imo. Any and all new gateways that want to get added and use existing UX need to get approved, and this has new UX on top of it. Please open an issue and we discuss it there how to proceed.

Are you affiliated with BEOS @szechem ? What are your thoughts?

@dnotestein
Copy link
Contributor

szechem is a contractor for BLCA (association for BitShares EOS). These code changes have been tested for a number of months now in a mildly customized version of the reference web wallet hosted by BLCA. IMO, it's a very small feature: it formats a memo when sending bitshares funds. It has clear text that describes risks involved in this operation. I've been requested to create a new issue to discuss inclusion of this PR (or some undesigned idea to replace it, which I think shouldn't delay this PR), so I've made one here: #2950

@sschiessl-bcp
Copy link
Contributor

Please continue discussion in #2950 (comment)

@sschiessl-bcp
Copy link
Contributor

stale, if not please reopen

@dnotestein
Copy link
Contributor

AFAIK, it's not particularly "stale". Currently the changes are being run on one hosting, because these changes weren't included into the reference wallet. Ideally, they would still be included in the reference wallet.

@dnotestein dnotestein reopened this May 1, 2020
@dnotestein
Copy link
Contributor

Apparently there's conflicts now. I'm not opposed to fixing those, if the changes will be included. But if they are just going to be rejected again, I don't want to waste my time.

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

Successfully merging this pull request may close these issues.

5 participants