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

PEAR-452: Add thousands comma separator for cDAVE and elsewhere #1347

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

paribartandhakal
Copy link
Contributor

@paribartandhakal paribartandhakal commented Oct 14, 2024

Description

Checklist

  • Added proper unit tests
  • Left proper TODO messages for any remaining tasks
  • Scanned for web accessibility with aXe, and mitigated or documented
    flagged issues

Screenshots/Screen Recordings (if Appropriate)

@paribartandhakal paribartandhakal added the WIP Work In Progress label Oct 14, 2024
@wteouchicago
Copy link
Contributor

  1. in cDAVE, the TSV download from the table includes commas for the property values. Can we remove those commas from the TSV download unless it's a pain to do so? While it's not the best TSV for sorting, some people may want to use scripts to parse (and we'll be consistent with other TSV counts).

  2. in cDAVE, the custom bins, we don't need to add the commas for the field inputs (we only want the comma for the "instructions"/information section.). When I type in various numbers like 11111 or 2222.22, I'm getting NaNs in the error messages.

  3. I'm having trouble typing in numbers in Cohort Builder and getting it to "accept" the number I type. For example, for Demographic > Age At Diagnosis, when I type in "11111" in the To:, it's becoming "11". Just in case this is due to comma separating -- we don't need to comma separate inputs.

  4. Badge number on the Cart in the header doesn't seem to have the comma separator.

@paribartandhakal
Copy link
Contributor Author

CI pipeline failure notice: Need to update typescript after I update mantine. [WIP]

@paribartandhakal paribartandhakal added the WIP Work In Progress label Oct 18, 2024
@wteouchicago
Copy link
Contributor

  1. in cDAVE, the TSV download from the table includes commas for the property values. Can we remove those commas from the TSV download unless it's a pain to do so? While it's not the best TSV for sorting, some people may want to use scripts to parse (and we'll be consistent with other TSV counts).

We decided to skip fixing this since the easy way would affect enums that should have commas in them.

Issues 2 - 4 all look good. 👍

@paribartandhakal paribartandhakal removed the WIP Work In Progress label Oct 22, 2024
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