fix: change user overview info #3332

adam-strzelec · 2024-10-15T09:27:59Z

Description

Change appearance of action overview card info

Testing

Create action with reputation
Finalize action

Diffs

New stuff ✨

Display date of complete action

Changes 🏗

Change text inside cards

Resolves #31415

CLAassistant · 2024-10-15T09:28:05Z

All committers have signed the CLA.

rdig

All good here, tested both with voting and without (just staking)

Also, a "action with reputation" is called a motion

jakubcolony

Thanks for your PR @adam-strzelec 👍

Unfortunately, when I voted support as a user who did not stake, I got the failed message despite the motion passing:

jakubcolony · 2024-10-22T22:55:22Z

src/components/v5/common/ActionSidebar/partials/Motions/steps/FinalizeStep/hooks.tsx

@@ -116,6 +119,7 @@ export const useClaimConfig = (
    actionData.motionData.motionStateHistory.hasFailedNotFinalizable;

  const userStake = usersStakes.find(({ address }) => address === userAddress);
+  const userVote = voterRecord.some((item) => item.address === userAddress);


how about hasUserVoted to indicate it's a boolean and not the vote value?

jakubcolony · 2024-10-22T22:56:37Z

src/components/v5/common/ActionSidebar/partials/Motions/steps/FinalizeStep/FinalizeStep.tsx

-              ? 'motion.finalizeStep.failed.statusText'
-              : 'motion.finalizeStep.statusText',
+            id:
+              isMotionFailedNotFinalizable || (!userStake && userVote)


Not sure I'm following this. Why do we want to show failed status text if user did not stake but has voted?

adam-strzelec · 2024-10-29T13:46:34Z

@jakubcolony done. According to Arren, in the case where "user who did not stake but did vote" and with the outcome passed, it should have the description "Action has passed and can be executed."

jakubcolony

Sorry, I'm still unsure this is correct. When I staked and voted, I saw this when motion was ready to be finalized:

Shouldn't that say "Action has passed and can be executed"? Before finalization, I don't think we can consider the action to be "complete".

Then as a user who staked but not voted, the same motion step had a different label:

And that persisted even past finalization (at which point the action has been executed, so the label seems incorrect):

Do you actually need to take into account whether the user has voted? I would've thought you only need to consider these cases:

Motion ready to be finalized: Action has passed and can be executed.
Motion finalized: Action has passed and is now complete.
Motion failed: Keep existing copy.

arrenv · 2024-11-06T09:25:21Z

Motion ready to be finalized: Action has passed and can be executed.
Motion finalized: Action has passed and is now complete.
Motion failed: Keep existing copy.

@adam-strzelec Just to confirm, Jakub is correct here.

Everyone should see the same description in the last steps.

bassgeta

Nicely ironed out all the cases and it's looking crisp right now 👍
A completely supported motion ✔️

A motion which went through voting ✔️

A rejected motion ✔️

iamsamgibbs

This is working great for motions which are approved:

User who staked: (It does show a negative reward which I don't think is correct).

User who did not (should a user who did not stake but has a reward because they voted, be able to claim the reward? I think they should but there is no claim button here for them)

After claiming as the other user, the claimed pill shows here.

As a user who did not stake or vote:

However, when a motion is rejected, the text still says it has passed:

And when finalised the rejection:

There is also no claim button but the UserHub implies there should be.

I think the claiming issues are out of scope for this, but the wording should be fixed up here for rejected motions.

adam-strzelec · 2024-11-12T15:15:23Z

@iamsamgibbs

The reward for user who staked but not voted should be positive?
the claimed pill should be visible for everyone?
If oppose wins, what should be the text instead of "Action has passed and can be executed", and what instead of "Action has passed and now is complete"

iamsamgibbs · 2024-11-12T15:26:17Z

@adam-strzelec we might need some clarification from @arrenv for all of these:

Sorry, I didn't make it clear. That user had staked and voted. ~~I believe their reward should be positive. (but I could be mistaken)~~ Actually I think it might be correct that the reward is negative.
I think the claim button should be visible for every who has either a stake to reclaim, or a reward to claim.
I'll let Arren confirm the wording for when a motion has been rejected.

arrenv · 2024-12-06T09:31:28Z

@adam-strzelec @iamsamgibbs

It depends if the staker was on the winning or losing side of the vote. If they were on the winning side, it will be positive, if they were on the losing side it will be negative.
The "Claimed" status should be visible per individual who had staked and/or received a reward. I'm pretty sure that rewards don't need to be claimed though and are transferred automatically, so, perhaps they is where the confusion is in the app. However, we should show to that recipient the same "Claimed" status pill.
The copy should be "Action was opposed and cannot be executed.". Figma link

iamsamgibbs

Nice job, thanks for the changes.

The wording is now correct for a failed motion:

Also looks good as a user who voted but did not stake:

This issue I mentioned previously with the transaction showing "Claimable" but no claim button being visible is occurring on master, so can definitely be addressed separately.

iamsamgibbs · 2024-12-12T10:18:03Z

src/components/v5/common/ActionSidebar/partials/Motions/steps/FinalizeStep/FinalizeStep.tsx

@@ -107,6 +108,12 @@ const FinalizeStep: FC<FinalizeStepProps> = ({
    actionData,
    refetchColony,
  ]);
+  // console.log('action data: ', actionData);


Leftover comment

bassgeta

A lot of hard work went into this, thanks for all the on the fly adjustments and following it through 💯
Finalizable ✔️

Finalized ✔️

Rejected ✔️

After voting as user who voted ✔️

rdig

Re-approving good to go

jakubcolony

Thank you @adam-strzelec, this is good to go. Just a reminder about the leftover console.log.

Passed without voting:

Passed with voting:

Failed without staking or voting:

Failed with opposing stake:

On a side note, there is a bug here (not introduced by this PR): #4004

Failed after voting:

Also a bug here, again not introduced by this PR. Added to the issue above.

Nice work! 💪

mmioana

Thanks for picking up this issue @adam-strzelec 🙌

The overview as a user who staked and voted

The overview as a user who only voted

After the reward was claimed

After fully supporting the staking step and claiming the reward

After fully opposing the staking step

After opposing the voting step

And after both users opposed the voting step, but only leela staked

Nice job! 👍

arrenv

@adam-strzelec Thank you for your continued work on this PR. All looks mostly resolved on the actions side, but I ran into a couple of issues and noticed there a few issues still with agreements:

For a user that voted on the winning side, but did not stake, it shows that they have rewards correctly, however, it is showing a "Claim" button. The reward should auto pay out on the reveal so there shouldn't need to be a claim button in this case. The claim is button should be required if they staked. At the moment, when clicking in this state you get this error in console and nothing else happening in the UI.
I have a missing translation error showing up in the feed.
There is a margin bottom, this should be removed, without impacting the margin for when the button is present.
For Agreements, the description for users who DID NOT STAKE is not correct. Given they cannot claim or return stakes for others. The text should be the same format others, with the change from Action to Agreement.
- PASSED: "Agreement has passed and now is complete."
- FAILED: "Agreement has failed and now is complete."
For Agreements, the description and button text for users who DID STAKE is not correct. They should be:
- Unclaimed stakes:
  - PASSED: "Agreement has passed, stakes can be claimed."
  - FAILED: "Agreement has failed, stakes can be claimed."
- Claimed stakes:
  - PASSED: "Agreement has passed and now is complete."
  - FAILED: "Agreement has failed and now is complete."
When the agreement fails, the staking overview is still not correct. It is showing the stake correctly, but not a total, and the button is not visible, despite there being a stake available to claim.

jakubcolony · 2025-01-20T22:21:19Z

Hey @adam-strzelec, just a heads up there have been changes requested by Arren that need to be addressed before we can review further. Thanks

…py for opposed action

adam-strzelec · 2025-01-21T20:45:40Z

@arren done. I was not able to replicate the bugs from points 2 and 6. They work fine for me.

jakubcolony

Thank you for your continued work on this PR, motions are pretty complex so nice work tackling that.

A few UI issues that I noted:

There is some extra padding when finalizing a motion:

Spacing between elements breaks when claim button goes into pending state:

Screen.Recording.2025-01-21.at.22.47.37.mov

Motion failed without staking ✅

Motion failed after objecting - there should be a claim button here, but it was broken before (#4004)

As a user who staked and voted (support) ✅

As a user who staked and voted (oppose) - unable to claim, part of the same issue #4004:

As a user who didn't stake but voted (support) ✅

As a user who didn't stake but voted (oppose) ✅

As a user who staked but didn't vote (support) ✅

As a user who staked but didn't vote (oppose) - again, should be able to claim, but the overview itself is correct ✅

mmioana

Thank you for your continuous efforts on this issue @adam-strzelec 🙌

1st motion
As a user who staked and voted with Support

2nd motion
As a user who opposed the staking

3rd motion
As a user who staked and voted Support

As the user who only voted Oppose

4th motion
As a user who staked and voted Support

As a user who only voted Support

Seems good from my side, nice work! 👍

adam-strzelec self-assigned this Oct 15, 2024

adam-strzelec requested review from a team as code owners October 15, 2024 09:28

rdig previously approved these changes Oct 16, 2024

View reviewed changes

jakubcolony requested changes Oct 22, 2024

View reviewed changes

adam-strzelec dismissed rdig’s stale review via 7aa9a18 October 29, 2024 11:24

adam-strzelec requested review from jakubcolony and rdig October 29, 2024 13:46

jakubcolony requested changes Nov 1, 2024

View reviewed changes

adam-strzelec requested a review from jakubcolony November 7, 2024 08:51

bassgeta previously approved these changes Nov 11, 2024

View reviewed changes

iamsamgibbs requested changes Nov 12, 2024

View reviewed changes

adam-strzelec dismissed bassgeta’s stale review via 10456e2 December 3, 2024 12:10

adam-strzelec force-pushed the feat/16108652-agreement-overview branch from 1494d33 to 10456e2 Compare December 3, 2024 12:10

adam-strzelec requested review from iamsamgibbs and bassgeta December 9, 2024 12:07

iamsamgibbs previously approved these changes Dec 12, 2024

View reviewed changes

bassgeta previously approved these changes Dec 12, 2024

View reviewed changes

rdig previously approved these changes Dec 12, 2024

View reviewed changes

jakubcolony previously approved these changes Dec 20, 2024

View reviewed changes

adam-strzelec dismissed stale reviews from jakubcolony, rdig, and bassgeta via 05b8a7c December 23, 2024 11:46

adam-strzelec dismissed iamsamgibbs’s stale review via 05b8a7c December 23, 2024 11:46

mmioana previously approved these changes Dec 30, 2024

View reviewed changes

adam-strzelec requested review from jakubcolony, iamsamgibbs, bassgeta and rdig January 3, 2025 09:23

arrenv requested changes Jan 10, 2025

View reviewed changes

strzelec and others added 6 commits January 21, 2025 15:13

fix: change description for user who not stake but voted

2759395

fix: change status text for passed and completed action

fa4c4f3

fix: user who voted but not staked can claim

7e97828

fix: display claimed pill for users who staked or woted and change co…

8c511b3

…py for opposed action

fix: remove console log

07a5eb2

fix: styles and handle translations for agreements

a8c3454

adam-strzelec dismissed mmioana’s stale review via a8c3454 January 21, 2025 20:43

adam-strzelec force-pushed the feat/16108652-agreement-overview branch from 05b8a7c to a8c3454 Compare January 21, 2025 20:43

adam-strzelec requested review from arrenv and mmioana January 21, 2025 20:46

jakubcolony requested changes Jan 21, 2025

View reviewed changes

fix: add claim button for not supported actions

b3c24ec

adam-strzelec requested a review from jakubcolony January 22, 2025 15:30

mmioana approved these changes Jan 23, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: change user overview info #3332

fix: change user overview info #3332

adam-strzelec commented Oct 15, 2024

CLAassistant commented Oct 15, 2024 •

edited

Loading

rdig left a comment

jakubcolony left a comment

jakubcolony Oct 22, 2024

jakubcolony Oct 22, 2024

adam-strzelec commented Oct 29, 2024

jakubcolony left a comment

arrenv commented Nov 6, 2024

bassgeta left a comment

iamsamgibbs left a comment

adam-strzelec commented Nov 12, 2024

iamsamgibbs commented Nov 12, 2024 •

edited

Loading

arrenv commented Dec 6, 2024 •

edited

Loading

iamsamgibbs left a comment

iamsamgibbs Dec 12, 2024

bassgeta left a comment

rdig left a comment

jakubcolony left a comment

mmioana left a comment

arrenv left a comment

jakubcolony commented Jan 20, 2025

adam-strzelec commented Jan 21, 2025

jakubcolony left a comment

mmioana left a comment

fix: change user overview info #3332

Are you sure you want to change the base?

fix: change user overview info #3332

Conversation

adam-strzelec commented Oct 15, 2024

Description

Testing

Diffs

CLAassistant commented Oct 15, 2024 • edited Loading

rdig left a comment

Choose a reason for hiding this comment

jakubcolony left a comment

Choose a reason for hiding this comment

jakubcolony Oct 22, 2024

Choose a reason for hiding this comment

jakubcolony Oct 22, 2024

Choose a reason for hiding this comment

adam-strzelec commented Oct 29, 2024

jakubcolony left a comment

Choose a reason for hiding this comment

arrenv commented Nov 6, 2024

bassgeta left a comment

Choose a reason for hiding this comment

iamsamgibbs left a comment

Choose a reason for hiding this comment

adam-strzelec commented Nov 12, 2024

iamsamgibbs commented Nov 12, 2024 • edited Loading

arrenv commented Dec 6, 2024 • edited Loading

iamsamgibbs left a comment

Choose a reason for hiding this comment

iamsamgibbs Dec 12, 2024

Choose a reason for hiding this comment

bassgeta left a comment

Choose a reason for hiding this comment

rdig left a comment

Choose a reason for hiding this comment

jakubcolony left a comment

Choose a reason for hiding this comment

mmioana left a comment

Choose a reason for hiding this comment

arrenv left a comment

Choose a reason for hiding this comment

jakubcolony commented Jan 20, 2025

adam-strzelec commented Jan 21, 2025

jakubcolony left a comment

Choose a reason for hiding this comment

mmioana left a comment

Choose a reason for hiding this comment

CLAassistant commented Oct 15, 2024 •

edited

Loading

iamsamgibbs commented Nov 12, 2024 •

edited

Loading

arrenv commented Dec 6, 2024 •

edited

Loading