-
Notifications
You must be signed in to change notification settings - Fork 103
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
Worked on issue #1215 #1456
base: master
Are you sure you want to change the base?
Worked on issue #1215 #1456
Conversation
Hi @sachdevlaksh thanks for your contribution, it looks like the initial needs to be added to the SpouseAndDependent return value
|
I see. Thanks for the update. Right now my local UsTaxes setup is not able to detect CommonTests.ts as test class, so not able to find root cause in test cases failure. |
@thegrims , Did you get chance to look into PR. |
@sachdevlaksh the UI looks good, I ran the unit tests, and it looks like the first name check needs to be updated, and some other errors need to be fixed. You can validate locally by running |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
failing unit tests
@thegrims I fixed two test error, which i could easily find it was because of my new changes, for the rest two I need your help, want to know what is these all about. |
@@ -22,7 +22,8 @@ import ListItemText from '@material-ui/core/ListItemText' | |||
import PersonIcon from '@material-ui/icons/Person' | |||
|
|||
export const labels = { | |||
fname: 'First Name and Initial', | |||
fname: 'First Name', | |||
initial: 'Initial', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this be more clear if it was called 'Middle Initial'?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, midInitial: 'Middle Initial'
would likely be better naming for the key and the label.
What kind of change does this PR introduce? (delete not applicable)
Fixes #1215