New results #1096
New results #1096
Conversation
|
Dynamic branch URL: https://app-eligibilityestimator-dev-pr-preview-0.azurewebsites.net |
|
Dynamic branch URL: https://app-eligibilityestimator-dev-pr-preview-0.azurewebsites.net |
There was a problem hiding this comment.
Looking great Max, well done! This was a lot of work here. I am still reviewing but the only things I am seeing now are:
- potentially padding could be larger in places? (haven't consulted the Figma designs myself). Specifically, referring to the arrows on dropdowns being close to their border and numbers in the table seem cramped.
- This one here, seems like wrong messaging. For reference, the user was 67.5 and partner 61.5 years old and I am getting the message: "until the age 62..." for partner which doesn't seem right.
| import { WebTranslations } from '../../i18n/web' | ||
| import { MaritalStatus } from '../../utils/api/definitions/enums' | ||
| import { useTranslation } from '../Hooks' | ||
|
|
| /* | ||
| returns benefit name with from/de and proper article. ... french nuances. | ||
| */ | ||
|
|
||
| const openModal = (e) => { | ||
| setModalPosition({ x: e.clientX, y: e.clientY }) |
There was a problem hiding this comment.
See comment in the Modal component as well. Don't think tracking position is necessary.
| }> = ({ partner, resultObject, resultArray, age, maritalStatus }) => { | ||
| const tsln = useTranslation<WebTranslations>() | ||
| const apiTrans = getTranslations(tsln._language) | ||
| age = Math.round(age) |
There was a problem hiding this comment.
passed in props should be immutable. Instead of reassigning, should assign to a local var, "roundedAge" maybe?
|
|
||
| const eligibleAmt = numberToStringCurrency(eligibleTotalAmount, language) | ||
|
|
||
| const arrayofben = benefitResultArray |
There was a problem hiding this comment.
small comment, just for consistency: "arrayOfBen"
| } | ||
|
|
||
| //BUILD THE SUMMARY STRINGS FOR EACH BENFIT | ||
| const buildSummaryString = () => { |
There was a problem hiding this comment.
This function is a bit difficult to read with the nested ifs but should be alright. Could try to refactor with an LLM potentially?
|
Dynamic branch URL: https://app-eligibilityestimator-dev-pr-preview-0.azurewebsites.net |
|
Dynamic branch URL: https://app-eligibilityestimator-dev-pr-preview-0.azurewebsites.net |
|
Dynamic branch URL: https://app-eligibilityestimator-dev-pr-preview-0.azurewebsites.net |
|
Dynamic branch URL: https://app-eligibilityestimator-dev-pr-preview-0.azurewebsites.net |
|
Dynamic branch URL: https://app-eligibilityestimator-dev-pr-preview-0.azurewebsites.net |
|
closed to open new PR with dynamic link |
AB#145598 - New Results
Description
List of proposed changes:
What to test for/How to test
Additional Notes