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

WETGPCII-2788-trailing-decimal-truncation #3221

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

Conversation

LCRQ
Copy link

@LCRQ LCRQ commented Sep 6, 2024

Summary

ISSUE: Not able to move forward when APTC is selected little higher with third decimal after PCII truncates the trailing decimal

STEPS TO RECREATE:
Create APTC eligible FA app and navigate to PCII
Now, select 'SOME of the tax credit each month' option
Enter eligible aptc with third decimal {Example from video $1099.008}
Notice that decimal is truncated and amount adjusts to $1099
Also, notice the error message saying 'Enter $1,099 or less.' even the amount seen is $1099 after PCII truncated the decimal.

EXPECTATION: No error message to be seen and able to move forward

How to test

  1. Run Unit Tests
  2. Start Storybook
    • Under the list of Components, Click "TextField > MaskedCurrency"
    • Follow the Guidelines for testing the Masked TextField Element
      • Specifically, ensure the input elements is checking against the "pattern" attribute regex. No alpha numbers should be allowed
      • Ensure the error handling is correct border is turning red when alpha characters are entered
      • Ensure truncation of numbers consisting of three decimal points takes place
      • Ensure that number with no valid decimal points are converted to whole numbers

Checklist

  • Prefixed the PR title with the Jira ticket number as [WNMGDS-####] Title or [NO-TICKET] if this is unticketed work.
  • Selected appropriate Type (only one) label for this PR, if it is a breaking change, label should only be Type: Breaking
  • Selected appropriate Impacts, multiple can be selected.
  • Selected appropriate release milestone

If this is a change to design:

  • If visual regression image references have been changed, design MUST be assigned to review. In this instance, designer approval is a requirement before the PR can be merged.

If this is a change to code:

  • Created or updated unit tests to cover any new or modified code
  • If necessary, updated unit-test snapshots (yarn test:unit:update) and browser-test snapshots (yarn test:browser:update)

If this is a change to documentation:

  • Checked for spelling and grammatical errors

@LCRQ LCRQ marked this pull request as draft September 6, 2024 20:03
@LCRQ LCRQ marked this pull request as ready for review September 9, 2024 14:01
@jack-ryan-nava-pbc
Copy link
Collaborator

Hi @LCRQ!
Thanks for submitting a contribution to the CMS Design System. After looking at your changes I have the following suggestions:

  1. See if you can utilize the LabelMask component, it is designed to handle this use case and has several other improvements. Documentation here.
  2. If you cannot use the LabelMask component, you should consider supplying a custom function to the onBlur parameter of the TextInput component. We have established the pattern of firing changes to user input during the onBlur event, rather than the onChange event. Which we consider reserved for user input. Any masking is designed to occur after the user is done typing.
    cc @tamara-corbalt

@kim-cmsds
Copy link
Collaborator

Jenkins, not a wolf

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