Skip to content
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.

Sign up for GitHub

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

AAE-27343 Listen to form rules changes from reactive widgets #10392

Merged
merged 5 commits into from
Nov 29, 2024
Merged

AAE-27343 Listen to form rules changes from reactive widgets #10392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
85c7049
AAE-27343 Listen to form rules changes from reactive widgets
tomgny Nov 15, 2024
1270085
remove leftover
tomgny Nov 15, 2024
fbdec84
apply interface for reactive widgets and unit test
tomgny Nov 15, 2024
96ff193
update readonly control status
tomgny Nov 19, 2024
7935d17
[ci:force][link-adf:fix/AAE-27343-listen-to-form-rules-changes-in-rea…
tomgny Nov 25, 2024
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
70 changes: 39 additions & 31 deletions lib/core/src/lib/form/components/form-field/form-field.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('FormFieldComponent', () => {
fixture.destroy();
});

it('should create default component instance', (done) => {
it('should create default component instance', () => {
const field = new FormFieldModel(form, {
type: FormFieldTypes.TEXT,
id: 'FAKE-TXT-WIDGET'
Expand All @@ -51,14 +51,29 @@ describe('FormFieldComponent', () => {
component.field = field;
fixture.detectChanges();

fixture.whenStable().then(() => {
expect(component.componentRef).toBeDefined();
expect(component.componentRef.instance instanceof TextWidgetComponent).toBeTruthy();
done();
expect(component.componentRef).toBeDefined();
expect(component.componentRef.instance instanceof TextWidgetComponent).toBeTruthy();
});

it('should call update form control state for reactive type widget on formRulesEvent change', () => {
const field = new FormFieldModel(form, {
type: FormFieldTypes.DATE,
id: 'FAKE-DATE-WIDGET'
});

component.field = field;
fixture.detectChanges();

const widgetInstance = component.componentRef.instance;
const updateFormControlState = spyOn(widgetInstance, 'updateReactiveFormControl');

widgetInstance.formService.formRulesEvent.next();
fixture.detectChanges();

expect(updateFormControlState).toHaveBeenCalled();
});

it('should create custom component instance', (done) => {
it('should create custom component instance', () => {
formRenderingService.setComponentTypeResolver(FormFieldTypes.AMOUNT, () => CheckboxWidgetComponent, true);

const field = new FormFieldModel(form, {
Expand All @@ -67,63 +82,56 @@ describe('FormFieldComponent', () => {
});

component.field = field;

fixture.detectChanges();

fixture.whenStable().then(() => {
expect(component.componentRef).toBeDefined();
expect(component.componentRef.instance instanceof CheckboxWidgetComponent).toBeTruthy();
done();
});
expect(component.componentRef).toBeDefined();
expect(component.componentRef.instance instanceof CheckboxWidgetComponent).toBeTruthy();
});

it('should require component type to be resolved', (done) => {
it('should require component type to be resolved', () => {
const field = new FormFieldModel(form, {
type: FormFieldTypes.TEXT,
id: 'FAKE-TXT-WIDGET'
});

spyOn(formRenderingService, 'resolveComponentType').and.returnValue(null);
component.field = field;

fixture.detectChanges();

fixture.whenStable().then(() => {
expect(formRenderingService.resolveComponentType).toHaveBeenCalled();
expect(component.componentRef).toBeUndefined();
done();
});
expect(formRenderingService.resolveComponentType).toHaveBeenCalled();
expect(component.componentRef).toBeUndefined();
});

it('should hide the field when it is not visible', (done) => {
it('should hide the field when it is not visible', () => {
const field = new FormFieldModel(form, {
type: FormFieldTypes.TEXT,
id: 'FAKE-TXT-WIDGET'
});

component.field = field;
component.field.isVisible = false;

fixture.detectChanges();
fixture.whenStable().then(() => {
const debugElement = fixture.nativeElement.querySelector('#field-FAKE-TXT-WIDGET-container').style;
expect(debugElement.visibility).toEqual('hidden');
expect(debugElement.display).toEqual('none');
done();
});

const debugElement = fixture.nativeElement.querySelector('#field-FAKE-TXT-WIDGET-container').style;
expect(debugElement.visibility).toEqual('hidden');
expect(debugElement.display).toEqual('none');
});

it('should show the field when it is visible', (done) => {
it('should show the field when it is visible', () => {
const field = new FormFieldModel(form, {
type: FormFieldTypes.TEXT,
id: 'FAKE-TXT-WIDGET'
});

component.field = field;

fixture.detectChanges();

fixture.whenStable().then(() => {
expect(fixture.nativeElement.querySelector('#field-FAKE-TXT-WIDGET-container').style.visibility).toEqual('visible');
expect(fixture.nativeElement.querySelector('#field-FAKE-TXT-WIDGET-container').style.display).toEqual('block');
done();
});
expect(fixture.nativeElement.querySelector('#field-FAKE-TXT-WIDGET-container').style.visibility).toEqual('visible');
expect(fixture.nativeElement.querySelector('#field-FAKE-TXT-WIDGET-container').style.display).toEqual('block');
});

it('should hide a visible element', () => {
Expand All @@ -142,7 +150,7 @@ describe('FormFieldComponent', () => {
expect(fixture.nativeElement.querySelector('#field-FAKE-TXT-WIDGET-container').style.display).toEqual('none');
});

it('[C213878] - Should fields be correctly rendered when filled with process variables', async () => {
it('[C213878] - Should fields be correctly rendered when filled with process variables', () => {
const field = new FormFieldModel(form, {
fieldType: 'HyperlinkRepresentation',
id: 'label2',
Expand Down
21 changes: 20 additions & 1 deletion lib/core/src/lib/form/components/form-field/form-field.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
Component,
ComponentFactory,
ComponentRef,
DestroyRef,
inject,
Input,
NgModule,
Expand All @@ -33,6 +34,8 @@ import { FormRenderingService } from '../../services/form-rendering.service';
import { WidgetVisibilityService } from '../../services/widget-visibility.service';
import { FormFieldModel } from '../widgets/core/form-field.model';
import { FieldStylePipe } from '../../pipes/field-style.pipe';
import { FormFieldTypes } from '../widgets/core/form-field-types';
import { takeUntilDestroyed } from '@angular/core/rxjs-interop';

declare const adf: any;

Expand Down Expand Up @@ -62,6 +65,7 @@ export class FormFieldComponent implements OnInit, OnDestroy {

private readonly formRenderingService = inject(FormRenderingService);
private readonly visibilityService = inject(WidgetVisibilityService);
private readonly destroyRef = inject(DestroyRef);
private readonly compiler = inject(Compiler);

ngOnInit() {
Expand All @@ -88,14 +92,29 @@ export class FormFieldComponent implements OnInit, OnDestroy {
instance.fieldChanged.subscribe((field) => {
if (field && this.field.form) {
this.visibilityService.refreshVisibility(field.form);
field.form.onFormFieldChanged(field);
this.triggerFormFieldChanged(field);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it worth to have a method just for a single line of code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method now is used in two places, so thats why i move it to separate method, but i can agree that maybe its not worth

}
});

if (FormFieldTypes.isReactiveType(instance?.field?.type)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was expecting this to be called when something has changed into the form field so inside the fieldChanged stream

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm why? we subscribe to the formRulesEvent when we create this component instance, why do we need to put it under another subscription (nested)? it should listen for these events for the whole lifecycle of the instance

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to subscribe to the form rules here, you just need to put your part of the code (the if) into the fieldChanged subscription, otherwise other event relying on triggering a change on the form field will fail to update the validation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we do not subscribe for formRules changes, we wont be able to update the state of particular instance until the instance.fieldChanged will be triggered (so for example after blur, or input something event).
to make sure I understand you, this is what you mean, right?

Suggested change
if (FormFieldTypes.isReactiveType(instance?.field?.type)) {
if (componentType) {
this.componentRef = this.container.createComponent(componentType);
const instance = this.componentRef.instance;
instance.field = this.field;
instance.fieldChanged.subscribe((field) => {
if (field && this.field.form) {
this.visibilityService.refreshVisibility(field.form);
this.triggerFormFieldChanged(field);
if (FormFieldTypes.isReactiveType(instance?.field?.type)) {
instance?.updateReactiveFormControl();
this.triggerFormFieldChanged(instance.field);
}
}
});

(i tested it and it doesnt work)

this.updateReactiveFormControlOnFormRulesEvent(instance);
}
}
}
}
}

private updateReactiveFormControlOnFormRulesEvent(instance: any): void {
instance?.formService.formRulesEvent.pipe(takeUntilDestroyed(this.destroyRef)).subscribe(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reducing the check to only the form rules might miss some other parts where we update the form and the field changed event is triggered

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on each onFieldChanged call, inside the base widget.component.ts , and also on the all events, we update the value of formRulesEvent subject so it will be always triggered for all the events (and its needed for all events, because in modeling, we can set rules for all available event types, like blur, click etc). i think it should be enough, but maybe im not aware of some corner case

instance?.updateReactiveFormControl();
Copy link
Contributor

@BSekula BSekula Nov 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add an interface to instance? what I mean is adding something like:

interface ReactiveFormWidget {
   updateReactiveFormControl: (): void; 
   formService: Type
}

and implement it in reactive widgets

export class DateTimeWidgetComponent extends WidgetComponent implements OnInit, ReactiveFormWidget {..

So updateReactiveFormControl could be easily traced?

Also we can add some comment to this interface, explaining what it is

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, will add it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you add an interface (or an abstract class like WidgetComponent) please double check with a custom form widget if it's all good

this.triggerFormFieldChanged(instance.field);
});
}

private triggerFormFieldChanged(field: FormFieldModel): void {
field.form.onFormFieldChanged(field);
}

ngOnDestroy() {
if (this.componentRef) {
this.componentRef.destroy();
Expand Down
21 changes: 14 additions & 7 deletions lib/core/src/lib/form/components/widgets/date-time/date-time.widget.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,14 @@ export class DateTimeWidgetComponent extends WidgetComponent implements OnInit {
maxDate: Date;
datetimeInputControl: FormControl<Date> = new FormControl<Date>(null);


public readonly formService = inject(FormService);

private readonly destroyRef = inject(DestroyRef);
private readonly dateAdapter = inject(DateAdapter);
private readonly dateTimeAdapter = inject(DatetimeAdapter);

ngOnInit(): void {
this.patchFormControl();
this.setFormControlValue();
this.updateFormControlState();
this.initDateAdapter();
this.initDateRange();
this.subscribeToDateChanges();
Expand All @@ -77,12 +76,20 @@ export class DateTimeWidgetComponent extends WidgetComponent implements OnInit {
this.onFieldChanged(this.field);
}

private patchFormControl(): void {
updateReactiveFormControl(): void {
this.updateFormControlState();
this.validateField();
}

private setFormControlValue(): void {
this.datetimeInputControl.setValue(this.field.value, { emitEvent: false });
}

private updateFormControlState(): void {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private updateFormControlState(): void {
private updateFormControlValidators(): void {

WDYT?
To me state is really broad term

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think state in this case better describe what this method does, because it updates also readonly/disabled state (required is also state of the field)

this.datetimeInputControl.setValidators(this.isRequired() ? [Validators.required] : []);
if (this.field?.readOnly || this.readOnly) {
this.datetimeInputControl.disable({ emitEvent: false });
}
this.field?.readOnly || this.readOnly
? this.datetimeInputControl.disable({ emitEvent: false })
: this.datetimeInputControl.enable({ emitEvent: false });

this.datetimeInputControl.updateValueAndValidity({ emitEvent: false });
}
Expand Down
23 changes: 17 additions & 6 deletions lib/core/src/lib/form/components/widgets/date/date.widget.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { WidgetComponent } from '../widget.component';
import { ErrorMessageModel } from '../core/error-message.model';
import { parseISO } from 'date-fns';
import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
import { ReactiveFormWidget } from '../reactive-widget.interface';

@Component({
selector: 'date-widget',
Expand All @@ -55,7 +56,7 @@ import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
imports: [MatFormFieldModule, TranslateModule, MatInputModule, MatDatepickerModule, ReactiveFormsModule, ErrorWidgetComponent, NgIf],
encapsulation: ViewEncapsulation.None
})
export class DateWidgetComponent extends WidgetComponent implements OnInit {
export class DateWidgetComponent extends WidgetComponent implements OnInit, ReactiveFormWidget {
minDate: Date;
maxDate: Date;
startAt: Date;
Expand All @@ -68,7 +69,8 @@ export class DateWidgetComponent extends WidgetComponent implements OnInit {
private readonly destroyRef = inject(DestroyRef);

ngOnInit(): void {
this.patchFormControl();
this.setFormControlValue();
this.updateFormControlState();
this.initDateAdapter();
this.initDateRange();
this.initStartAt();
Expand All @@ -80,12 +82,21 @@ export class DateWidgetComponent extends WidgetComponent implements OnInit {
this.validateField();
this.onFieldChanged(this.field);
}
private patchFormControl(): void {

updateReactiveFormControl(): void {
this.updateFormControlState();
this.validateField();
}

private setFormControlValue(): void {
this.dateInputControl.setValue(this.field.value, { emitEvent: false });
}

private updateFormControlState(): void {
this.dateInputControl.setValidators(this.isRequired() ? [Validators.required] : []);
Copy link
Contributor

@BSekula BSekula Nov 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use custom validators in date widget? 🤔 or only required

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as default option we use only required from default validators. for other cases, like min/max constraints, we handle it in the handleErrors method, and its needed to be compatible with existing form structure + other non-reactive form widgets.

if (this.field?.readOnly || this.readOnly) {
this.dateInputControl.disable({ emitEvent: false });
}
this.field?.readOnly || this.readOnly
? this.dateInputControl.disable({ emitEvent: false })
: this.dateInputControl.enable({ emitEvent: false });

this.dateInputControl.updateValueAndValidity({ emitEvent: false });
}
Expand Down
1 change: 1 addition & 0 deletions lib/core/src/lib/form/components/widgets/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { DecimalWidgetComponent } from './decimal/decimal.component';

// core
export * from './widget.component';
export * from './reactive-widget.interface';
export * from './core';

// primitives
Expand Down
23 changes: 23 additions & 0 deletions lib/core/src/lib/form/components/widgets/reactive-widget.interface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*!
* @license
* Copyright © 2005-2024 Hyland Software, Inc. and its affiliates. All rights reserved.
*
* Licensed 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 { FormService } from '../../services/form.service';

export interface ReactiveFormWidget {
updateReactiveFormControl(): void;
formService: FormService;
}
28 changes: 19 additions & 9 deletions lib/process-services-cloud/src/lib/form/components/widgets/date/date-cloud.widget.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

/* eslint-disable @angular-eslint/component-selector */

import { Component, DestroyRef, inject, OnInit, ViewEncapsulation } from '@angular/core';
import { Component, OnInit, ViewEncapsulation, DestroyRef, inject } from '@angular/core';
import { DateAdapter, MAT_DATE_FORMATS } from '@angular/material/core';
import {
ADF_DATE_FORMATS,
Expand All @@ -27,7 +27,8 @@ import {
ErrorMessageModel,
ErrorWidgetComponent,
FormService,
WidgetComponent
WidgetComponent,
ReactiveFormWidget
} from '@alfresco/adf-core';
import { MatDatepickerModule } from '@angular/material/datepicker';
import { addDays, parseISO } from 'date-fns';
Expand Down Expand Up @@ -61,7 +62,7 @@ import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
},
encapsulation: ViewEncapsulation.None
})
export class DateCloudWidgetComponent extends WidgetComponent implements OnInit {
export class DateCloudWidgetComponent extends WidgetComponent implements OnInit, ReactiveFormWidget {
typeId = 'DateCloudWidgetComponent';

minDate: Date = null;
Expand All @@ -71,12 +72,13 @@ export class DateCloudWidgetComponent extends WidgetComponent implements OnInit
dateInputControl: FormControl<Date> = new FormControl<Date>(null);

public readonly formService = inject(FormService);

private readonly destroyRef = inject(DestroyRef);
private readonly dateAdapter = inject(DateAdapter);

ngOnInit(): void {
this.patchFormControl();
this.setFormControlValue();
this.updateFormControlState();
this.initDateAdapter();
this.initRangeSelection();
this.initStartAt();
Expand All @@ -89,12 +91,20 @@ export class DateCloudWidgetComponent extends WidgetComponent implements OnInit
this.onFieldChanged(this.field);
}

private patchFormControl(): void {
updateReactiveFormControl(): void {
this.updateFormControlState();
this.validateField();
}

private setFormControlValue(): void {
this.dateInputControl.setValue(this.field.value, { emitEvent: false });
}

private updateFormControlState(): void {
this.dateInputControl.setValidators(this.isRequired() ? [Validators.required] : []);
if (this.field?.readOnly || this.readOnly) {
this.dateInputControl.disable({ emitEvent: false });
}
this.field?.readOnly || this.readOnly
? this.dateInputControl.disable({ emitEvent: false })
: this.dateInputControl.enable({ emitEvent: false });

this.dateInputControl.updateValueAndValidity({ emitEvent: false });
}
Expand Down
Loading
Loading