Aman-Mittal commented on code in PR #829:
URL: 
https://github.com/apache/fineract-backoffice-ui/pull/829#discussion_r4359155280


##########
src/app/features/clients/client-form.component.ts:
##########
@@ -421,6 +433,18 @@ import {
                         ></ion-datetime>
                       </ng-template>
                     </ion-modal>
+                    @if (!isEditMode() && dateOfBirth()) {
+                      <app-button
+                        type="button"
+                        intent="neutral"
+                        emphasis="quiet"
+                        size="small"
+                        data-testid="clear-date-of-birth"

Review Comment:
   Nit: the clear action is create-only, so a DOB entered by mistake on an 
existing client still can't be removed. You called that out of scope in the 
description. Worth a follow-up issue so it isn't lost (Fineract accepts 
`dateOfBirth: null` with `dateFormat`/`locale` on `PUT /clients/{id}`).



##########
src/app/features/clients/client-form.component.ts:
##########
@@ -407,7 +413,13 @@ import {
                       'CLIENTS.DATE_OF_BIRTH' | appTranslate
                     }}</ion-label>
                     @if (pickersReady()) {
-                      <ion-datetime-button 
datetime="dateOfBirth-picker"></ion-datetime-button>
+                      <ion-datetime-button datetime="dateOfBirth-picker">
+                        @if (!dateOfBirth()) {
+                          <span slot="date-target">{{

Review Comment:
   The family-member form has the same issue, which #747 asked to check: 
`kyc/client-family-member-form.component.ts:250` renders a bare 
`<ion-datetime-button datetime="family-dob-picker">`, so an empty family-member 
DOB also shows today's date. Since it's the same fix (the `date-target` 
placeholder plus a clear action), could you apply it there in this PR? If you'd 
rather keep this PR small, please open a follow-up issue and link it.



##########
src/app/features/clients/client-form.component.ts:
##########
@@ -421,6 +433,18 @@ import {
                         ></ion-datetime>

Review Comment:
   Not introduced here, but it's next to the change: the DOB picker 
(`id="dateOfBirth-picker"`, just above) has no `max`, so a future date of birth 
can be picked and is only rejected by Fineract on save. `[max]` set to the 
business date (or today) would stop that in the picker. Optional.



##########
src/app/features/clients/client-form.component.ts:
##########
@@ -99,7 +101,11 @@ import {
                 ? ('CLIENTS.EDIT_CLIENT' | appTranslate)
                 : ('CLIENTS.CREATE_CLIENT' | appTranslate)
             }}
-            <app-help-icon 
helpTextKey="HELP.CLIENTS_CONTRACTS_DESC"></app-help-icon>
+            <app-help-icon
+              [helpTextKey]="
+                isEditMode() ? 'HELP.CLIENTS_CONTRACTS_DESC' : 
'HELP.CREATE_CLIENT_DESC'

Review Comment:
   The create case now has its own text, but edit mode still uses 
`HELP.CLIENTS_CONTRACTS_DESC`, which is the Clients list description that #747 
item 2 flagged. An `HELP.EDIT_CLIENT_DESC` ("Update this client's details.") 
would close that out. Non-blocking.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to