rsksmart / rns-manager-react

RNS Manager - ⌨️ buidling
https://manager.rns.rifos.org/
MIT License
6 stars 13 forks source link

Storage of text records in RNS #470

Closed EvelinaBuiciag closed 1 year ago

EvelinaBuiciag commented 2 years ago

Added to our RNS the text records resolver ENSIP-5/EIP-634 in a separated tab, in the advanced view, with the following functionalities:

lgtm-com[bot] commented 2 years ago

This pull request introduces 1 alert when merging 72fa71ae190f63ef85ecc9e71f3d99777914127b into 4c10bba336e49c9e831c40b83504362897ab05b7 - view on LGTM.com

new alerts:

lgtm-com[bot] commented 2 years ago

This pull request introduces 1 alert when merging e8f79b0bd6b7fac1769a47a30a1ba5905acd70a9 into 4c10bba336e49c9e831c40b83504362897ab05b7 - view on LGTM.com

new alerts:

lgtm-com[bot] commented 2 years ago

This pull request introduces 1 alert when merging c7161b5a043966404d29b85321544e4d40f372ae into 4c10bba336e49c9e831c40b83504362897ab05b7 - view on LGTM.com

new alerts:

lgtm-com[bot] commented 2 years ago

This pull request introduces 1 alert when merging cd7a3812b653c99b19cfc23a8a54f73a8176af03 into 4c10bba336e49c9e831c40b83504362897ab05b7 - view on LGTM.com

new alerts:

lgtm-com[bot] commented 2 years ago

This pull request introduces 1 alert when merging 5aec767458c8934d1e2f1e2cee81b7c572517173 into 4c10bba336e49c9e831c40b83504362897ab05b7 - view on LGTM.com

new alerts:

lgtm-com[bot] commented 2 years ago

This pull request introduces 1 alert when merging 5096ece554d31fce08edfc215a142247cce4205d into 4c10bba336e49c9e831c40b83504362897ab05b7 - view on LGTM.com

new alerts:

EvelinaBuiciag commented 2 years ago

Great job overall! The code looks good and the functionality works when I test it out. I did leave a few comments to improve what you have built. In addition to the code requests, there are a few other things I noticed when I run the app:

Thank you also for the reviews. Addressed almost all requested changes. Please see the comments on the ones remained opened.

  • If I have the public resolver set, it still allows me to put in a text record, however, this resolver does not allow for this method. Instead, I get an error. To test this out, set your resolver to '0x1e7AE43e3503eFB886104ace36051Ea72b301CDf' and attempt to set a text record. The App should first check to see if text records are a supportedInterface and if not, let the user know.

Change addressed on the used resolver. The App checks first the domain resolver to see if text records are a supportedInterface and if not, lets the user know.

image
  • When I add a custom key, it doesn't show up in the list after it is confirmed and the text remains in the text box. This makes it look like it doesn't work. I have to search for the key first and then it will show up and then save into localstorage

For a new custom key, the key is saved from the moment of adding it in the local storage however as the text record component was done similar with the myURL using same operations, reducer and actions from the resolver component and multiple components are using just 1 function from the operations (getTextRecord) the state does't re-render automatically. The user must do some kind of operation (search, refresh etc.)