AAE-23521 Fix improve dropdown reactive form #10040

kathrine0 · 2024-08-05T09:22:57Z

Please check if the PR fulfills these requirements

The commit message follows our guidelines

Tests for the changes have been added (for bug fixes / features)

Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (check one with "x")

Bugfix

Feature

Code style update (formatting, local variables)

Refactoring (no functional changes, no api changes)

Build related changes

Documentation

Other... Please describe:

What is the current behaviour? (You can also link to an open issue here)

AAE-23521

What is the new behaviour?

Does this PR introduce a breaking change? (check one with "x")

Yes

No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

eromano · 2024-08-05T09:28:02Z

does the https://github.com/Alfresco/alfresco-ng2-components/tree/develop/lib/process-services/src/lib/form/widgets/dropdown needs any change with this modify?

tomgny

added some notes and small suggestions

lib/process-services-cloud/src/lib/form/components/widgets/dropdown/dropdown-cloud.widget.html

tomgny · 2024-08-05T09:42:44Z

lib/process-services-cloud/src/lib/form/components/widgets/dropdown/dropdown-cloud.widget.html

-            *ngIf="isRequired()">*</span>
+<div
+    class="adf-dropdown-widget {{field.className}}"
+    [class.adf-invalid]="dropdownControl.invalid && dropdownControl.touched"


maybe we can also replate it by getter?
something like

Suggested change

[class.adf-invalid]="dropdownControl.invalid && dropdownControl.touched"

get isDropdownInvalidAndTouched(): boolean {

return this.dropdownControl.invalid && this.dropdownControl.touched;

}

I think it's not needed here - no additional code

tomgny · 2024-08-05T09:45:13Z

lib/process-services-cloud/src/lib/form/components/widgets/dropdown/dropdown-cloud.widget.ts

+        MatSelectModule,
+        ErrorWidgetComponent,
+        TranslateModule,
+        CardViewModule // imported for adf-select-filter-input


i think this comment is not needed

I left it here because I hope that soon we will modify the adf-select-filter-input to be standalone and then this import can be removed

lib/process-services-cloud/src/lib/form/components/widgets/dropdown/dropdown-cloud.widget.ts

tomgny · 2024-08-05T10:09:25Z

lib/process-services-cloud/src/lib/form/components/widgets/dropdown/dropdown-cloud.widget.ts

+            this.dropdownControl.disable({ emitEvent: false });
+        }
+
+        this.dropdownControl.valueChanges


maybe we can move this subscriptions to separate method? what do you think?

tomgny · 2024-08-05T10:21:25Z

lib/process-services-cloud/src/lib/form/components/widgets/dropdown/dropdown-cloud.widget.html

+                class="adf-dropdown-failed-message"
+                *ngIf="variableOptionsFailed"
+                required="{{ 'FORM.FIELD.VARIABLE_DROPDOWN_OPTIONS_FAILED' | translate }}"
+            ></error-widget>


i think we can display errors using only one error-widget (by assign errors to the validationSummary property), but its something optional, in the future i believe we will replace this by mat-error (we have ticket for it)
so its just a suggestion

tomgny · 2024-08-05T10:24:51Z

lib/core/src/lib/form/components/widgets/core/form-field-validator.ts

@@ -51,22 +50,6 @@ export class RequiredFieldValidator implements FormFieldValidator {

    validate(field: FormFieldModel): boolean {
        if (this.isSupported(field) && field.isVisible) {
-            if (field.type === FormFieldTypes.DROPDOWN) {
-                if (field.hasMultipleValues) {


just a note, please make sure we still have a correct behaviour of form validity as a whole, like disable 'Complete' button where field.hasMultipleValues and Array.isArray(field.value) && !!field.value.length return false

lib/process-services-cloud/src/lib/form/components/widgets/dropdown/dropdown-cloud.widget.ts

kathrine0 · 2024-08-05T13:06:29Z

does the https://github.com/Alfresco/alfresco-ng2-components/tree/develop/lib/process-services/src/lib/form/widgets/dropdown needs any change with this modify?

That's right, I modified the dropdown widget as well

tomgny

good job

tomgny · 2024-08-05T16:00:00Z

lib/process-services/src/lib/form/widgets/dropdown/dropdown.widget.ts

    }

-    isReadOnlyType(): boolean {
-        return this.field.type === 'readonly';
+    private initFormControl() {


Suggested change

private initFormControl() {

private initFormControl(): void {

tomgny · 2024-08-05T16:00:08Z

lib/process-services/src/lib/form/widgets/dropdown/dropdown.widget.ts

    }

-    showRequiredMessage(): boolean {
-        return (this.isInvalidFieldRequired() || this.field.value === 'empty') && this.isTouched();
+    private handleErrors() {


Suggested change

private handleErrors() {

private handleErrors(): void {

tomgny · 2024-08-05T16:00:26Z

lib/process-services/src/lib/form/widgets/dropdown/dropdown.widget.ts

    }

-    get isReadOnlyField(): boolean {
-        return this.field.readOnly;
+    private setOptionValue(option: string | FormFieldOption, field: FormFieldModel) {


Suggested change

private setOptionValue(option: string | FormFieldOption, field: FormFieldModel) {

private setOptionValue(option: string | FormFieldOption, field: FormFieldModel): void {

tomgny · 2024-08-05T16:00:56Z

lib/process-services/src/lib/form/widgets/dropdown/dropdown.widget.ts

    }

-    private isReadOnlyForm(): boolean {
-        return !!this.field?.form?.readOnly;
+    private getOptionValue(value?: string | FormFieldOption) {


Suggested change

private getOptionValue(value?: string | FormFieldOption) {

private getOptionValue(value?: string | FormFieldOption): FormFieldOption | undefined {

tomgny · 2024-08-05T16:01:16Z

lib/process-services/src/lib/form/widgets/dropdown/dropdown.widget.html

-            </mat-option>
-            <mat-option id="readonlyOption" *ngIf="isReadOnlyType()" [value]="field.value">{{field.value}}</mat-option>
+        <mat-select class="adf-select" [id]="field.id" [formControl]="dropdownControl">
+            <mat-option *ngFor="let opt of field.options" [value]="opt" [id]="opt.id">{{opt.name}} </mat-option>


Suggested change

<mat-option *ngFor="let opt of field.options" [value]="opt" [id]="opt.id">{{opt.name}} </mat-option>

<mat-option *ngFor="let opt of field.options" [value]="opt" [id]="opt.id">{{opt.name}}</mat-option>

:D

DenysVuika · 2024-08-05T16:19:11Z

lib/core/src/lib/form/components/widgets/error/error.component.scss

@@ -1,5 +1,6 @@
 .adf-error {
    display: flex;
+    align-items: center;


is this expected? the error under each field should be centered, not left-aligned?

…mponents into fix/AAE-23521-improve-dropdown-reactive-form

sonarqubecloud · 2024-08-06T13:53:32Z

Quality Gate failed

Failed conditions
3.3% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

This reverts commit f88ff10.

* Revert "AAE-23521 Fix improve dropdown reactive form (#10040)" This reverts commit f88ff10. * Revert "AAE-24371 Fix invalid endpoint is called when process with form is st… (#10052)" This reverts commit d38e782. * Revert "Revert "AAE-24371 Fix invalid endpoint is called when process with form is st… (#10052)"" This reverts commit 125ada7.

AAE-23521 Fix improve dropdown reactive form

1fb680e

kathrine0 requested review from eromano, popovicsandras and DenysVuika as code owners August 5, 2024 09:22

kathrine0 requested review from mauriziovitale, tomgny and wiktord2000 August 5, 2024 09:23

mauriziovitale approved these changes Aug 5, 2024

View reviewed changes

tomgny reviewed Aug 5, 2024

View reviewed changes

AAE-23521 Update process services dropdown

db02251

kathrine0 added 2 commits August 5, 2024 15:10

AAE-23521 pr suggestions

4d7513a

AAE-23521 move tests

cd085a2

tomgny approved these changes Aug 5, 2024

View reviewed changes

DenysVuika reviewed Aug 5, 2024

View reviewed changes

kathrine0 and others added 5 commits August 6, 2024 08:28

AAE-23521 fixes

03be487

Merge branch 'develop' into fix/AAE-23521-improve-dropdown-reactive-form

24dc67c

Merge branch 'develop' into fix/AAE-23521-improve-dropdown-reactive-form

cba0f5d

Merge branch 'develop' into fix/AAE-23521-improve-dropdown-reactive-form

e60c0a1

Merge branch 'develop' of https://github.com/Alfresco/alfresco-ng2-co…

03fcf09

…mponents into fix/AAE-23521-improve-dropdown-reactive-form

kathrine0 merged commit f88ff10 into develop Aug 7, 2024
32 of 33 checks passed

kathrine0 deleted the fix/AAE-23521-improve-dropdown-reactive-form branch August 7, 2024 13:21

VitoAlbano added a commit that referenced this pull request Aug 9, 2024

Revert "AAE-23521 Fix improve dropdown reactive form (#10040)"

2a60f73

This reverts commit f88ff10.

VitoAlbano mentioned this pull request Aug 9, 2024

Revert "AAE-23521 Fix improve dropdown reactive form" #10068

Merged

ehsan-2019 pushed a commit that referenced this pull request Aug 9, 2024

Revert "AAE-23521 Fix improve dropdown reactive form (#10040)"

03fcfed

This reverts commit f88ff10.

VitoAlbano added a commit that referenced this pull request Aug 9, 2024

Revert "AAE-23521 Fix improve dropdown reactive form (#10040)"

8dfc458

This reverts commit f88ff10.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AAE-23521 Fix improve dropdown reactive form #10040

AAE-23521 Fix improve dropdown reactive form #10040

kathrine0 commented Aug 5, 2024

eromano commented Aug 5, 2024

tomgny left a comment

tomgny Aug 5, 2024

kathrine0 Aug 5, 2024

tomgny Aug 5, 2024

kathrine0 Aug 5, 2024 •

edited

Loading

tomgny Aug 5, 2024

tomgny Aug 5, 2024

tomgny Aug 5, 2024

kathrine0 commented Aug 5, 2024

tomgny left a comment

tomgny Aug 5, 2024

tomgny Aug 5, 2024

tomgny Aug 5, 2024

tomgny Aug 5, 2024

tomgny Aug 5, 2024

DenysVuika Aug 5, 2024

sonarqubecloud bot commented Aug 6, 2024

-    [class.adf-invalid]="dropdownControl.invalid && dropdownControl.touched"
+    get isDropdownInvalidAndTouched(): boolean {
+        return this.dropdownControl.invalid && this.dropdownControl.touched;
+   }

	private initFormControl() {
	private initFormControl(): void {

	private setOptionValue(option: string \| FormFieldOption, field: FormFieldModel) {
	private setOptionValue(option: string \| FormFieldOption, field: FormFieldModel): void {

	private getOptionValue(value?: string \| FormFieldOption) {
	private getOptionValue(value?: string \| FormFieldOption): FormFieldOption \| undefined {

	<mat-option *ngFor="let opt of field.options" [value]="opt" [id]="opt.id">{{opt.name}} </mat-option>
	<mat-option *ngFor="let opt of field.options" [value]="opt" [id]="opt.id">{{opt.name}}</mat-option>

AAE-23521 Fix improve dropdown reactive form #10040

AAE-23521 Fix improve dropdown reactive form #10040

Conversation

kathrine0 commented Aug 5, 2024

eromano commented Aug 5, 2024

tomgny left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kathrine0 Aug 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kathrine0 commented Aug 5, 2024

tomgny left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Aug 6, 2024

Quality Gate failed

kathrine0 Aug 5, 2024 •

edited

Loading