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

feat: Support decimal values in table progress cell type #824 #2329

Merged
merged 2 commits into from
May 15, 2024

Conversation

dbranley
Copy link
Contributor

Closes #824

The PR fulfills these requirements: (check all the apply)

  • It's submitted to the main branch.
  • When resolving a specific issue, it's referenced in the PR's title (e.g. feat: Add a button #xxx, where "xxx" is the issue number).
  • When resolving a specific issue, the PR description includes Closes #xxx, where "xxx" is the issue number.
  • If changes were made to ui folder, unit tests (make test) still pass.
  • New/updated tests are included

As explained in the issue, the table progress cell currently rounds values to whole numbers when diplaying them as a percentage. With this change, the percentage will now render with up to 2 decimal places. This is accomplished by using toFixed(2) and parseFloat() together to ensure data is rounded properly and then trailing zeros are removed. The code change in ui/src/progress_table_cell_type.tsx looks like this:

<div className={css.percent}>{`${Number.parseFloat((progress * 100).toFixed(2))}%`}</div>

Here is a partial screen shot of the Table example where you can see the progress data now being displayed with up to 2 decimal places:

output_example_issue_824

There is a new test case in progress_table_cell_type.test.tsx. The test rerenders the XProgressTableCellType component in a loop to make sure that the component is rendering the following input decimal values correctly:

  progressValues = [
    {input: 0.88, output: '88%'},
    {input: 0.888, output: '88.8%'},
    {input: 0.8888, output: '88.88%'},
    {input: 0.88888, output: '88.89%'},
    {input: 0.88899, output: '88.9%'},
    {input: 0.88999, output: '89%'},]

I ran all the ui tests and confirmed they passed. Here is a screen shot for that:

wave_ui_test_issue_824

@dbranley dbranley requested review from lo5 and mturoci as code owners May 10, 2024 20:16
Copy link
Collaborator

@mturoci mturoci left a comment

Choose a reason for hiding this comment

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

Thanks @dbranley! Just 1 change required, otherwise like it a lot :)

ui/src/progress_table_cell_type.test.tsx Outdated Show resolved Hide resolved
…ers consistently when percentage data has more than 2 decimal places
Copy link
Collaborator

@mturoci mturoci left a comment

Choose a reason for hiding this comment

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

LGTM. Great job @dbranley!

Especially appreciate your independence, quick and constructive discussion and flawless execution (+ you included tests without me explicitly asking!). Prime example of how OSS contributions should look like.

@mturoci mturoci merged commit 0260430 into h2oai:main May 15, 2024
@dbranley dbranley deleted the feat/issue-824 branch May 15, 2024 17:16
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.

Support decimal values in table progress cell type
2 participants