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

Fix: Display user reputation in its non-scientific notation format #2946

Merged
merged 2 commits into from
Aug 15, 2024

Conversation

rumzledz
Copy link
Contributor

@rumzledz rumzledz commented Aug 13, 2024

Description

Before After
image image

Testing

  1. Visit the Members page
  2. Click on leela, the User Info popover should come up
  3. Hover on the Reputation percentage
  4. Verify that reputation value in the tooltip is not being shown in scientific notation anymore

Resolves #2706

@rumzledz rumzledz self-assigned this Aug 13, 2024
@rumzledz rumzledz marked this pull request as ready for review August 13, 2024 18:17
@rumzledz rumzledz requested review from a team as code owners August 13, 2024 18:17
@rumzledz rumzledz changed the title Fix: Add decimals to the reputation calculation Fix: Display user reputation in its non-scientific notation format Aug 13, 2024
mmioana
mmioana previously approved these changes Aug 14, 2024
Copy link
Contributor

@mmioana mmioana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really cool you picked this up @rumzledz 🤩
Now the tooltip value shows up nicely
Screenshot 2024-08-14 at 09 30 22

Only left a small comment if you consider to pick it up 🙏

Comment on lines 144 to 147
decimals={getTokenDecimalsWithFallback(
colony.nativeToken.decimals,
DEFAULT_TOKEN_DECIMALS,
)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the getTokenDecimalsWithFallback already uses DEFAULT_TOKEN_DECIMALS as it's fallback value I think we can skip passing it as the 2nd argument

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh you're right @mmioana , you know what I'll clean up other places where this is being done as well 👌

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mmioana pushed! 🚀 I've also cleaned up other areas that use this redundant 2nd argument

Copy link
Contributor

@bassgeta bassgeta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice small fix and a nice cleanup of the redundant fallback, good job 💪 😤
image
Awarding a reputation shows up nicely
image
🚢

Copy link
Contributor

@davecreaser davecreaser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix, nice clean up, nice job @rumzledz! 🧹

Working nicely:
Screenshot 2024-08-14 at 14 21 16

Copy link
Contributor

@Nortsova Nortsova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job @rumzledz ! Thanks for clean up

Reputation value showed up correctly:
image

@rumzledz rumzledz merged commit 1865431 into master Aug 15, 2024
4 of 6 checks passed
@rumzledz rumzledz deleted the fix/2706-fix-popover-rep-units branch August 15, 2024 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

User popover shows reputation in wrong units
5 participants