Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(Bug) Modifiable crowdsale: incorrect behavior if user tries to add existing whitelisted address #596

Closed
dennis00010011b opened this issue Feb 14, 2018 · 8 comments · Fixed by #672

Comments

@dennis00010011b
Copy link

dennis00010011b commented Feb 14, 2018

Steps to reproduce:

  1. Create modifiable crowdsale with whitelisted addresses
  2. Open manage
  3. Add in the whitelist existed whitelisted address, fill out min/max value
  4. Click button "+"
  5. Click button "Save"

Expected result: should be error message like "address already exist ..."
OR user should be allowed to modify existed addresses(Feature)
Actual result: button "Save' is enabled, user can proceed, message "Congrats. You've successfully updated the crowdsale "

If you are reporting a problem with Token Wizard, please include the following information:

Which network did you use? (Mainnet, Kovan, Rinkeby, etc.)

Rinkeby

If you were able to create it, what is the URL of your crowdsale?

https://wizard.poa.network/invest?addr=0x011C0608e9858f22564C31199438f9a732B6f157&networkID=4

Do you have screenshots showing the problem?

incorrbehavior

https://drive.google.com/open?id=1iAliRoG85jqjbjE1Ws_B-Q33yJsUCUSy

Do you see errors in the dev console? If yes, please include a screenshot

To open the dev console in Google Chrome, press F12, or go to View -> Developer -> Developer Tools, and open the Console tab

If you see errors, please right click on them and "Save as..". Zip saved file and attach it to the Issue.

No


@ghost ghost assigned fernandomg Feb 26, 2018
@ghost ghost added in progress and removed to do labels Feb 26, 2018
@pablofullana pablofullana changed the title (Bug) Modifiable crowdsale: incorrect behavior if user try to add existed whitelisted address (Bug) Modifiable crowdsale: incorrect behavior if user tries to add existing whitelisted address Feb 27, 2018
@fernandomg
Copy link
Contributor

@dennis00010011b @vbaranov what you think about adding the address to the list, but highlighting it and with a warning icon?

Using the title attribute:
image

Adding a component for tooltips (https://github.com/wwayne/react-tooltip):
image

Please, let me know which approach you think is the best. Or if you have any other idea to implement this solution.

@vbaranov
Copy link
Collaborator

@fernandomg personally, I like the 2nd one with react-tooltip. My worry is about how to revert this change before saving if a user forgot how many it was reserved initially (I guess, only with refreshing the page). Maybe, we can add a small revert button, also? It will get saved values for this address in smart-contracts.

@fernandomg
Copy link
Contributor

@vbaranov thanks for the reply.

It wasn't that neat solution after all. The address is duplicated in the list (the last one before the highlighted) has the current value.

Maybe I should highlight that as well (with a different style) or differenciate the stored list from the one being created/added.

@vbaranov
Copy link
Collaborator

vbaranov commented Feb 27, 2018

oh, I see now, that it is duplicated in the list.

Maybe I should highlight that as well (with a different style) or differenciate the stored list from the one being created/added.

yes, it would be great to highlight the same address.

@dennis00010011b
Copy link
Author

@fernandomg @vbaranov
2nd one is better. I want to propose one more approach.

  1. User can't add duplicate adresses. If address already added there should be warning
  2. User can modify existing values for address in list below. Min/max fields is modifiable for each address. If user change something then button Save will enabled etc.

@fernandomg
Copy link
Contributor

  1. User can modify existing values for address in list below. Min/max fields is modifiable for each address. If user change something then button Save will enabled etc.

That's neat. There's only one thing that concerns me with this approach. What will happen if the user wants to "bulk-update" several whitelists? He/She will be forced to change one-by-one the items in the list.

Thoughts?

@fernandomg
Copy link
Contributor

@dennis00010011b @vbaranov

I sorted elements by addresses and grouped/highlighted duplicated addresses, and highlighted new addresses.

This is the look and feel
manage-whitelists

@dennis00010011b
Copy link
Author

dennis00010011b commented Feb 27, 2018

Looks good!
Don't forget to add this feature to the manage page. There is option to change whitelist too (if crowdsale modifiable)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants