Refactoring of the Dapp Registry #4589
Conversation
ngotchac
added
A3-inprogress ⏳
|
Wishlist -
|
| @@ -15,10 +15,12 @@ | |||
| /* along with Parity. If not, see <http://www.gnu.org/licenses/>. | |||
| */ | |||
|
|
|||
| @import '../_colors.css'; | |||
|
|
|||
| .body { | |||
| color: #333; | |||
There was a problem hiding this comment.
Probably would have made sense to put this colour in colors.css as well. (Minor)
| open: false | ||
| }; | ||
|
|
||
| dappsStore = DappsStore.instance(); |
js/src/dapps/dappreg/Modal/modal.css
Outdated
| text-align: center; | ||
| z-index: 50; | ||
| align-items: center; | ||
| background-color: rgba(40, 40, 40, 0.75); |
There was a problem hiding this comment.
Also would have put this in colors.css (There are a couple, not commenting on all). Not a crisis where there are no re-use, bust consistency for all.
| > * { | ||
| display: inline-block; | ||
| font-family: 'Roboto Mono', monospace; | ||
| font-size: 0.8rem; |
There was a problem hiding this comment.
Slightly different from the 0.75rem used elsewhere. (Although this is inside a header.)
js/src/dapps/dappreg/_colors.css
Outdated
| @@ -15,5 +15,4 @@ | |||
| /* along with Parity. If not, see <http://www.gnu.org/licenses/>. | |||
| */ | |||
|
|
|||
| .app { | |||
| } | |||
| $blue: rgb(41, 128, 185); | |||
There was a problem hiding this comment.
Would have made the name "title-background" or "title-color" (something specific, not tied to the color but actual usage thereof)
| /> | ||
| <Button | ||
| className={ styles.button } | ||
| label='Fetch Registry' |
There was a problem hiding this comment.
I find this a bit confusing when I looked at the UI. "Reset" maybe?
There was a problem hiding this comment.
So this is in fact for #4342
It fetches the registered IMG and CONTENT hashes from the registry, based on the selected dapp owner : owner address ==(registry reverse)==> registry name ==(registry lookup)==> [ IMG_GHH_hash ; CONTENT_GHH_hash ] ==> fill the form.
So it's not a reset, it's fetching data from the registry, based on the selected owner address. Couldn't think of a better explicit name though :/
There was a problem hiding this comment.
Ahhh, that one. Ok, will ponder a bit on a better more explicit name, it escapes me at present as well...
|
Couple of minor comments. My only grumble is that we lost the ability for the contract owner to remove apps from the dapp. (e.g. the contract owner account should have the ability to delete any dapp - existing in current version, not in this one - it can never edit them, but can remove any app) Huge step forward, I really like it as a base to build on now. |
jacogr
added
A5-grumble 🔥
ngotchac
added
A0-pleasereview 🤓
|
Ok, so I addressed the PR grumbles, and added the ability for the dapp reg contract owner to delete dapps. |
jacogr
added
A8-looksgood 🦄
* Added React Hot Reload to dapps + TokenDeplpoy fix (#4846) * Fix method decoding (#4845) * Fix contract deployment method decoding in Signer * Linting * Fix TxViewer when no `to` (contract deployment) (#4847) * Added React Hot Reload to dapps + TokenDeplpoy fix * Fixes to the LocalTx dapp * Don't send the nonce for mined transactions * Don't encode empty to values for options * Pull steps from actual available steps (#4848) * Wait for the value to have changed in the input (#4844) * Backport Regsirty changes from #4589 * Test fixes for #4589
Refactoring of the Dapp Registry built-in dapp.
No need to register the Manifest/Image/Content URL in GithubHint dapp anymore, the Dapp Registry is taking care of this now (thus, is becoming a secure dapp).
New UI as well.
Stil todo: