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

#2696. use fee selection feature on the transaction confirmation #3001

Conversation

OpenLedgerApp
Copy link

@OpenLedgerApp OpenLedgerApp commented Jul 31, 2019

General

Closes #2969

Added this feature for all used transaction operations.

General

Please make sure the following is done:

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
for example create asset operation
image

@sschiessl-bcp
Copy link
Contributor

What you did here is the very first step of this task: #2975

@sschiessl-bcp
Copy link
Contributor

sschiessl-bcp commented Jul 31, 2019

This is the scope of the task
#2969 (comment)

@abitmore
Copy link
Member

abitmore commented Jul 31, 2019

@OpenLedgerApp please take a screenshot from Barter transaction confirmation page (https://develop.bitshares.org/#/barter)? Thanks. I need the JSON of the whole transaction but not only JSON of individual operations (for issue #2657).

@OpenLedgerApp
Copy link
Author

@sschiessl-bcp fixed.
now it looks like:
image
SetDefaultFeeAssetModal is openning when you click on the arrow.
image

@OpenLedgerApp
Copy link
Author

OpenLedgerApp commented Aug 2, 2019

@abitmore
image

@abitmore
Copy link
Member

abitmore commented Aug 3, 2019

@OpenLedgerApp thanks. The transaction about proposal is fine, but I wrongly stated my needs. So this is the update: in the first picture, the transaction to cancel multiple orders in a batch, I need a JSON of the whole transaction, but not (only) multiple JSONs for individual operations.

@sschiessl-bcp
Copy link
Contributor

@OpenLedgerApp thanks. The transaction about proposal is fine, but I wrongly stated my needs. So this is the update: in the first picture, the transaction to cancel multiple orders in a batch, I need a JSON of the whole transaction, but not (only) multiple JSONs for individual operations.

The "Show raw json" shows you the tree of the whole transaction. Let's do copyable text in #3020

@abitmore
Copy link
Member

abitmore commented Aug 9, 2019

The "Show raw json" shows you the tree of the whole transaction.

What I meant is a json of the whole transaction to be signed, but not an operation in the transaction. Current implementation only shows a tree of a single operation. I asked for barter because I made a mistake, thought it would contain multiple operations. Later I asked for "the transaction to cancel multiple orders in a batch" which will contain multiple operations in a transaction.

@sschiessl-bcp
Copy link
Contributor

@OpenLedgerApp does it only show assets that are valid in fee pool and user balance to actually pay the tx fee?

@OpenLedgerApp
Copy link
Author

@sschiessl-bcp no, it shows all user balances, like on the settings page

@sschiessl-bcp
Copy link
Contributor

@sschiessl-bcp no, it shows all user balances, like on the settings page

It should only show asset that can actually be used, in the settings that is fine, but here it would break UX. Please add it to this issue, but please note #3044 first.

@sschiessl-bcp
Copy link
Contributor

Do I assume correctly that you are not interested in finishing this task? @OpenLedgerApp

@OpenLedgerApp
Copy link
Author

@sschiessl-bcp No, i'm interested in this task, but i will finish it in a few weeks

@sschiessl-bcp
Copy link
Contributor

@sschiessl-bcp No, i'm interested in this task, but i will finish it in a few weeks

Could you please be a bit more precise in the expected timeline?

@sschiessl-bcp
Copy link
Contributor

I assume you have no interest anymore.

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.

[2] Allow "single" change of Fee asset on TX
3 participants