diff --git a/e2e/date-picker-revisit.spec.ts b/e2e/date-picker-revisit.spec.ts index 2170db2d0..6ba61fa37 100644 --- a/e2e/date-picker-revisit.spec.ts +++ b/e2e/date-picker-revisit.spec.ts @@ -152,22 +152,19 @@ test.describe('Date pickers on revisited forms', () => { listUrl: '/products/floating-rates', createButton: 'Create Floating Rate', createRole: 'button' as const, - // This form's pickers live inside a @for over rate periods, so they are created after the - // first render rather than with the page. The button and its modal then mount in the same - // change-detection pass and the button binds to nothing -- #548, which `createPickersReady` - // does not reach because the flag is already true by the time a row is added. Expected to - // fail until that is fixed; when it starts passing, drop the marker with the fix. - knownBroken: true, + // Pickers live inside a @for over rate periods (#548). Add two rows so the + // revisit assertion also covers unique ids — a shared id would bind both + // buttons to the first picker. prepare: async (page: import('@playwright/test').Page) => { await page.getByRole('button', { name: 'Add Period', exact: true }).click(); + await page.getByRole('button', { name: 'Add Period', exact: true }).click(); }, + expectUniquePickerIds: true, }, ]; - for (const { name, listUrl, createButton, createRole, prepare, knownBroken } of cases) { + for (const { name, listUrl, createButton, createRole, prepare, expectUniquePickerIds } of cases) { test(`${name} form keeps its pickers usable on every visit`, async ({ page }) => { - if (knownBroken) test.fail(); - const pickerErrors: string[] = []; page.on('console', (message) => { const text = message.text(); @@ -186,6 +183,14 @@ test.describe('Date pickers on revisited forms', () => { .poll(() => blankPickers(page), { message: `visit ${visit} left a picker unbound` }) .toBe(0); + if (expectUniquePickerIds) { + const ids = await page + .locator('ion-datetime-button') + .evaluateAll((buttons) => buttons.map((button) => button.getAttribute('datetime'))); + expect(ids.length).toBeGreaterThan(1); + expect(new Set(ids).size).toBe(ids.length); + } + await page.getByRole('button', { name: 'Cancel', exact: true }).click(); await expect(page).toHaveURL(listUrl); } diff --git a/src/app/features/products/floating-rates/floating-rate-form.component.test.ts b/src/app/features/products/floating-rates/floating-rate-form.component.test.ts index 9192799e3..d388aeaca 100644 --- a/src/app/features/products/floating-rates/floating-rate-form.component.test.ts +++ b/src/app/features/products/floating-rates/floating-rate-form.component.test.ts @@ -25,6 +25,10 @@ import { ActivatedRoute, Router, convertToParamMap } from '@angular/router'; import { of } from 'rxjs'; import { TranslateModule } from '@ngx-translate/core'; import { provideNoopAnimations } from '@angular/platform-browser/animations'; +import { provideIonicTesting } from '../../../testing/ionic-testing'; + +const PERIOD_FROM_DATE_PICKER_0 = 'periodfromDate-picker-0'; +const PERIOD_FROM_DATE_PICKER_1 = 'periodfromDate-picker-1'; describe('FloatingRateFormComponent', () => { let component: FloatingRateFormComponent; @@ -47,6 +51,7 @@ describe('FloatingRateFormComponent', () => { { provide: Router, useValue: routerSpy }, { provide: ActivatedRoute, useValue: { paramMap: of(convertToParamMap({})) } }, provideNoopAnimations(), + provideIonicTesting(), ], }).compileComponents(); @@ -75,10 +80,10 @@ describe('FloatingRateFormComponent', () => { ); component.rate.set({ name: 'BLR', isBaseLendingRate: true, isActive: true }); component.periods.set([ - { fromDate: new Date(2026, 0, 1), interestRate: 9.5, isDifferentialToBaseLendingRate: false }, - { fromDate: new Date(2026, 1, 1), interestRate: 0, isDifferentialToBaseLendingRate: true }, + { fromDate: '2026-01-01', interestRate: 9.5, isDifferentialToBaseLendingRate: false }, + { fromDate: '2026-02-01', interestRate: 0, isDifferentialToBaseLendingRate: true }, { - fromDate: new Date(2026, 2, 1), + fromDate: '2026-03-01', interestRate: null, isDifferentialToBaseLendingRate: false, }, @@ -104,7 +109,7 @@ describe('FloatingRateFormComponent', () => { component.isEditMode.set(true); component.rate.set({ name: 'Updated Rate', isBaseLendingRate: false, isActive: true }); component.periods.set([ - { fromDate: new Date(2026, 5, 1), interestRate: 12, isDifferentialToBaseLendingRate: false }, + { fromDate: '2026-06-01', interestRate: 12, isDifferentialToBaseLendingRate: false }, ]); component.onSubmit(); @@ -116,4 +121,35 @@ describe('FloatingRateFormComponent', () => { expect(arg.ratePeriods?.length).toBe(1); expect(arg.ratePeriods?.[0].interestRate).toBe(12); }); + + it('gives each period row a unique picker id and stamps it on the button', async () => { + component.periods.set([ + { fromDate: '2026-01-01', interestRate: 9.5, isDifferentialToBaseLendingRate: false }, + { fromDate: '2026-02-01', interestRate: 10, isDifferentialToBaseLendingRate: false }, + ]); + fixture.detectChanges(); + await fixture.whenStable(); + fixture.detectChanges(); + await fixture.whenStable(); + + expect(component.periodFromDatePickerId(0)).toBe(PERIOD_FROM_DATE_PICKER_0); + expect(component.periodFromDatePickerId(1)).toBe(PERIOD_FROM_DATE_PICKER_1); + + const buttons: HTMLElement[] = Array.from( + fixture.nativeElement.querySelectorAll('ion-datetime-button'), + ); + expect(buttons).toHaveLength(2); + expect(buttons[0].getAttribute('datetime')).toBe(PERIOD_FROM_DATE_PICKER_0); + expect(buttons[1].getAttribute('datetime')).toBe(PERIOD_FROM_DATE_PICKER_1); + expect(buttons[0].getAttribute('datetime')).not.toBe(buttons[1].getAttribute('datetime')); + + const pickers: HTMLElement[] = Array.from( + fixture.nativeElement.querySelectorAll('ion-datetime'), + ); + const pickerIds = pickers.map((el) => el.id); + if (pickerIds.length > 0) { + expect(pickerIds).toEqual([PERIOD_FROM_DATE_PICKER_0, PERIOD_FROM_DATE_PICKER_1]); + } + expect(fixture.nativeElement.querySelector('#periodfromDate-picker')).toBeNull(); + }); }); diff --git a/src/app/features/products/floating-rates/floating-rate-form.component.ts b/src/app/features/products/floating-rates/floating-rate-form.component.ts index 995927c0c..4390ce382 100644 --- a/src/app/features/products/floating-rates/floating-rate-form.component.ts +++ b/src/app/features/products/floating-rates/floating-rate-form.component.ts @@ -34,7 +34,6 @@ import { IonCardTitle, IonCheckbox, IonDatetime, - IonDatetimeButton, IonIcon, IonInput, IonItem, @@ -46,12 +45,13 @@ import { formatDateToFineract, FINERACT_DATE_FORMAT, FINERACT_LOCALE, + toIsoDate, } from '../../../core/utils/date-formatter'; -import { createPickersReady } from '../../../shared/utils/pickers-ready'; +import { DeferredDatetimeButtonComponent } from '../../../ui/deferred-datetime-button/deferred-datetime-button.component'; /** A single editable rate period row in the form. */ interface RatePeriodRow { - fromDate: Date; + fromDate: string; interestRate: number | null; isDifferentialToBaseLendingRate: boolean; } @@ -77,8 +77,8 @@ interface RatePeriodRow { IonCard, IonCheckbox, IonDatetime, - IonDatetimeButton, IonModal, + DeferredDatetimeButtonComponent, ], template: `
@@ -129,16 +129,14 @@ interface RatePeriodRow { {{ 'FLOATING_RATES.FROM_DATE' | translate }} - @if (pickersReady()) { - - } + @@ -228,9 +226,6 @@ interface RatePeriodRow { ], }) export class FloatingRateFormComponent implements OnInit { - /** See `createPickersReady` — the date buttons must not outrun their pickers. */ - readonly pickersReady = createPickersReady(); - private readonly floatingRatesService = inject(FloatingRatesService); private readonly route = inject(ActivatedRoute); private readonly router = inject(Router); @@ -273,8 +268,8 @@ export class FloatingRateFormComponent implements OnInit { return { fromDate: Array.isArray(arr) && arr.length >= 3 - ? new Date(arr[0], arr[1] - 1, arr[2]) - : new Date(), + ? toIsoDate(new Date(arr[0], arr[1] - 1, arr[2])) + : toIsoDate(new Date()), interestRate: p.interestRate ?? null, isDifferentialToBaseLendingRate: !!p.isDifferentialToBaseLendingRate, }; @@ -283,9 +278,14 @@ export class FloatingRateFormComponent implements OnInit { }); } + /** Unique per row so each button's `getElementById` hits its own picker (#548). */ + periodFromDatePickerId(index: number): string { + return `periodfromDate-picker-${index}`; + } + addPeriod(): void { this.periods().push({ - fromDate: new Date(), + fromDate: toIsoDate(new Date()), interestRate: null, isDifferentialToBaseLendingRate: false, }); diff --git a/src/app/shared/utils/pickers-ready.ts b/src/app/shared/utils/pickers-ready.ts index 235ec92a2..d060aa884 100644 --- a/src/app/shared/utils/pickers-ready.ts +++ b/src/app/shared/utils/pickers-ready.ts @@ -37,6 +37,9 @@ import { Signal, afterNextRender, signal } from '@angular/core'; * readonly pickersReady = createPickersReady(); * ``` * + * A page-level flag does not help pickers created later inside `@for` — by then it is already + * true. Use `DeferredDatetimeButtonComponent` for those rows (see #548). + * * See https://github.com/apache/fineract-backoffice-ui/issues/541. */ export function createPickersReady(): Signal { diff --git a/src/app/ui/deferred-datetime-button/deferred-datetime-button.component.test.ts b/src/app/ui/deferred-datetime-button/deferred-datetime-button.component.test.ts new file mode 100644 index 000000000..13cd5913e --- /dev/null +++ b/src/app/ui/deferred-datetime-button/deferred-datetime-button.component.test.ts @@ -0,0 +1,74 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { DeferredDatetimeButtonComponent } from './deferred-datetime-button.component'; +import { provideIonicTesting } from '../../testing/ionic-testing'; + +const FIRST_PICKER_ID = 'periodfromDate-picker-0'; +const SECOND_PICKER_ID = 'periodfromDate-picker-1'; + +describe('DeferredDatetimeButtonComponent', () => { + let fixture: ComponentFixture; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [DeferredDatetimeButtonComponent], + providers: [provideIonicTesting()], + }).compileComponents(); + + fixture = TestBed.createComponent(DeferredDatetimeButtonComponent); + fixture.componentRef.setInput('datetimeId', FIRST_PICKER_ID); + }); + + it('does not create the button on the first render', () => { + fixture.detectChanges(); + + expect(fixture.nativeElement.querySelector('ion-datetime-button')).toBeNull(); + }); + + it('creates the button after the next render and stamps a real datetime attribute', async () => { + fixture.detectChanges(); + await fixture.whenStable(); + fixture.detectChanges(); + await fixture.whenStable(); + + const button = fixture.nativeElement.querySelector('ion-datetime-button'); + expect(button).toBeTruthy(); + expect(button.getAttribute('datetime')).toBe(FIRST_PICKER_ID); + }); + + it("does not reuse another instance's datetime id", async () => { + const second = TestBed.createComponent(DeferredDatetimeButtonComponent); + second.componentRef.setInput('datetimeId', SECOND_PICKER_ID); + + fixture.detectChanges(); + second.detectChanges(); + await fixture.whenStable(); + fixture.detectChanges(); + second.detectChanges(); + await fixture.whenStable(); + + const firstButton = fixture.nativeElement.querySelector('ion-datetime-button'); + const secondButton = second.nativeElement.querySelector('ion-datetime-button'); + expect(firstButton.getAttribute('datetime')).toBe(FIRST_PICKER_ID); + expect(secondButton.getAttribute('datetime')).toBe(SECOND_PICKER_ID); + expect(firstButton.getAttribute('datetime')).not.toBe(secondButton.getAttribute('datetime')); + }); +}); diff --git a/src/app/ui/deferred-datetime-button/deferred-datetime-button.component.ts b/src/app/ui/deferred-datetime-button/deferred-datetime-button.component.ts new file mode 100644 index 000000000..3f62c448d --- /dev/null +++ b/src/app/ui/deferred-datetime-button/deferred-datetime-button.component.ts @@ -0,0 +1,86 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { Component, Directive, ElementRef, InjectionToken, inject, input } from '@angular/core'; +import { IonDatetimeButton } from '@ionic/angular/standalone'; +import { createPickersReady } from '../../shared/utils/pickers-ready'; + +const DATETIME_TARGET_ID = new InjectionToken('DATETIME_TARGET_ID'); + +/** + * Stamps `datetime` as a real attribute in the constructor, before Ionic's + * `componentWillLoad` reads it. A `[datetime]` property binding is too late (#544). + */ +@Directive({ + selector: 'ion-datetime-button[appStampDatetime]', + standalone: true, +}) +export class StampDatetimeDirective { + constructor() { + inject(ElementRef).nativeElement.setAttribute( + 'datetime', + inject(DATETIME_TARGET_ID), + ); + } +} + +/** + * `ion-datetime-button` that is safe to create inside `@for` / `@if`. + * + * The button resolves its target `ion-datetime` exactly once, in `componentWillLoad`, + * through `getElementById`. A page-level `createPickersReady()` does not help rows added + * after first render — the flag is already true, so the button and its modal mount in the + * same pass and the lookup misses. This wrapper holds its own ready flag, so each instance + * waits for the render that mounts *its* picker. + * + * Lives under `src/app/ui/` per ADR 0005: naming Ionic is the primitive's job, not the + * feature's. Consumers import `DeferredDatetimeButtonComponent` and never the vendor tag. + * + * See https://github.com/apache/fineract-backoffice-ui/issues/548. + */ +@Component({ + selector: 'app-deferred-datetime-button', + standalone: true, + imports: [IonDatetimeButton, StampDatetimeDirective], + providers: [ + { + provide: DATETIME_TARGET_ID, + useFactory: () => inject(DeferredDatetimeButtonComponent).datetimeId(), + }, + ], + template: ` + @if (pickersReady()) { + + } + `, + styles: [ + ` + :host { + display: contents; + } + `, + ], +}) +export class DeferredDatetimeButtonComponent { + /** Id of the `ion-datetime` this button should open. Must be unique per instance. */ + readonly datetimeId = input.required(); + + /** See `createPickersReady` — deferred relative to this instance, not the page. */ + readonly pickersReady = createPickersReady(); +}