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

Round the usage percentage to 1 decimal point #1370

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Yingyingcheng
Copy link

Context

As described in #1044, the feature usage chart was displaying percentages with an excessive number of decimal places. We want to eliminate this visual noise and just show one decimal place.

Changes

  • I updated webstatus-feature-usage-chart-panel.ts to use the toFixed() method to round usage percentages to 1 decimal place. Note that toFixed returns a string, so for valueExtractor, I had to convert it back to a number.
  • Also updated webstatus-feature-usage-chart-panel.test.ts to update and add one test case to make sure the changes are covered in the tests.

@Yingyingcheng
Copy link
Author

@jcscottiii please review my changes :) ! Seems like the workflows need your approval to run

Copy link
Collaborator

@jcscottiii jcscottiii left a comment

Choose a reason for hiding this comment

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

@Yingyingcheng thanks for the contribution! Can you make a common helper method named formatUsagePercentage that both the value extractor and tool tip extractor use?

Thanks for modifying both extractors by the way. If you only modified the tooltip, the value extractor would still have the raw value. Causing the graph to be potentially confusing compared to the value in the tooltip.

@jcscottiii
Copy link
Collaborator

FYI The playwright test for the feature page failed due to the new rounding of the values. You will need to regenerate the screenshots

@Yingyingcheng Yingyingcheng requested a review from jcscottiii April 7, 2025 02:17
@jcscottiii
Copy link
Collaborator

@Yingyingcheng Thanks for re-generating the screenshots.

Could you also take a look at this feedback in this comment:

Can you make a common helper method named formatUsagePercentage that both the value extractor and tool tip extractor use?

@Yingyingcheng
Copy link
Author

@jcscottiii just updated, sorry! I forgot to commit the changes from that file

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.

2 participants