Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions e2e/date-picker-revisit.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -47,6 +51,7 @@ describe('FloatingRateFormComponent', () => {
{ provide: Router, useValue: routerSpy },
{ provide: ActivatedRoute, useValue: { paramMap: of(convertToParamMap({})) } },
provideNoopAnimations(),
provideIonicTesting(),
],
}).compileComponents();

Expand Down Expand Up @@ -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,
},
Expand All @@ -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();
Expand All @@ -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();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import {
IonCardTitle,
IonCheckbox,
IonDatetime,
IonDatetimeButton,
IonIcon,
IonInput,
IonItem,
Expand All @@ -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;
}
Expand All @@ -77,8 +77,8 @@ interface RatePeriodRow {
IonCard,
IonCheckbox,
IonDatetime,
IonDatetimeButton,
IonModal,
DeferredDatetimeButtonComponent,
],
template: `
<div class="form-container">
Expand Down Expand Up @@ -129,16 +129,14 @@ interface RatePeriodRow {
<ion-label position="stacked">{{
'FLOATING_RATES.FROM_DATE' | translate
}}</ion-label>
@if (pickersReady()) {
<ion-datetime-button datetime="periodfromDate-picker"></ion-datetime-button>
}
<app-deferred-datetime-button [datetimeId]="periodFromDatePickerId($index)" />
<ion-modal [keepContentsMounted]="true">
<ng-template>
<ion-datetime
id="periodfromDate-picker"
data-testid="periodfromDate-picker"
[id]="periodFromDatePickerId($index)"
[attr.data-testid]="periodFromDatePickerId($index)"
presentation="date"
name="periodfromDate"
[name]="'periodfromDate' + $index"
[(ngModel)]="period.fromDate"
required
></ion-datetime>
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
};
Expand All @@ -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,
});
Expand Down
3 changes: 3 additions & 0 deletions src/app/shared/utils/pickers-ready.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<boolean> {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<DeferredDatetimeButtonComponent>;

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'));
});
});
Original file line number Diff line number Diff line change
@@ -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<string>('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<HTMLElement>).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()) {
<ion-datetime-button appStampDatetime></ion-datetime-button>
}
`,
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<string>();

/** See `createPickersReady` — deferred relative to this instance, not the page. */
readonly pickersReady = createPickersReady();
}
Loading