Skip to content

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

Closed
OpenLedgerApp wants to merge 3 commits intobitshares:developfrom
OpenLedgerApp:#2969-default-fee-asset-for-tx
Closed

#2696. use fee selection feature on the transaction confirmation#3001
OpenLedgerApp wants to merge 3 commits intobitshares:developfrom
OpenLedgerApp:#2969-default-fee-asset-for-tx

Conversation

@OpenLedgerApp
Copy link
Copy Markdown

@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
Copy Markdown
Contributor

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

@sschiessl-bcp
Copy link
Copy Markdown
Contributor

sschiessl-bcp commented Jul 31, 2019

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

@abitmore
Copy link
Copy Markdown
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
Copy Markdown
Author

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

@OpenLedgerApp
Copy link
Copy Markdown
Author

OpenLedgerApp commented Aug 2, 2019

@abitmore
image

@abitmore
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Author

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

@sschiessl-bcp
Copy link
Copy Markdown
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
Copy Markdown
Contributor

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

@OpenLedgerApp
Copy link
Copy Markdown
Author

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

@sschiessl-bcp
Copy link
Copy Markdown
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
Copy Markdown
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