poanetwork / poa-dapps-validators

DApp for a list of validators with metadata for POA Network (Core/Sokol). Validators can update metadata using DApp.
https://validators.poa.network
GNU General Public License v3.0
13 stars 41 forks source link

(Feature) New style for xDai network #103

Closed gabitoesmiapodo closed 5 years ago

gabitoesmiapodo commented 5 years ago

Closes #97

Description:

Added support for xDai Network's styles.

Updated styles for the rest of the networks.

Added some standard components taken from other POA Network's dApps, in hopes of having a more coherent and consistent ecosystem.

Some screenshots:

screen shot 2019-01-16 at 17 22 14 screen shot 2019-01-16 at 17 22 24 screen shot 2019-01-16 at 17 26 33 screen shot 2019-01-16 at 17 26 44 screen shot 2019-01-16 at 17 27 09 screen shot 2019-01-16 at 17 33 02 screen shot 2019-01-16 at 17 33 11
vbaranov commented 5 years ago

@gabitoesmiapodo

Also,

gabitoesmiapodo commented 5 years ago

@vbaranov

1) There is no visible loader when switching to Pending changes or All back -> Fixed.

2) Impossible to search by full name (to be truth, it is not the issue of this PR, but it would be nice if it could be fixed) -> I think the way this is implemented it can get quite complicated easily (I mean, it could be improved), but I believe it's working as expected now. Please check it out.

3) change warning text The key is not valid voting Key or you are connected to the wrong chain! Please make sure you have loaded correct voting key in Metamask to The key is not valid voting Key or you are connected to the wrong chain! Please make sure you have loaded correct voting key in Metamask/Nifty Wallet -> Changed it.

4) Does loader object change after assignment? if no, let's change to const -> Fixed.

5) dai and other networks names are meet in different places in the code. Let's define constants for them. -> Done.

6) if to open directly /set page, the loader is shown permanently -> Fixed.

7) wrong loader color for Sokol network by clicking on Update metadata button -> Fixed.

varasev commented 5 years ago

I will also review that.

vbaranov commented 5 years ago

@gabitoesmiapodo

screenshot 2019-01-18 at 19 57 59

vbaranov commented 5 years ago

@gabitoesmiapodo

Will you plan to address the problem above?

gabitoesmiapodo commented 5 years ago

@vbaranov Sorry, I was on vacation until yesterday.

Yes, I'm working on it.

Any other feedback from you or @varasev ?

varasev commented 5 years ago

@gabitoesmiapodo I didn't review this yet, but I'll do this after the error above is fixed.

vbaranov commented 5 years ago

Any other feedback from you

Oh, thanks. No, as far as I can see this is the last issue blocking the merging from my perspective.

gabitoesmiapodo commented 5 years ago

@vbaranov

It should be fixed now.

I think it had something to do with me not rendering unnecessary components when they were not needed (instead of just hiding them), so I changed that and now they are hidden / shown depending on the situation (as they were before I changed that).

vbaranov commented 5 years ago

@gabitoesmiapodo I am still experiencing this issue in https://github.com/poanetwork/poa-dapps-validators/pull/103/commits/a8db3ac471d13900a471998ca53c323e15eaf952

gabitoesmiapodo commented 5 years ago

@vbaranov

OK, any leads about what could be causing it? I'm not an actual validator (and don't have a validator key) and can't test the problem in real conditions.

I'm not sure about what could be causing it, all required nine fields are there.

vbaranov commented 5 years ago

@gabitoesmiapodo

TL;DR: There is no problem with this PR. This is the issue of setup for testing. Could you also revert the last commit? Not rendering is more preferable, than hiding.

I figured out what is going on. Actually, I am not a validator too, and I am using https://github.com/poanetwork/poa-test-setup to test the set of governance dapps. By default, it retrieves the latest version of governance smart-contracts and is trying to bootstrap Sokol network (networkID = 77). But, actually, real Sokol network uses the previous version of smart-contract, where CreateMetadata method accepts only 7 parameters. And this is why @varasev implemented a condition here https://github.com/poanetwork/poa-dapps-validators/commit/4e0bed64461c4c7d8c47bd75c2639d0ec8ac1ec4#diff-7aae9601c64b3824098bf15eead67f6eR66, that adds 2 additional parameters to CreateMetadata method only for xDai network. So, this is an issue with poa-test-setup, not with this update of validators dapp.

varasev commented 5 years ago

ValidatorMetadata.createMetadata method was changed for xDai network and now it expects 9 parameters instead of 7. However, we have previous ValidatorMetadata.createMetadata deployed in Sokol and Core, so for these networks it expects 7 parameters.

varasev commented 5 years ago

The corresponding changes were introduced into Validators DApp in https://github.com/poanetwork/poa-dapps-validators/pull/98 and https://github.com/poanetwork/poa-dapps-validators/pull/102.

gabitoesmiapodo commented 5 years ago

@vbaranov

Okay, reverted the changes for this section.

I hope it's fine now.

varasev commented 5 years ago

Please don't merge this yet, I'll also check.

gabitoesmiapodo commented 5 years ago

@varasev Sure, no problem.