feat: Show accounts with unknown network #392
Conversation
|
Thanks for the PR! I have not start the part to change the account store, but basically I will change some state to the account, and will use promise to pipeline the authentication and account getter functions. I agree with the new way to access the account data, and I think merging will not be a big problem. |
| this.scrollToIndex = this.scrollToIndex.bind(this); | ||
| } | ||
|
|
||
| scrollToIndex() { |
There was a problem hiding this comment.
This is useless IMO, and barely working. Removing it (it was playing with accountIds as well)
| @@ -51,10 +51,6 @@ export default class MessageDetails extends React.PureComponent { | |||
| message={isU8a(message) ? u8aToHex(message) : message} | |||
| dataToSign={isU8a(dataToSign) ? u8aToHex(dataToSign) : dataToSign} | |||
| isHash={scannerStore.getIsHash()} | |||
| onPressAccount={async account => { | |||
There was a problem hiding this comment.
This is an unwanted behaviour that is not related to this PR. Users shouldn't be able to click on an account card in the message details view.
| @@ -126,7 +126,7 @@ export default class ScannerStore extends Container<ScannerState> { | |||
| // all the frames are filled | |||
| if (Object.keys(this.state.multipartData.length) === frameCount) { | |||
| // fixme: this needs to be concated to a binary blob | |||
| const concatMultipartData = Object.keys(this.state.multipartData).reduce((result, data) => res.concat(this.state.multipartData[data])); | |||
| const concatMultipartData = Object.keys(this.state.multipartData).reduce((result, data) => result.concat(this.state.multipartData[data])); | |||
There was a problem hiding this comment.
also unrelated to this PR, and this code isn't used (yet) now that's a bug for sure :)
| JSON.stringify(account, null, 0), | ||
| accountsStore | ||
| ); | ||
|
|
||
| export const saveAccounts = accounts => accounts.forEach(saveAccount); |
|
When creating new account, there is still the UNKNOWN NETWORK:
Need to be removed from |
Tbaut
changed the title
| > | ||
| <Text | ||
| { Object.entries(NETWORK_LIST) | ||
| .filter(([networkKey]) => networkKey !== UnKnownNetworkKeys.UNKNOWN ) |
There was a problem hiding this comment.
The commit looks bigger than it is, all I did is add this filter to make sure Unkown network isn't showing up.
|
Nice catch, thanks for the review. Your comments were addressed :) |
Co-Authored-By: YJ <yjkimjunior@gmail.com>
Co-Authored-By: YJ <yjkimjunior@gmail.com>
Co-Authored-By: YJ <yjkimjunior@gmail.com>
Co-Authored-By: YJ <yjkimjunior@gmail.com>
Co-Authored-By: YJ <yjkimjunior@gmail.com>
Co-Authored-By: YJ <yjkimjunior@gmail.com>
Co-Authored-By: YJ <yjkimjunior@gmail.com>
Tbaut
changed the title
closes #389
How to test:
networkKeyof Kusama insrc/constants.jsBecause the genesisHash used to create (1) isn't known any more, we can't match the Kusama Network. Still this account can be seen, its backup retrieved with the pin and deleted.
edit:
This was quite a painful PR, trying to remove many weird things that were done. Now we use the same [key, value] mapping in the accountStore and in the db. This allows to show accounts which we have no corresponding
NetworkKey. It also helps to seemlessly save or delete accounts (even if the network is not known) because the db and the store have the same keys.This PR introduces the notion of
selectedKeythat corresponds to aselectedAccount(more broadly namedselectedin the app). I think it's quite straightforward, we should use this key as much as possible and only useaccountId()when dealing with Data coming in (QR code scanning) or saving new accounts.I tested the migration but please feel free to do it as well (as explained here)