Closed Metizik closed 5 years ago
Will do integration test and code review on monday
See here as well, starting https://github.com/blockchainprojects/bitshares-ui/issues/111#issuecomment-502655596
@sschiessl-bcp , all TODOs and referenced issues are closed now, if there are no other change requests I believe this request can be synced with actual develop branch and merged
You can try my account on mainnet (no login necessary) http://localhost:8080/account/sschiessl
When the modal is opened, the default asset for fee and the fee amount is loaded and then the remaining assets of the user account queried and displayed in the fee asset select dropdown.
I want to change the logic: Show the default asset and asset fee as is now, but only load other choices lazily when the user clicks on the dropdown. Following reasoning:
Ideally, the modal loads the default and the checkbox is clickable. When the user click, a loading indicator is shown and when done the dropdown expands.
The select modal also doesnt seem to load properly when opened from within the new component. Select e.g. BADCOIN from my account
@sschiessl-bcp , I fixed all the places to use default fee asset from settings appropriately. Left to do is lazy loading of available assets, which will be done today.
@sschiessl-bcp , all stated tasks are done.
Regarding lazy load - I did not find a way to implement lazy loading like you proposed with AssetSelect
component, but I removed all redundant blockchain fetching made it lightweight enough IMO, to address issue you mentioned.
We have a few modals left that could be refactored to use new FeeAssetSelector
. Should those be refactored too?
@sschiessl-bcp , all stated tasks are done. Regarding lazy load - I did not find a way to implement lazy loading like you proposed with
AssetSelect
component, but I removed all redundant blockchain fetching made it lightweight enough IMO, to address issue you mentioned. We have a few modals left that could be refactored to use newFeeAssetSelector
. Should those be refactored too?
Which modals are you referencing that could be refactored?
Please state which modals are missing. I will forward this PR now to not get out of touch with codebase too much. We can use new issue to streamline usage.
@sschiessl-bcp , modals I am talking about:
General
Closes #111
Refactored SendModal and extracted FeeAssetSelector component with all logic related to fee calculations and managing assets to use. Implemented SetDefaultFeeAssetModal, which allows to select Asset to pay fee and update settings as described in issue comments. Added settings entry for default fee asset selection and integrated it with new modal. Integrated default fee asset settings into Fee Schedule view of the wallet.
General
Please make sure the following is done:
Code Preparation
Please review all your changes one last time before committing
Testing
The branch has been tested on the following browsers (desktop and mobile view)
User interface changes
Delete this section if there weren't any UI changes. Please make sure you tested your changes in all themes
Please provide screenshots/licecap of your changes below