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

Undelegation tx still shows Claim button after click #1410

Closed
fedekunze opened this issue Oct 4, 2018 · 18 comments · Fixed by #1516
Closed

Undelegation tx still shows Claim button after click #1410

fedekunze opened this issue Oct 4, 2018 · 18 comments · Fixed by #1516
Assignees
Labels
bug 🐛 issues related to unhandled errors in the code that need to be fixed

Comments

@fedekunze
Copy link
Contributor

fedekunze commented Oct 4, 2018

Description:

I completed my undelegation to claim back the tokens and the undelegation tx still shows the Claim button.

screen shot 2018-10-04 at 1 02 04 pm

Full error:

deliverTx failed: (262246) Msg 0 failed: 
=== ABCI Log ===
Codespace: 4
Code:      102
ABCICode:  262246
Error:    
   --= Error =--
      Data: common.FmtError{format:"no unbonding delegation found", args:[]interface {}(nil)}
      Msg Traces:
   --= /Error =-
-=== /ABCI Log ===
@fedekunze fedekunze added bug 🐛 issues related to unhandled errors in the code that need to be fixed staking1 high priority ❗ labels Oct 4, 2018
@NodeGuy
Copy link
Contributor

NodeGuy commented Oct 8, 2018

I'm confused. Isn't the screenshot showing the error Ending undelegation failed, which means the undelegation didn't complete?

@fedekunze
Copy link
Contributor Author

It completed (hence the Ended Unbonding tx) but it’s still showing the claim button, which when you click it again throws the error

@fedekunze fedekunze self-assigned this Oct 8, 2018
@faboweb faboweb self-assigned this Oct 9, 2018
@fedekunze fedekunze changed the title Undelegation tx still shows Claim button after completion Undelegation tx still shows Claim button after click Oct 9, 2018
@faboweb
Copy link
Collaborator

faboweb commented Oct 10, 2018

Scope:

  • if clicking on the claim button, and the claim is successful, refreshing the UI should not show the claim button

@jbibla
Copy link
Collaborator

jbibla commented Oct 10, 2018

refreshing the UI should not show the claim button

can we start moving towards not having to press the refresh button?

@faboweb
Copy link
Collaborator

faboweb commented Oct 10, 2018

refreshing the UI

this means in development -> when Voyager is restarted

can we start moving towards not having to press the refresh button?

this should be mostly the case already. I would leave a refresh button in there anyway to give users a sense of security.

@jbibla
Copy link
Collaborator

jbibla commented Oct 10, 2018

I would leave a refresh button in there anyway to give users a sense of security.

nooooooooo~~! we should remove them and have proper loading images on every screen and in every component.

giphy

@faboweb
Copy link
Collaborator

faboweb commented Oct 11, 2018

yes, plus a refresh option. which product do you know, that doesn't have a refresh option? I assume users would feel unsecure if the values they are seeing are the latest.

@jbibla
Copy link
Collaborator

jbibla commented Oct 11, 2018

browsers have a refresh option, i don't know any desktop or mobile apps that provide this. can you provide some example apps?

my expectation is that the data i see is always the most up to date. having to click refresh / reload is an indication that we are not providing enough information or feedback to our users. the impulse to click refresh on any of our pages will only happen if we aren't communicating what is happening and the users expectations are different from what has been communicated / displayed.

sometimes, software bugs out - so i would be in favour of command R refreshing Voyager but there should be no reason to provide a refresh button that is specific to a page or component.

@NodeGuy
Copy link
Contributor

NodeGuy commented Oct 11, 2018

Total agreement with Jordan. 90% of apps don't have a refresh option and it's a design failure when they do, including browsers.

@faboweb
Copy link
Collaborator

faboweb commented Oct 11, 2018 via email

@faboweb
Copy link
Collaborator

faboweb commented Oct 11, 2018 via email

@jbibla
Copy link
Collaborator

jbibla commented Oct 11, 2018

ok you are kind of right. sometimes, you can "refresh" but this is more and more like requesting more data, not data that you should have been pushed already.

@NodeGuy
Copy link
Contributor

NodeGuy commented Oct 12, 2018

Having to request more data in those situations is still a design wart.

@fedekunze fedekunze assigned jbibla and unassigned faboweb and fedekunze Oct 23, 2018
@NodeGuy NodeGuy assigned NodeGuy, cwgoes and jbibla and unassigned jbibla and cwgoes Oct 23, 2018
@NodeGuy NodeGuy added the blocked ✋ issues blocked by other implementations/issues label Oct 23, 2018
@NodeGuy
Copy link
Contributor

NodeGuy commented Oct 23, 2018

Blocked because @faboweb needs to publish the new version of tendermint/ui.

@NodeGuy NodeGuy removed their assignment Oct 23, 2018
@NodeGuy NodeGuy assigned faboweb and unassigned jbibla Oct 23, 2018
@cwgoes
Copy link

cwgoes commented Oct 23, 2018

@NodeGuy Did you want anything from me here or was that an accidental assignment?

@NodeGuy
Copy link
Contributor

NodeGuy commented Oct 23, 2018

@cwgoes It was an accident, sorry.

@faboweb
Copy link
Collaborator

faboweb commented Oct 30, 2018

published 0.3.1

@faboweb faboweb removed the blocked ✋ issues blocked by other implementations/issues label Oct 30, 2018
@NodeGuy NodeGuy removed high priority ❗ low priority has been discussed, will be addressed later labels Oct 30, 2018
@faboweb
Copy link
Collaborator

faboweb commented Nov 5, 2018

The unbonding delegation received from /stake/delegator/{addr} doesn't have a height associated. So we can't map transactions with active unbondingDelegations. Will check if this still is the case in the latest develop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 issues related to unhandled errors in the code that need to be fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants