Skip to content

Fix: Add rounding for big number display (Issue #187)#191

Merged
vkoves merged 2 commits into
vkoves:mainfrom
Natkuma01:187-add-rounding-for-big-number
Jun 19, 2025
Merged

Fix: Add rounding for big number display (Issue #187)#191
vkoves merged 2 commits into
vkoves:mainfrom
Natkuma01:187-add-rounding-for-big-number

Conversation

@Natkuma01

Copy link
Copy Markdown
Contributor

Description

Add a floor function with the condition that if the value is greater than 1000, round it up to a whole number; if it is less than 1000, do not make any changes.

Fixes #
Issue #187

#Before Fix
Screenshot 2025-06-15 at 4 00 53 AM
The values for District Steam Use and District Chilled Water Use are displayed with one decimal place, even if the number greater 1000.

#After Fix
Screenshot 2025-06-15 at 3 01 14 AM

Changes Made

  1. on HistoricalBuildingDataTable.vue
    Screenshot 2025-06-15 at 3 56 21 AM
    I noticed there are comments mentioning the need to round and process an optional float to a locale string, so I added that to the code. However, this refers to the Table, so please delete my code if this is not an issue.

  2. on StatTile.vue
    Screenshot 2025-06-15 at 3 08 28 AM
    This fixed the issue for the number format under District Steam Use and District Chilled Water Use

This is my first time making an open-source contribution. I'm happy to learn more if any part of this format is incorrect.

@netlify

netlify Bot commented Jun 15, 2025

Copy link
Copy Markdown

Deploy Preview for radiant-cucurucho-d09bae ready!

Name Link
🔨 Latest commit d04c588
🔍 Latest deploy log https://app.netlify.com/projects/radiant-cucurucho-d09bae/deploys/68526101fa8b9f00081cde11
😎 Deploy Preview https://deploy-preview-191--radiant-cucurucho-d09bae.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@vkoves

vkoves commented Jun 18, 2025

Copy link
Copy Markdown
Owner

@Natkuma01 - this looks great, but could you make a separate function for the rounding and put it into common-functions.vue? That way we're not duplicating the < 1_000 check in several places. And the toLocaleString is correct, since it adds in the commas!

@vkoves

vkoves commented Jun 18, 2025

Copy link
Copy Markdown
Owner

Also @Natkuma01 how did you hear about this project? I'm surprised to get a random contributor, but we definitely appreciate the help! 🎉

@Natkuma01

Copy link
Copy Markdown
Contributor Author

Hi @vkoves, sorry if this is a little awkward. I'm a CS college student. I found your issues on https://goodfirstissues.com/. Your project has a tag of "good first issue" and I'm new to open source projects; your issue seems like a good start and practice for me.

I refactored the code , add a function in common-functions.vue
Screenshot 2025-06-18 at 3 03 16 AM

I’m totally open to revisiting or refining the code as many times as needed. I have whole semester 🤗
Thanks for letting me practice.

@vkoves vkoves left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This looks great, thank you for the contribution!

Comment thread src/common-functions.vue
/**
* Rounds a number to whole number if it is greater than 1000,
* otherwise returns the original number
* This function always returns a NUMBER

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Small optional note, this comment is unnecessary because of the typing - since the function has the number return type, we know that it isn't returning a string.

@vkoves

vkoves commented Jun 19, 2025

Copy link
Copy Markdown
Owner

The linting is failing (I had to approve it running), but since this is a fork I'm just going to merge and I'll run yarn lint-fix on master. Thanks again for the contribution!

@vkoves vkoves merged commit 93ae96a into vkoves:main Jun 19, 2025
5 of 6 checks passed
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