From a12674441821a4260a6fc84fb8b31cc0c8900106 Mon Sep 17 00:00:00 2001 From: Adrien Crivelli Date: Sun, 19 Feb 2023 12:28:48 +0700 Subject: [PATCH 01/10] NaturalSelectHierarchic supports async configuration If the given configuration is null, an empty array or an array without any element with `selectableAtKey`, then the hierarchic dialog will never open at all. --- .../select-hierarchic.component.spec.ts | 49 ++++++++++++++++++- .../select-hierarchic.component.ts | 20 ++++---- 2 files changed, 57 insertions(+), 12 deletions(-) diff --git a/projects/natural/src/lib/modules/select/select-hierarchic/select-hierarchic.component.spec.ts b/projects/natural/src/lib/modules/select/select-hierarchic/select-hierarchic.component.spec.ts index 2881a315..cc7c5b00 100644 --- a/projects/natural/src/lib/modules/select/select-hierarchic/select-hierarchic.component.spec.ts +++ b/projects/natural/src/lib/modules/select/select-hierarchic/select-hierarchic.component.spec.ts @@ -127,7 +127,7 @@ function testSelectHierarchicBehavior(data: TestFixture { + const hierarchicSelectorDialogService = TestBed.inject(NaturalHierarchicSelectorDialogService); + + const spy = spyOn(hierarchicSelectorDialogService, 'open'); + + data.selectComponent.config = null; + data.selectComponent.selectLabel = 'test select label'; + + // Trigger the selection of item in mocked dialog + data.selectComponent.openDialog(); + + expect(spy).not.toHaveBeenCalled(); + expect(data.selectComponent.showSelectButton()).toBeFalse(); + })); + + it('should never open with empty array config', fakeAsync(() => { + const hierarchicSelectorDialogService = TestBed.inject(NaturalHierarchicSelectorDialogService); + + const spy = spyOn(hierarchicSelectorDialogService, 'open'); + + data.selectComponent.config = []; + data.selectComponent.selectLabel = 'test select label'; + + // Trigger the selection of item in mocked dialog + data.selectComponent.openDialog(); + + expect(spy).not.toHaveBeenCalled(); + expect(data.selectComponent.showSelectButton()).toBeFalse(); + })); + + it('should never open with non-empty array but without `selectableAtKey` config', fakeAsync(() => { + const hierarchicSelectorDialogService = TestBed.inject(NaturalHierarchicSelectorDialogService); + + const spy = spyOn(hierarchicSelectorDialogService, 'open'); + + data.selectComponent.config = [{service: ItemService}]; + data.selectComponent.selectLabel = 'test select label'; + + // Trigger the selection of item in mocked dialog + data.selectComponent.openDialog(); + + expect(spy).not.toHaveBeenCalled(); + expect(data.selectComponent.showSelectButton()).toBeFalse(); + })); } diff --git a/projects/natural/src/lib/modules/select/select-hierarchic/select-hierarchic.component.ts b/projects/natural/src/lib/modules/select/select-hierarchic/select-hierarchic.component.ts index 0d111dcc..2d64f32f 100644 --- a/projects/natural/src/lib/modules/select/select-hierarchic/select-hierarchic.component.ts +++ b/projects/natural/src/lib/modules/select/select-hierarchic/select-hierarchic.component.ts @@ -48,8 +48,10 @@ export class NaturalSelectHierarchicComponent /** * Configuration for hierarchic relations + * + * It should be an array with at least one element with `selectableAtKey` configured, otherwise the selector will never open. */ - @Input() public config!: NaturalHierarchicConfiguration[]; + @Input() public config: NaturalHierarchicConfiguration[] | null = null; /** * Filters formatted for hierarchic selector @@ -109,6 +111,10 @@ export class NaturalSelectHierarchicComponent } const selectAtKey = this.getSelectKey(); + if (!selectAtKey || !this.config) { + return; + } + const selected: OrganizedModelSelection = {}; if (this.internalCtrl.value) { @@ -142,16 +148,10 @@ export class NaturalSelectHierarchicComponent } public showSelectButton(): boolean { - return !!(this.internalCtrl?.enabled && this.selectLabel && this.config); + return !!(this.internalCtrl?.enabled && this.selectLabel && this.getSelectKey()); } - private getSelectKey(): string { - const selectKey = this.config.filter(c => !!c.selectableAtKey)[0].selectableAtKey; - - if (!selectKey) { - throw new Error('Hierarchic selector must be configured with at least one selectableAtKey'); - } - - return selectKey; + private getSelectKey(): string | undefined { + return this.config?.filter(c => !!c.selectableAtKey)[0]?.selectableAtKey; } } From ee5730130c049a2f0cddae2147123fe56cac33a9 Mon Sep 17 00:00:00 2001 From: Adrien Crivelli Date: Sun, 19 Feb 2023 15:16:29 +0700 Subject: [PATCH 02/10] NaturalSelectHierarchic can be used again when config becomes valid #9423 --- .../select-hierarchic.component.spec.ts | 21 ++++++++++++++++++- .../select-hierarchic.component.ts | 10 ++++----- .../src/lib/modules/select/testing/utils.ts | 18 +++++++++------- 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/projects/natural/src/lib/modules/select/select-hierarchic/select-hierarchic.component.spec.ts b/projects/natural/src/lib/modules/select/select-hierarchic/select-hierarchic.component.spec.ts index cc7c5b00..bb2c24d6 100644 --- a/projects/natural/src/lib/modules/select/select-hierarchic/select-hierarchic.component.spec.ts +++ b/projects/natural/src/lib/modules/select/select-hierarchic/select-hierarchic.component.spec.ts @@ -16,6 +16,7 @@ import {MockApolloProvider} from '../../../testing/mock-apollo.provider'; import { AbstractTestHostWithFormControlComponent, AbstractTestHostWithNgModelComponent, + itemHierarchicConfig, TestFixture, testSelectAndSelectHierarchicCommonBehavior, } from '../testing/utils'; @@ -165,7 +166,9 @@ function testSelectHierarchicBehavior(data: TestFixture { const hierarchicSelectorDialogService = TestBed.inject(NaturalHierarchicSelectorDialogService); - const spy = spyOn(hierarchicSelectorDialogService, 'open'); + const spy = spyOn(hierarchicSelectorDialogService, 'open').and.callFake(hierarchicConfig => + mockDialogRef(hierarchicConfig.hierarchicSelection), + ); data.selectComponent.config = null; data.selectComponent.selectLabel = 'test select label'; @@ -175,6 +178,22 @@ function testSelectHierarchicBehavior(data: TestFixture { diff --git a/projects/natural/src/lib/modules/select/select-hierarchic/select-hierarchic.component.ts b/projects/natural/src/lib/modules/select/select-hierarchic/select-hierarchic.component.ts index 2d64f32f..08129ff4 100644 --- a/projects/natural/src/lib/modules/select/select-hierarchic/select-hierarchic.component.ts +++ b/projects/natural/src/lib/modules/select/select-hierarchic/select-hierarchic.component.ts @@ -104,17 +104,17 @@ export class NaturalSelectHierarchicComponent return; } + const selectAtKey = this.getSelectKey(); + if (!selectAtKey || !this.config) { + return; + } + this.lockOpenDialog = true; if (this.onTouched) { this.onTouched(); } - const selectAtKey = this.getSelectKey(); - if (!selectAtKey || !this.config) { - return; - } - const selected: OrganizedModelSelection = {}; if (this.internalCtrl.value) { diff --git a/projects/natural/src/lib/modules/select/testing/utils.ts b/projects/natural/src/lib/modules/select/testing/utils.ts index 9980c732..03e2c289 100644 --- a/projects/natural/src/lib/modules/select/testing/utils.ts +++ b/projects/natural/src/lib/modules/select/testing/utils.ts @@ -6,6 +6,15 @@ import {ItemService} from '../../../testing/item.service'; import {FormControl, Validators} from '@angular/forms'; import {AbstractSelect} from '../abstract-select.component'; +export const itemHierarchicConfig: NaturalHierarchicConfiguration[] = [ + { + service: ItemService, + parentsRelationNames: ['parent'], + childrenRelationNames: ['parent'], + selectableAtKey: 'any', + }, +]; + /** * Base for test host */ @@ -13,14 +22,7 @@ import {AbstractSelect} from '../abstract-select.component'; abstract class TestHostComponent { public selectedValue: any; public blurred = 0; - public hierarchicConfig: NaturalHierarchicConfiguration[] = [ - { - service: ItemService, - parentsRelationNames: ['parent'], - childrenRelationNames: ['parent'], - selectableAtKey: 'any', - }, - ]; + public hierarchicConfig = itemHierarchicConfig; public constructor(public readonly service: ItemService) {} From fe6c3f40e7649a26801c9d3e702e1f15468c32d7 Mon Sep 17 00:00:00 2001 From: Adrien Crivelli Date: Wed, 22 Feb 2023 13:03:17 +0700 Subject: [PATCH 03/10] Never show icon name in Google search result --- projects/natural/src/lib/modules/icon/icon.component.html | 4 ++-- projects/natural/src/lib/modules/icon/icon.component.ts | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/projects/natural/src/lib/modules/icon/icon.component.html b/projects/natural/src/lib/modules/icon/icon.component.html index 0246248a..b869f9cf 100644 --- a/projects/natural/src/lib/modules/icon/icon.component.html +++ b/projects/natural/src/lib/modules/icon/icon.component.html @@ -1,4 +1,4 @@ -{{ icon.font }} - + +
{{ label }}
diff --git a/projects/natural/src/lib/modules/icon/icon.component.ts b/projects/natural/src/lib/modules/icon/icon.component.ts index b0306382..c4c22a85 100644 --- a/projects/natural/src/lib/modules/icon/icon.component.ts +++ b/projects/natural/src/lib/modules/icon/icon.component.ts @@ -40,7 +40,9 @@ export class NaturalIconComponent { @Input() public labelColor: 'primary' | 'warn' | 'accent' = 'accent'; @Input() public labelPosition: 'top-right' | 'top-left' | 'bottom-right' | 'bottom-left' = 'top-right'; - public icon!: NaturalIconType; + public icon: NaturalIconType = { + name: '', + }; public constructor( private readonly matIconRegistry: MatIconRegistry, From bab12d278165eb60ceb7fe1809cdfbf57df247c0 Mon Sep 17 00:00:00 2001 From: Adrien Crivelli Date: Sat, 25 Feb 2023 17:06:33 +0700 Subject: [PATCH 04/10] NaturalDialogTriggerComponent can be configured with SEO #8520 The dialog title will prepend the page (and app) title --- .../common/services/seo.service.spec.ts | 131 +++++++++++++++--- .../modules/common/services/seo.service.ts | 58 ++++++-- 2 files changed, 161 insertions(+), 28 deletions(-) diff --git a/projects/natural/src/lib/modules/common/services/seo.service.spec.ts b/projects/natural/src/lib/modules/common/services/seo.service.spec.ts index 68394bed..ff4fb7b1 100644 --- a/projects/natural/src/lib/modules/common/services/seo.service.spec.ts +++ b/projects/natural/src/lib/modules/common/services/seo.service.spec.ts @@ -1,5 +1,12 @@ import {TestBed} from '@angular/core/testing'; -import {NATURAL_SEO_CONFIG, NaturalSeo, NaturalSeoConfig, NaturalSeoService} from '@ecodev/natural'; +import { + NATURAL_SEO_CONFIG, + NaturalDialogTriggerComponent, + NaturalDialogTriggerRoutingData, + NaturalSeo, + NaturalSeoConfig, + NaturalSeoService, +} from '@ecodev/natural'; import {stripTags} from './seo.service'; import {RouterTestingModule} from '@angular/router/testing'; import {Component, NgZone} from '@angular/core'; @@ -7,7 +14,7 @@ import {Router, Routes} from '@angular/router'; import {Meta, Title} from '@angular/platform-browser'; @Component({ - template: `
Test component
`, + template: `
Test simple component
`, }) class TestSimpleComponent {} @@ -32,6 +39,12 @@ describe('stripTags', () => { expect(stripTags('one > two > three')).toBe('one > two > three'); }); }); + +const dialogTrigger: NaturalDialogTriggerRoutingData = { + component: TestSimpleComponent, + dialogConfig: {}, +}; + const routes: Routes = [ { path: 'no-seo', @@ -94,6 +107,43 @@ const routes: Routes = [ }) as NaturalSeo, }, }, + { + path: 'basic-dialog', + component: NaturalDialogTriggerComponent, + outlet: 'secondary', + data: { + trigger: dialogTrigger, + seo: {title: 'basic dialog title'} as NaturalSeo, + }, + }, + { + path: 'resolve-dialog', + component: NaturalDialogTriggerComponent, + outlet: 'secondary', + data: { + trigger: dialogTrigger, + // Here we simulate the data structure after the resolve, + // but in a real app it would be resolved by a proper Resolver + user: { + model: { + name: 'dialog user name', + description: 'dialog user description', + }, + }, + seo: { + resolveKey: 'user', + robots: 'dialog resolve robots', + } as NaturalSeo, + }, + }, + { + path: 'primary-dialog', + component: NaturalDialogTriggerComponent, + data: { + trigger: dialogTrigger, + seo: {title: 'primary dialog title'} as NaturalSeo, + }, + }, ]; describe('NaturalSeoService', () => { @@ -105,22 +155,29 @@ describe('NaturalSeoService', () => { function assertSeo( url: string, + secondary: string | null, expectedTitle: string, expectedDescription: string | undefined, expectedRobots: string | undefined, ): Promise { return ngZone.run(() => - router.navigateByUrl(url).then(() => { - expect(title.getTitle()).toBe(expectedTitle); - expect(meta.getTag('name="description"')?.getAttribute('value')).toBe(expectedDescription); - expect(meta.getTag('name="robots"')?.getAttribute('value')).toBe(expectedRobots); - }), + router + .navigate([url]) + .then(() => { + if (secondary) return router.navigate([{outlets: {secondary: [secondary]}}]); + else return Promise.resolve(true); + }) + .then(() => { + expect(title.getTitle()).toBe(expectedTitle); + expect(meta.getTag('name="description"')?.getAttribute('value')).toBe(expectedDescription); + expect(meta.getTag('name="robots"')?.getAttribute('value')).toBe(expectedRobots); + }), ); } async function configure(config: NaturalSeoConfig): Promise { await TestBed.configureTestingModule({ - imports: [RouterTestingModule.withRoutes(routes)], + imports: [RouterTestingModule.withRoutes(routes, {enableTracing: true})], providers: [ { provide: NATURAL_SEO_CONFIG, @@ -148,23 +205,51 @@ describe('NaturalSeoService', () => { }); it('should update SEO automatically from default values', async () => { - await assertSeo('no-seo', 'my app', undefined, undefined); + await assertSeo('no-seo', null, 'my app', undefined, undefined); }); it('should update SEO automatically from basic routing', async () => { - await assertSeo('basic-seo', 'basic title - my app', 'basic description', 'basic robots'); + await assertSeo('basic-seo', null, 'basic title - my app', 'basic description', 'basic robots'); }); it('should update SEO automatically from resolve routing', async () => { - await assertSeo('resolve-seo', 'user name - my app', 'user description', 'resolve robots'); + await assertSeo('resolve-seo', null, 'user name - my app', 'user description', 'resolve robots'); }); it('should update SEO automatically from resolve routing even with null resolved', async () => { - await assertSeo('resolve-null-seo', 'my app', undefined, 'resolve null robots'); + await assertSeo('resolve-null-seo', null, 'my app', undefined, 'resolve null robots'); }); it('should update SEO automatically from callback routing', async () => { - await assertSeo('callback-seo', 'callback title - my app', 'callback description', 'callback robots'); + await assertSeo('callback-seo', null, 'callback title - my app', 'callback description', 'callback robots'); + }); + + it('should update SEO automatically with NaturalDialogTriggerComponent with basic SEO', async () => { + await assertSeo('no-seo', 'basic-dialog', 'basic dialog title - my app', undefined, undefined); + }); + + it('should update SEO automatically and combine SEO from page and NaturalDialogTriggerComponent', async () => { + await assertSeo( + 'basic-seo', + 'basic-dialog', + 'basic dialog title - basic title - my app', + undefined, + undefined, + ); + }); + + it('should update SEO automatically and resolve from NaturalDialogTriggerComponent data', async () => { + await assertSeo( + 'basic-seo', + 'resolve-dialog', + 'dialog user name - basic title - my app', + 'dialog user description', + 'dialog resolve robots', + ); + }); + + it('should duplicate title part of a NaturalDialogTriggerComponent which is on primary outlet', async () => { + await assertSeo('primary-dialog', null, 'primary dialog title - my app', undefined, undefined); }); }); @@ -183,20 +268,33 @@ describe('NaturalSeoService', () => { }); it('should update SEO automatically from default values', async () => { - await assertSeo('no-seo', 'my extra part - my app', 'my default description', 'my default robots'); + await assertSeo('no-seo', null, 'my extra part - my app', 'my default description', 'my default robots'); }); it('should update SEO automatically from basic routing', async () => { - await assertSeo('basic-seo', 'basic title - my extra part - my app', 'basic description', 'basic robots'); + await assertSeo( + 'basic-seo', + null, + 'basic title - my extra part - my app', + 'basic description', + 'basic robots', + ); }); it('should update SEO automatically from resolve routing', async () => { - await assertSeo('resolve-seo', 'user name - my extra part - my app', 'user description', 'resolve robots'); + await assertSeo( + 'resolve-seo', + null, + 'user name - my extra part - my app', + 'user description', + 'resolve robots', + ); }); it('should update SEO automatically from resolve routing even with null resolved', async () => { await assertSeo( 'resolve-null-seo', + null, 'my extra part - my app', 'my default description', 'resolve null robots', @@ -206,6 +304,7 @@ describe('NaturalSeoService', () => { it('should update SEO automatically from callback routing', async () => { await assertSeo( 'callback-seo', + null, 'callback title - my extra part - my app', 'callback description', 'callback robots', diff --git a/projects/natural/src/lib/modules/common/services/seo.service.ts b/projects/natural/src/lib/modules/common/services/seo.service.ts index fee37ac6..2d880d89 100644 --- a/projects/natural/src/lib/modules/common/services/seo.service.ts +++ b/projects/natural/src/lib/modules/common/services/seo.service.ts @@ -1,7 +1,8 @@ import {Inject, Injectable, InjectionToken} from '@angular/core'; import {Meta, Title} from '@angular/platform-browser'; -import {ActivatedRouteSnapshot, Data, NavigationEnd, Router} from '@angular/router'; +import {ActivatedRouteSnapshot, Data, NavigationEnd, PRIMARY_OUTLET, Router} from '@angular/router'; import {filter} from 'rxjs/operators'; +import {NaturalDialogTriggerComponent} from '../../dialog-trigger/dialog-trigger.component'; export type NaturalSeo = NaturalSeoBasic | NaturalSeoCallback | NaturalSeoResolve; @@ -97,7 +98,10 @@ type ResolvedData = { * * The full title has the following structure: * - * page title - extra part - app name + * dialog title - page title - extra part - app name + * + * `dialog title` only exists if a `NaturalDialogTriggerComponent` is currently open, and that some SEO is + * configured for it in the routing. */ @Injectable({ providedIn: 'root', @@ -112,9 +116,21 @@ export class NaturalSeoService { private readonly metaTagService: Meta, ) { this.router.events.pipe(filter(event => event instanceof NavigationEnd)).subscribe(() => { - this.routeData = this.getRouteData(this.router.routerState.root.snapshot); + const root = this.router.routerState.root.snapshot; + this.routeData = this.getRouteData(root); const seo: NaturalSeo = this.routeData.seo ?? {title: ''}; - const basic = this.toBasic(seo); + + const dialogRouteData = this.getDialogRouteData(root); + const dialogSeo: NaturalSeo = dialogRouteData?.seo; + + let basic = this.toBasic(seo, this.routeData); + if (dialogRouteData && dialogSeo) { + const dialogBasic = this.toBasic(dialogSeo, dialogRouteData); + basic = { + ...dialogBasic, + title: this.join([dialogBasic.title, basic.title]), + }; + } this.update(basic); }); @@ -133,11 +149,11 @@ export class NaturalSeoService { // Title const parts = [ seo.title, - this.config.extraPart && this.routeData ? this.config.extraPart(this.routeData) : null, + this.config.extraPart && this.routeData ? this.config.extraPart(this.routeData) : '', this.config.applicationName, ]; - const title = parts.filter(s => !!s).join(' - '); + const title = this.join(parts); this.titleService.setTitle(title); // Description @@ -149,6 +165,10 @@ export class NaturalSeoService { this.updateTag('robots', robots); } + private join(parts: string[]): string { + return parts.filter(s => !!s).join(' - '); + } + private updateTag(name: string, value?: string): void { if (value) { this.metaTagService.updateTag({ @@ -167,19 +187,33 @@ export class NaturalSeoService { if (route.firstChild) { return this.getRouteData(route.firstChild); } else { - return route.data ?? null; + return route.data; } } - private toBasic(seo: NaturalSeo): NaturalSeoBasic { - if (!this.routeData) { - throw new Error('Must have some route data to get basic SEO'); + /** + * Returns the data from the `NaturalDialogTriggerComponent` if one is open + */ + private getDialogRouteData(route: ActivatedRouteSnapshot): Data | null { + if (route.component === NaturalDialogTriggerComponent && route.outlet !== PRIMARY_OUTLET) { + return route.data; + } + + for (const child of route.children) { + const data = this.getDialogRouteData(child); + if (data) { + return data; + } } + return null; + } + + private toBasic(seo: NaturalSeo, routeData: Data): NaturalSeoBasic { if (typeof seo === 'function') { - return seo(this.routeData); + return seo(routeData); } else if ('resolveKey' in seo) { - const data: ResolvedData | undefined = this.routeData[seo.resolveKey]; + const data: ResolvedData | undefined = routeData[seo.resolveKey]; if (!data) { throw new Error('Could not find resolved data for SEO service with key: ' + seo.resolveKey); } From 92f10ac6fcc457c3a96eb5a957eb3c402424bbd8 Mon Sep 17 00:00:00 2001 From: Adrien Crivelli Date: Sat, 25 Feb 2023 17:44:53 +0700 Subject: [PATCH 05/10] Simplify tests #8520 --- .../common/services/seo.service.spec.ts | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/projects/natural/src/lib/modules/common/services/seo.service.spec.ts b/projects/natural/src/lib/modules/common/services/seo.service.spec.ts index ff4fb7b1..d2aa5c8d 100644 --- a/projects/natural/src/lib/modules/common/services/seo.service.spec.ts +++ b/projects/natural/src/lib/modules/common/services/seo.service.spec.ts @@ -9,7 +9,7 @@ import { } from '@ecodev/natural'; import {stripTags} from './seo.service'; import {RouterTestingModule} from '@angular/router/testing'; -import {Component, NgZone} from '@angular/core'; +import {Component} from '@angular/core'; import {Router, Routes} from '@angular/router'; import {Meta, Title} from '@angular/platform-browser'; @@ -151,7 +151,6 @@ describe('NaturalSeoService', () => { let router: Router; let title: Title; let meta: Meta; - let ngZone: NgZone; function assertSeo( url: string, @@ -160,19 +159,17 @@ describe('NaturalSeoService', () => { expectedDescription: string | undefined, expectedRobots: string | undefined, ): Promise { - return ngZone.run(() => - router - .navigate([url]) - .then(() => { - if (secondary) return router.navigate([{outlets: {secondary: [secondary]}}]); - else return Promise.resolve(true); - }) - .then(() => { - expect(title.getTitle()).toBe(expectedTitle); - expect(meta.getTag('name="description"')?.getAttribute('value')).toBe(expectedDescription); - expect(meta.getTag('name="robots"')?.getAttribute('value')).toBe(expectedRobots); - }), - ); + return router + .navigate([url]) + .then(() => { + if (secondary) return router.navigate([{outlets: {secondary: [secondary]}}]); + else return Promise.resolve(true); + }) + .then(() => { + expect(title.getTitle()).toBe(expectedTitle); + expect(meta.getTag('name="description"')?.getAttribute('value')).toBe(expectedDescription); + expect(meta.getTag('name="robots"')?.getAttribute('value')).toBe(expectedRobots); + }); } async function configure(config: NaturalSeoConfig): Promise { @@ -190,7 +187,6 @@ describe('NaturalSeoService', () => { router = TestBed.inject(Router); title = TestBed.inject(Title); meta = TestBed.inject(Meta); - ngZone = TestBed.inject(NgZone); } describe('with simplest config', () => { From 79836ea7b6142c19c4c6e1ed7268f968e94e16d4 Mon Sep 17 00:00:00 2001 From: Adrien Crivelli Date: Mon, 27 Feb 2023 12:35:49 +0700 Subject: [PATCH 06/10] natural-select can specify which search operator to use #9429 This is useful if we need a different operator than the default `search` --- .../modules/select/select/select.component.ts | 62 ++++++++++++++----- 1 file changed, 47 insertions(+), 15 deletions(-) diff --git a/projects/natural/src/lib/modules/select/select/select.component.ts b/projects/natural/src/lib/modules/select/select/select.component.ts index 32540110..7942893c 100644 --- a/projects/natural/src/lib/modules/select/select/select.component.ts +++ b/projects/natural/src/lib/modules/select/select/select.component.ts @@ -14,28 +14,40 @@ type V = string | ExtractTallOne; /** * Default usage: - * + * ```html + * + * ``` * * Custom template usage : + * ```html * * * {{ item.xxx }} * * + * ``` * - * [(ngModel)] and (ngModelChange) are optional + * `[(ngModel)]` and `(ngModelChange)` are optional. * * Placeholder : - * + * ```html + * + * ``` * * Never float placeholder : - * + * ```html + * + * ``` * - * Search with like %xxx% on specified attribute name instead of custom filter on whole object - * + * Search with like %xxx% on specified field `name` instead of custom filter on whole object + * ```html + * + * ``` * * Allows to input free string without selecting an option from autocomplete suggestions - * + * ```html + * + * ``` */ @Component({ selector: 'natural-select', @@ -73,10 +85,15 @@ export class NaturalSelectComponent< @Input() public optionRequired = true; /** - * The filter attribute to bind when searching for a term + * The field on which to search for, default to 'custom'. */ @Input() public searchField: 'custom' | string = 'custom'; + /** + * The operator with which to search for, default to 'search' if `searchField` is 'custom', else 'like'. + */ + @Input() public searchOperator: 'search' | string | null = null; + /** * Additional filter for query */ @@ -221,7 +238,7 @@ export class NaturalSelectComponent< this.loading = !!this.items; } - this.variablesManager.merge('variables', this.getSearchFilter(term as string | null)); + this.variablesManager.merge('variables', this.getSearchFilter(term)); } } @@ -230,14 +247,29 @@ export class NaturalSelectComponent< } private getSearchFilter(term: string | null): QueryVariables { - let field: Literal = {}; + if (!term) { + return {}; + } - if (this.searchField === 'custom') { - field = {custom: term ? {search: {value: term}} : null}; - } else if (term) { - field[this.searchField] = {like: {value: '%' + term + '%'}}; + const searchOperator = this.searchOperator ?? (this.searchField === 'custom' ? 'search' : 'like'); + if (searchOperator === 'like') { + term = '%' + term + '%'; } - return {filter: {groups: [{conditions: [field]}]}}; + return { + filter: { + groups: [ + { + conditions: [ + { + [this.searchField]: { + [searchOperator]: {value: term}, + }, + }, + ], + }, + ], + }, + }; } } From 26ad8c35a049d472eebf86595abd6bdd99606a6d Mon Sep 17 00:00:00 2001 From: Adrien Crivelli Date: Thu, 9 Mar 2023 15:13:41 +0700 Subject: [PATCH 07/10] Emptying field with "close" button does not throw error Instead, it sets the selection to `null` Fixes #236 --- .../classes/abstract-editable-list.spec.ts | 2 +- .../src/lib/classes/abstract-editable-list.ts | 2 + .../relations/relations.component.html | 2 +- .../relations/relations.component.spec.ts | 49 +++++++++++++++++++ .../modules/relations/relations.component.ts | 19 ++++--- .../natural/src/lib/testing/item.service.ts | 2 + src/app/panels-routing.ts | 9 +++- 7 files changed, 74 insertions(+), 11 deletions(-) create mode 100644 projects/natural/src/lib/modules/relations/relations.component.spec.ts diff --git a/projects/natural/src/lib/classes/abstract-editable-list.spec.ts b/projects/natural/src/lib/classes/abstract-editable-list.spec.ts index 2954e4ee..11213d7c 100644 --- a/projects/natural/src/lib/classes/abstract-editable-list.spec.ts +++ b/projects/natural/src/lib/classes/abstract-editable-list.spec.ts @@ -47,7 +47,7 @@ describe('NaturalAbstractEditableList', () => { expect(list.formArray.length).toBe(1); expect(list.dataSource.data.length).toBe(1); expect(list.getItems()).toEqual([ - {id: '1', name: 'name-1', description: 'description-1', children: [], parent: null}, + {id: '1', name: 'name-1', description: 'description-1', children: [], parent: null} as unknown as Item, ]); list.setItems([service.getItem(), {} as Item]); diff --git a/projects/natural/src/lib/classes/abstract-editable-list.ts b/projects/natural/src/lib/classes/abstract-editable-list.ts index 94ae9ec6..e80bb4ec 100644 --- a/projects/natural/src/lib/classes/abstract-editable-list.ts +++ b/projects/natural/src/lib/classes/abstract-editable-list.ts @@ -103,6 +103,8 @@ export class NaturalAbstractEditableList< * - AbstractModelService.getPartialVariablesForCreation() * - AbstractModelService.getPartialVariablesForUpdate() * - some other required treatment. + * + * TODO return type is incorrect and should be closer to `Partial[]` or an even looser type, because we don't really know what fields exists in the form. When we fix this, we should also remove type coercing in unit tests. */ public getItems(): T[] { return this.formArray.getRawValue(); diff --git a/projects/natural/src/lib/modules/relations/relations.component.html b/projects/natural/src/lib/modules/relations/relations.component.html index 8a615e0b..94c70562 100644 --- a/projects/natural/src/lib/modules/relations/relations.component.html +++ b/projects/natural/src/lib/modules/relations/relations.component.html @@ -53,7 +53,7 @@ { + let component: NaturalRelationsComponent; + let fixture: ComponentFixture>; + let spy: jasmine.Spy; + const main = {__typename: 'Main', id: '123'} as const; + const relation = {__typename: 'Relation', id: '456'} as const; + const otherName = 'my-other-name'; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + providers: [MockApolloProvider], + imports: [NaturalRelationsModule], + }).compileComponents(); + fixture = TestBed.createComponent(NaturalRelationsComponent); + component = fixture.componentInstance; + component.main = main; + component.otherName = otherName; + + fixture.detectChanges(); + + spy = spyOn(TestBed.inject(NaturalLinkMutationService), 'link').and.callFake(() => of(null as any)); + }); + + it('should be created', () => { + expect(component).toBeTruthy(); + }); + + it('should link if adding relation', done => { + component.selectionChange.subscribe(() => { + expect(spy).toHaveBeenCalledOnceWith(main, relation, otherName); + done(); + }); + component.addRelations([relation]); + }); + + it('should do nothing if adding null relations', done => { + component.selectionChange.subscribe(() => { + expect(spy).not.toHaveBeenCalled(); + done(); + }); + component.addRelations([null]); + }); +}); diff --git a/projects/natural/src/lib/modules/relations/relations.component.ts b/projects/natural/src/lib/modules/relations/relations.component.ts index 8b01b908..6690b4e7 100644 --- a/projects/natural/src/lib/modules/relations/relations.component.ts +++ b/projects/natural/src/lib/modules/relations/relations.component.ts @@ -11,7 +11,7 @@ import { ViewChild, } from '@angular/core'; import {LegacyPageEvent as PageEvent} from '@angular/material/legacy-paginator'; -import {forkJoin, tap} from 'rxjs'; +import {forkJoin, of, tap} from 'rxjs'; import {NaturalAbstractController} from '../../classes/abstract-controller'; import {NaturalDataSource, PaginatedData} from '../../classes/data-source'; import {NaturalQueryVariablesManager, PaginationInput, QueryVariables} from '../../classes/query-variable-manager'; @@ -24,7 +24,7 @@ import {Filter} from '../search/classes/graphql-doctrine.types'; import {NaturalSelectComponent} from '../select/select/select.component'; import {finalize, takeUntil} from 'rxjs/operators'; import {NaturalAbstractModelService} from '../../services/abstract-model.service'; -import {Literal} from '../../types/types'; +import {ExtractTallOne} from '../../types/types'; /** * Custom template usage : @@ -44,7 +44,7 @@ export class NaturalRelationsComponent< TService extends NaturalAbstractModelService< any, any, - PaginatedData, + PaginatedData, QueryVariables, any, any, @@ -107,7 +107,7 @@ export class NaturalRelationsComponent< /** * Listing service instance */ - public dataSource?: NaturalDataSource; + public dataSource?: NaturalDataSource>; public loading = false; /** @@ -185,10 +185,13 @@ export class NaturalRelationsComponent< * Refetch result to display it in table * TODO : could maybe use "update" attribute of apollo.mutate function to update table faster (but hard to do it here) */ - public addRelations(relations: LinkableObject[]): void { - const observables = relations.map(relation => - this.linkMutationService.link(this.main, relation, this.otherName), - ); + public addRelations(relations: (LinkableObject | ExtractTallOne | string | null)[]): void { + const observables = [ + of(null), + ...relations + .filter((relation): relation is LinkableObject => !!relation && typeof relation === 'object') + .map(relation => this.linkMutationService.link(this.main, relation, this.otherName)), + ]; forkJoin(observables).subscribe(() => { this.selectionChange.emit(); diff --git a/projects/natural/src/lib/testing/item.service.ts b/projects/natural/src/lib/testing/item.service.ts index 7fed3a63..ebc6f6a6 100644 --- a/projects/natural/src/lib/testing/item.service.ts +++ b/projects/natural/src/lib/testing/item.service.ts @@ -12,6 +12,7 @@ import {deepClone} from '../modules/search/classes/utils'; import {NaturalDebounceService} from '../services/debounce.service'; export interface Item { + readonly __typename: 'Item'; readonly id: string; readonly name: string; readonly description: string; @@ -45,6 +46,7 @@ export class ItemService extends NaturalAbstractModelService< public getItem(withChildren = false, parentsDeep = 0, wantedId?: string): Item { const id = wantedId ?? this.id++; return deepFreeze({ + __typename: 'Item', id: '' + id, name: 'name-' + id, description: 'description-' + id, diff --git a/src/app/panels-routing.ts b/src/app/panels-routing.ts index 2cbc61f0..ac6c11b9 100644 --- a/src/app/panels-routing.ts +++ b/src/app/panels-routing.ts @@ -14,7 +14,14 @@ import {Item} from '../../projects/natural/src/lib/testing/item.service'; }) class MyResolver implements Resolve, NaturalPanelResolve { public resolve(): Observable { - return of({id: '123', name: 'resolved', description: 'resolved description', children: [], parent: null}); + return of({ + __typename: 'Item', + id: '123', + name: 'resolved', + description: 'resolved description', + children: [], + parent: null, + }); } } From c98092890bf3b16e19bed4626e92c13180bee31d Mon Sep 17 00:00:00 2001 From: Adrien Crivelli Date: Thu, 9 Mar 2023 15:43:19 +0700 Subject: [PATCH 08/10] Stricter types --- .../relations/relations.component.html | 3 +-- .../modules/relations/relations.component.ts | 23 +++++++++---------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/projects/natural/src/lib/modules/relations/relations.component.html b/projects/natural/src/lib/modules/relations/relations.component.html index 94c70562..740b7c46 100644 --- a/projects/natural/src/lib/modules/relations/relations.component.html +++ b/projects/natural/src/lib/modules/relations/relations.component.html @@ -52,10 +52,9 @@ , QueryVariables, + unknown, any, + unknown, any, - any, - any, - any, + unknown, any >, > @@ -58,7 +57,7 @@ export class NaturalRelationsComponent< implements OnInit, OnChanges, OnDestroy { @ViewChild(NaturalSelectComponent) private select?: NaturalSelectComponent; - @ContentChild(TemplateRef) public itemTemplate?: TemplateRef; + @ContentChild(TemplateRef) public itemTemplate?: TemplateRef; @Input() public service?: TService; @@ -70,12 +69,12 @@ export class NaturalRelationsComponent< /** * Filter for autocomplete selector */ - @Input() public autocompleteSelectorFilter?: Filter; + @Input() public autocompleteSelectorFilter?: ExtractVall['filter'] | null | undefined; /** * Function to customize the rendering of the selected item as text in input */ - @Input() public displayWith?: (item: any) => string; + @Input() public displayWith?: (item: ExtractTallOne | null) => string; /** * Whether the relations can be changed @@ -142,7 +141,7 @@ export class NaturalRelationsComponent< * the objectives that have indeed a relation to the particular action. */ @Input() - public set filter(filter: Filter) { + public set filter(filter: ExtractVall['filter']) { this.variablesManager.set('relations-filter', {filter: filter}); } @@ -216,12 +215,12 @@ export class NaturalRelationsComponent< this.variablesManager.set('pagination', {pagination: pagination ? pagination : this.defaultPagination}); } - public getDisplayFn(): (item: any) => string { + public getDisplayFn(): (item: ExtractTallOne | null) => string { if (this.displayWith) { return this.displayWith; } - return item => (item ? item.fullName || item.name : ''); + return (item: any) => (item ? item.fullName || item.name : ''); } public openNaturalHierarchicSelector(): void { From 32a24159f7a4fcd540bab6190828ddd60671e6eb Mon Sep 17 00:00:00 2001 From: Adrien Crivelli Date: Thu, 9 Mar 2023 17:08:28 +0700 Subject: [PATCH 09/10] Can cancel previous search --- .../select/select/select.component.spec.ts | 93 ++++++++++++++++++- .../modules/select/select/select.component.ts | 20 ++-- 2 files changed, 102 insertions(+), 11 deletions(-) diff --git a/projects/natural/src/lib/modules/select/select/select.component.spec.ts b/projects/natural/src/lib/modules/select/select/select.component.spec.ts index f310e905..156de8a3 100644 --- a/projects/natural/src/lib/modules/select/select/select.component.spec.ts +++ b/projects/natural/src/lib/modules/select/select/select.component.spec.ts @@ -76,7 +76,7 @@ describe('NaturalSelectComponent', () => { data.fixture.detectChanges(); }); - testSelectAndSelectHierarchicCommonBehavior(data); + testSelectComponent(data); }); describe('with formControl', () => { @@ -87,6 +87,95 @@ describe('NaturalSelectComponent', () => { data.fixture.detectChanges(); }); - testSelectAndSelectHierarchicCommonBehavior(data); + testSelectComponent(data); }); }); + +function testSelectComponent(data: TestFixture>): void { + testSelectAndSelectHierarchicCommonBehavior(data); + + it('search variables are correct', () => { + expect(data.selectComponent.getVariablesForDebug()).toEqual({ + filter: {groups: [{conditions: [{custom: null}]}]}, + pagination: { + pageIndex: 0, + pageSize: 10, + }, + }); + + // Can search with default `custom.search` + data.selectComponent.search('foo'); + expect(data.selectComponent.getVariablesForDebug()).toEqual({ + filter: {groups: [{conditions: [{custom: {search: {value: 'foo'}}}]}]}, + pagination: { + pageIndex: 0, + pageSize: 10, + }, + }); + + // Can cancel previous search + data.selectComponent.search(''); + expect(data.selectComponent.getVariablesForDebug()).toEqual({ + filter: {groups: [{conditions: [{custom: null}]}]}, + pagination: { + pageIndex: 0, + pageSize: 10, + }, + }); + + // Can search on specified field `name` and will default to `like` operator + data.selectComponent.searchField = 'name'; + data.selectComponent.search('foo'); + expect(data.selectComponent.getVariablesForDebug()).toEqual({ + filter: {groups: [{conditions: [{name: {like: {value: '%foo%'}}}]}]}, + pagination: { + pageIndex: 0, + pageSize: 10, + }, + }); + + // Can cancel previous search + data.selectComponent.search(''); + expect(data.selectComponent.getVariablesForDebug()).toEqual({ + filter: {groups: [{conditions: [{name: null}]}]}, + pagination: { + pageIndex: 0, + pageSize: 10, + }, + }); + + // Can search on default field `custom` with specified `like` operator + data.selectComponent.searchField = 'custom'; + data.selectComponent.searchOperator = 'like'; + data.selectComponent.search('foo'); + expect(data.selectComponent.getVariablesForDebug()).toEqual({ + filter: {groups: [{conditions: [{custom: {like: {value: '%foo%'}}}]}]}, + pagination: { + pageIndex: 0, + pageSize: 10, + }, + }); + + // Can cancel previous search + data.selectComponent.search(''); + expect(data.selectComponent.getVariablesForDebug()).toEqual({ + filter: {groups: [{conditions: [{custom: null}]}]}, + pagination: { + pageIndex: 0, + pageSize: 10, + }, + }); + + // Can search on specific `myField.myOperator` + data.selectComponent.searchField = 'myField'; + data.selectComponent.searchOperator = 'myOperator'; + data.selectComponent.search('foo'); + expect(data.selectComponent.getVariablesForDebug()).toEqual({ + filter: {groups: [{conditions: [{myField: {myOperator: {value: 'foo'}}}]}]}, + pagination: { + pageIndex: 0, + pageSize: 10, + }, + }); + }); +} diff --git a/projects/natural/src/lib/modules/select/select/select.component.ts b/projects/natural/src/lib/modules/select/select/select.component.ts index 7942893c..1592032a 100644 --- a/projects/natural/src/lib/modules/select/select/select.component.ts +++ b/projects/natural/src/lib/modules/select/select/select.component.ts @@ -108,7 +108,7 @@ export class NaturalSelectComponent< public items: null | Observable = null; /** - * Whether a we are searching something + * Whether we are searching something */ public loading = false; @@ -247,12 +247,8 @@ export class NaturalSelectComponent< } private getSearchFilter(term: string | null): QueryVariables { - if (!term) { - return {}; - } - const searchOperator = this.searchOperator ?? (this.searchField === 'custom' ? 'search' : 'like'); - if (searchOperator === 'like') { + if (term && searchOperator === 'like') { term = '%' + term + '%'; } @@ -262,9 +258,11 @@ export class NaturalSelectComponent< { conditions: [ { - [this.searchField]: { - [searchOperator]: {value: term}, - }, + [this.searchField]: term + ? { + [searchOperator]: {value: term}, + } + : null, }, ], }, @@ -272,4 +270,8 @@ export class NaturalSelectComponent< }, }; } + + public getVariablesForDebug(): Readonly | undefined { + return this.variablesManager.variables.value; + } } From 46da8879a8deeab608ed3a5078fefc8e8fcacd9c Mon Sep 17 00:00:00 2001 From: Adrien Crivelli Date: Fri, 10 Mar 2023 10:36:20 +0700 Subject: [PATCH 10/10] Support Safari 13 and 14 to upload files --- .../natural/src/lib/modules/file/utils.ts | 36 ++++++++++++------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/projects/natural/src/lib/modules/file/utils.ts b/projects/natural/src/lib/modules/file/utils.ts index 849cf01f..8d90fec0 100644 --- a/projects/natural/src/lib/modules/file/utils.ts +++ b/projects/natural/src/lib/modules/file/utils.ts @@ -85,18 +85,30 @@ function createFileInput(document: Document): HTMLInputElement { } export function isDirectory(file: File): Promise { - return file - .slice(0, 1) - .text() - .then( - text => { - // Firefox will return empty string for a folder, so we must check that special case. - // That means that any empty file will incorrectly be interpreted as a folder on all - // browsers, but that's tolerable because there is no real use-case to upload an empty file. - return text !== ''; - }, - () => false, - ); + return blobText(file.slice(0, 1)).then( + text => { + // Firefox will return empty string for a folder, so we must check that special case. + // That means that any empty file will incorrectly be interpreted as a folder on all + // browsers, but that's tolerable because there is no real use-case to upload an empty file. + return text !== ''; + }, + () => false, + ); +} + +/** + * This is a ponyfill for `Blob.text()`, because Safari 13 and 14 do not support it, https://caniuse.com/?search=blob.text, + * and we try our best not to break iPhone users too much. + */ +function blobText(blob: Blob): Promise { + return new Promise((resolve, reject) => { + const reader = new FileReader(); + reader.onload = () => { + resolve(reader.result as string); + }; + reader.onerror = reject; + reader.readAsText(blob); + }); } export function stopEvent(event: Event): void {