From 3f8e75b1520a393795bd9d0769e55579a5ab557b Mon Sep 17 00:00:00 2001 From: Nicolas Molina Monroy Date: Mon, 7 Oct 2024 13:31:51 -0400 Subject: [PATCH] chore(dotcms-ui): Fix error with autocomplete #30249 (#30269) ### Parent Issue #30249 ### Proposed Changes * Change singal approach ### The problem This computed signal caused the error: ```ts $filteredSuggestions = computed(() => { const classes = this.$classes(); const query = this.$query(); if (!query) { return classes; } return classes.filter((item) => item.includes(query)); }); filterClasses({ query }: AutoCompleteCompleteEvent): void { this.$query.set(query); } ``` The signals functionality in PrimeNg doesn't works very well. In the previous signal, `$filteredSuggestions` doesn't trigger a change if the classes or query are the same as the current state. This is desirable. However, PrimeNg [waits](https://github.com/primefaces/primeng/blob/d6c7ffcfc5cb9a8ed355399ddae276c279836ccf/src/app/components/autocomplete/autocomplete.ts#L541) for a change in the `suggestions` input after the `completeMethod` is called to turn off the loading. This is why the loading indicator remains active but never deactivates. To address this, we manually forced a change in `$filteredSuggestions` and used `[...filteredClasses]` to create a new array. ```ts filterClasses({ query }: AutoCompleteCompleteEvent): void { const classes = this.$classes(); const filteredClasses = query ? classes.filter((item) => item.includes(query)) : classes; this.$filteredSuggestions.set([...filteredClasses]); } ``` ### Checklist - [x] Tests - [x] Translations - [x] Security Implications Contemplated (add notes if applicable) ### Screenshots https://github.com/user-attachments/assets/5a199569-9192-4acb-89bd-3fb6530aefce --- .../add-style-classes-dialog.component.html | 12 +-- .../add-style-classes-dialog.component.scss | 6 +- ...add-style-classes-dialog.component.spec.ts | 98 +------------------ ...-style-classes-dialog.component.stories.ts | 15 ++- .../add-style-classes-dialog.component.ts | 81 +++------------ .../services/json-classes.service.spec.ts | 57 ++++++----- .../services/json-classes.service.ts | 15 ++- 7 files changed, 78 insertions(+), 206 deletions(-) diff --git a/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.html b/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.html index 71197946830b..6c206287a6d6 100644 --- a/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.html +++ b/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.html @@ -1,20 +1,20 @@
- @let isJsonClasses = $isJsonClasses(); + @let hasClasses = $hasClasses(); - @if (isJsonClasses) { + appendTo="body" + dotSelectItem /> + @if (hasClasses) {
  • diff --git a/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.scss b/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.scss index 355dc8a68a91..66bc34519ecb 100644 --- a/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.scss +++ b/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.scss @@ -10,9 +10,13 @@ justify-content: flex-end; } -p-autoComplete { +:host ::ng-deep p-autocomplete { margin-bottom: $spacing-3; display: block; + .p-autocomplete-dropdown.p-element.p-button { + border-top-left-radius: 0; + border-bottom-left-radius: 0; + } } ul { diff --git a/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.spec.ts b/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.spec.ts index 65628febe476..1ffd80d0cf0c 100644 --- a/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.spec.ts +++ b/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.spec.ts @@ -1,7 +1,7 @@ import { expect, it } from '@jest/globals'; import { createFakeEvent } from '@ngneat/spectator'; import { Spectator, byTestId, createComponentFactory, mockProvider } from '@ngneat/spectator/jest'; -import { of, throwError } from 'rxjs'; +import { of } from 'rxjs'; import { provideHttpClient } from '@angular/common/http'; import { FormsModule } from '@angular/forms'; @@ -68,7 +68,7 @@ describe('AddStyleClassesDialogComponent', () => { } }), mockProvider(JsonClassesService, { - getClasses: jest.fn().mockReturnValue(of({ classes: ['class1', 'class2'] })) + getClasses: jest.fn().mockReturnValue(of(['class1', 'class2'])) }) ] }); @@ -84,7 +84,6 @@ describe('AddStyleClassesDialogComponent', () => { expect(autocomplete.unique).toBe(true); expect(autocomplete.autofocus).toBe(true); expect(autocomplete.multiple).toBe(true); - expect(autocomplete.size).toBe(446); expect(autocomplete.inputId).toBe('auto-complete-input'); expect(autocomplete.appendTo).toBe('body'); expect(autocomplete.dropdown).toBe(true); @@ -170,7 +169,7 @@ describe('AddStyleClassesDialogComponent', () => { provide: JsonClassesService, useValue: { getClasses() { - return of({ classes: [] }); + return of([]); } } } @@ -200,95 +199,4 @@ describe('AddStyleClassesDialogComponent', () => { expect(list.textContent).toContain('no suggestions setup suggestions'); }); }); - - describe('bad format json', () => { - beforeEach(() => { - spectator = createComponent({ - providers: [ - ...providers, - { - provide: DynamicDialogConfig, - useValue: { - data: { - selectedClasses: [] - } - } - }, - { - provide: JsonClassesService, - useValue: { - getClasses() { - return of({ badFormat: ['class1'] }); - } - } - } - ] - }); - - jsonClassesService = spectator.inject(JsonClassesService, true); - dialogRef = spectator.inject(DynamicDialogRef); - autocomplete = spectator.query(AutoComplete); - }); - - it('should set dropdown to false in autocomplete', () => { - spectator.detectChanges(); - expect(autocomplete.dropdown).toBe(false); - }); - - it('should set component.classes empty', () => { - spectator.detectChanges(); - - expect(spectator.component.$classes()).toEqual([]); - }); - - it('should have multiples help message', () => { - spectator.detectChanges(); - const list = spectator.query(byTestId('list')); - - expect(list.textContent).toContain('no suggestions setup suggestions'); - }); - }); - - describe('error', () => { - beforeEach(() => { - spectator = createComponent({ - providers: [ - ...providers, - { - provide: DynamicDialogConfig, - useValue: { - data: { - selectedClasses: [] - } - } - }, - { - provide: JsonClassesService, - useValue: { - getClasses() { - return throwError( - new Error('An error occurred while fetching classes') - ); - } - } - } - ] - }); - - jsonClassesService = spectator.inject(JsonClassesService, true); - dialogRef = spectator.inject(DynamicDialogRef); - autocomplete = spectator.query(AutoComplete); - }); - - it('should set dropdown to false in autocomplete', () => { - spectator.detectChanges(); - expect(autocomplete.dropdown).toBe(false); - }); - - it('should set component.classes empty', () => { - spectator.detectChanges(); - - expect(spectator.component.$classes()).toEqual([]); - }); - }); }); diff --git a/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.stories.ts b/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.stories.ts index fffbba37090c..b0d9d66661d7 100644 --- a/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.stories.ts +++ b/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.stories.ts @@ -1,7 +1,7 @@ import { Meta, moduleMetadata, StoryObj, applicationConfig } from '@storybook/angular'; import { of } from 'rxjs'; -import { HttpClient, provideHttpClient } from '@angular/common/http'; +import { provideHttpClient } from '@angular/common/http'; import { FormsModule } from '@angular/forms'; import { BrowserAnimationsModule } from '@angular/platform-browser/animations'; @@ -46,18 +46,17 @@ const meta: Meta = { } } }, - { - provide: HttpClient, - useValue: { - get: (_: string) => of(MOCK_STYLE_CLASSES_FILE) - } - }, { provide: DotMessageService, useValue: DOT_MESSAGE_SERVICE_TB_MOCK }, DynamicDialogRef, - JsonClassesService + { + provide: JsonClassesService, + useValue: { + getClasses: () => of(MOCK_STYLE_CLASSES_FILE.classes) + } + } ] }) ] diff --git a/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.ts b/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.ts index 9a32bed31884..c7fd107db220 100644 --- a/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.ts +++ b/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/add-style-classes-dialog.component.ts @@ -1,5 +1,3 @@ -import { of } from 'rxjs'; - import { ChangeDetectionStrategy, Component, @@ -11,12 +9,10 @@ import { import { toSignal } from '@angular/core/rxjs-interop'; import { FormsModule } from '@angular/forms'; -import { AutoCompleteModule } from 'primeng/autocomplete'; +import { AutoCompleteCompleteEvent, AutoCompleteModule } from 'primeng/autocomplete'; import { ButtonModule } from 'primeng/button'; import { DynamicDialogConfig, DynamicDialogRef } from 'primeng/dynamicdialog'; -import { catchError, map, shareReplay, tap } from 'rxjs/operators'; - import { DotMessagePipe, DotSelectItemDirective } from '@dotcms/ui'; import { JsonClassesService } from './services/json-classes.service'; @@ -49,73 +45,28 @@ export class AddStyleClassesDialogComponent implements OnInit { * @memberof AddStyleClassesDialogComponent */ readonly #dialogRef = inject(DynamicDialogRef); + readonly #dynamicDialogConfig = inject(DynamicDialogConfig<{ selectedClasses: string[] }>); /** * Selected classes to be added * @memberof AddStyleClassesDialogComponent */ $selectedClasses = signal([]); /** - * Classes to be displayed - * - * @memberof AddStyleClassesDialogComponent - */ - $classes = signal([]); - /** - * Query to filter the classes - * - * @memberof AddStyleClassesDialogComponent - */ - $query = signal(null); - /** - * Filtered suggestions based on the query + * Check if the JSON file has classes * * @memberof AddStyleClassesDialogComponent */ - $filteredSuggestions = computed(() => { - const classes = this.$classes(); - const query = this.$query(); - - if (!query) { - return classes; - } - - return classes.filter((item) => item.includes(query)); + $classes = toSignal(this.#jsonClassesService.getClasses(), { + initialValue: [] }); /** - * Check if there are classes in the JSON file - * - * @memberof AddStyleClassesDialogComponent - */ - isJsonClasses$ = this.#jsonClassesService.getClasses().pipe( - tap(({ classes }) => { - if (classes?.length) { - this.$classes.set(classes); - } else { - this.$classes.set([]); - } - }), - map(({ classes }) => !!classes?.length), - catchError(() => { - this.$classes.set([]); - - return of(false); - }), - shareReplay(1) - ); - /** - * Check if the JSON file has classes + * Filtered suggestions based on the query * * @memberof AddStyleClassesDialogComponent */ - $isJsonClasses = toSignal(this.isJsonClasses$, { - initialValue: false - }); + $filteredSuggestions = signal(this.$classes()); - constructor( - public dynamicDialogConfig: DynamicDialogConfig<{ - selectedClasses: string[]; - }> - ) {} + $hasClasses = computed(() => this.$classes().length > 0); /** * Set the selected classes @@ -123,10 +74,7 @@ export class AddStyleClassesDialogComponent implements OnInit { * @memberof AddStyleClassesDialogComponent */ ngOnInit() { - const data = this.dynamicDialogConfig.data; - if (data) { - this.$selectedClasses.set(data.selectedClasses); - } + this.$selectedClasses.set(this.#dynamicDialogConfig?.data?.selectedClasses || []); } /** @@ -136,15 +84,14 @@ export class AddStyleClassesDialogComponent implements OnInit { * @return {*} * @memberof AddStyleClassesDialogComponent */ - filterClasses({ query }: { query: string }): void { + filterClasses({ query }: AutoCompleteCompleteEvent): void { /* - https://github.com/primefaces/primeng/blob/master/src/app/components/autocomplete/autocomplete.ts#L739 - + https://github.com/primefaces/primeng/blob/master/src/app/components/autocomplete/autocomplete.ts#L541 Sadly we need to pass suggestions all the time, even if they are empty because on the set is where the primeng remove the loading icon */ - - // PrimeNG autocomplete doesn't support async pipe in the suggestions - this.$query.set(query); + const classes = this.$classes(); + const filteredClasses = query ? classes.filter((item) => item.includes(query)) : classes; + this.$filteredSuggestions.set([...filteredClasses]); } /** diff --git a/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/services/json-classes.service.spec.ts b/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/services/json-classes.service.spec.ts index 7f9b685d157f..4ab5c1d64e38 100644 --- a/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/services/json-classes.service.spec.ts +++ b/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/services/json-classes.service.spec.ts @@ -1,38 +1,47 @@ -import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing'; -import { TestBed, getTestBed } from '@angular/core/testing'; +import { createHttpFactory, HttpMethod, SpectatorHttp } from '@ngneat/spectator'; import { JsonClassesService, STYLE_CLASSES_FILE_URL } from './json-classes.service'; +import { MOCK_STYLE_CLASSES_FILE } from '../../../utils/mocks'; + describe('JsonClassesService', () => { - let injector: TestBed; - let service: JsonClassesService; - let httpMock: HttpTestingController; - - beforeEach(() => { - TestBed.configureTestingModule({ - imports: [HttpClientTestingModule], - providers: [JsonClassesService] - }); + let spectator: SpectatorHttp; + const createHttp = createHttpFactory(JsonClassesService); + + beforeEach(() => (spectator = createHttp())); - injector = getTestBed(); - service = injector.inject(JsonClassesService); - httpMock = injector.inject(HttpTestingController); + it('should be requested to the expected URL', () => { + spectator.service.getClasses().subscribe(); + spectator.expectOne(STYLE_CLASSES_FILE_URL, HttpMethod.GET); }); - afterEach(() => { - httpMock.verify(); + it('should return all classes', () => { + spectator.service.getClasses().subscribe((res) => { + expect(res).toEqual(MOCK_STYLE_CLASSES_FILE.classes); + }); + + const req = spectator.expectOne(STYLE_CLASSES_FILE_URL, HttpMethod.GET); + + req.flush(MOCK_STYLE_CLASSES_FILE); }); - it('should return an Observable with classes', () => { - const expectedClasses = { classes: ['class1', 'class2', 'class3'] }; - // httpClientSpy.get.and.returnValue(of(expectedClasses)); + it('should return an empty array with a error', () => { + spectator.service.getClasses().subscribe((res) => { + expect(res).toEqual([]); + }); + + const req = spectator.expectOne(STYLE_CLASSES_FILE_URL, HttpMethod.GET); - service.getClasses().subscribe((classes) => { - expect(classes).toEqual(expectedClasses); + req.flush('', { status: 404, statusText: 'Not Found' }); + }); + + it('should return an empty array with a bad format', () => { + spectator.service.getClasses().subscribe((res) => { + expect(res).toEqual([]); }); - const req = httpMock.expectOne(STYLE_CLASSES_FILE_URL); - expect(req.request.method).toBe('GET'); - req.flush(expectedClasses); + const req = spectator.expectOne(STYLE_CLASSES_FILE_URL, HttpMethod.GET); + + req.flush({ bad: 'format' }); }); }); diff --git a/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/services/json-classes.service.ts b/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/services/json-classes.service.ts index eb0323d271f5..47808b6e8354 100644 --- a/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/services/json-classes.service.ts +++ b/core-web/libs/template-builder/src/lib/components/template-builder/components/add-style-classes-dialog/services/json-classes.service.ts @@ -1,15 +1,20 @@ -import { Observable } from 'rxjs'; +import { Observable, of } from 'rxjs'; import { HttpClient } from '@angular/common/http'; -import { Injectable } from '@angular/core'; +import { inject, Injectable } from '@angular/core'; + +import { catchError, map } from 'rxjs/operators'; export const STYLE_CLASSES_FILE_URL = '/application/templates/classes.json'; @Injectable() export class JsonClassesService { - constructor(private http: HttpClient) {} + readonly #http = inject(HttpClient); - getClasses(): Observable<{ classes: string[] }> { - return this.http.get<{ classes: string[] }>(STYLE_CLASSES_FILE_URL); + getClasses(): Observable { + return this.#http.get<{ classes: string[] }>(STYLE_CLASSES_FILE_URL).pipe( + map((res) => res?.classes || []), + catchError(() => of([])) + ); } }