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: truncate no. of peers to unit of "k" when > 999 #1053

Merged

Conversation

Carson12345
Copy link
Contributor

@Carson12345 Carson12345 commented Feb 18, 2022

#1017

About
The issue opener found out that when number of peers is 1000 or above, the last digit might be partially invisible and cause confusion due to the badge's max width.

The core team replied to the issue suggesting to truncate it in the format of "1k", "2k". So I implemented the approach and made this PR.

Original UI

128388115-9fcf5a09-6e82-4234-9d7d-b9225213ccad

Committed UI Change

  • truncate no. of peers to unit of "k" when > 999 (e.g. the count is 2352 in this case)

Screenshot 2022-02-18 at 11 44 58 AM

Show full number when < 999

Screenshot 2022-02-18 at 11 46 59 AM

This is my first PR, please let me know if I could do it better, looking forward to help with other tasks of this repo and IPFS!

@welcome

This comment was marked as resolved.

@lidel lidel changed the title chore: truncate no. of peers to unit of "k" when > 999 fix: truncate no. of peers to unit of "k" when > 999 Feb 18, 2022
@Carson12345
Copy link
Contributor Author

sorry that my code didn't pass the test for JS lint, has fixed by using standard JS lint and pushed. Thanks for reviewing!

@SgtPooki
Copy link
Member

sorry that my code didn't pass the test for JS lint, has fixed by using standard JS lint and pushed. Thanks for reviewing!

Not a problem. Thanks so much for the contribution! I touched up the code a tiny bit and am giving it my approval now.

Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Thank you, works as expected, needs linter fix tho.

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.

3 participants