# gbdirectives Library — Deep-Dive Analysis **Location:** `libs/gbdirectives/` **Date:** 2026-02-25 **Scope:** All components, services, models, and module --- ## Library Overview `gbdirectives` is the shared form-field component library used by virtually every module in the application. It contains: | Component | Selector | Purpose | |---|---|---| | `GbinputComponent` | `gb-input` | Text/number/email/percentage input | | `GbcomboboxComponent` | `gb-combobox` | Static dropdown from `PickLists` | | `GbradiobuttonComponent` | `gb-radiobutton` | Radio button group | | `GbcheckboxComponent` | `gb-checkbox` | Single checkbox (0/1) | | `GbDateComponent` | `gb-date` | Date picker (MS `/Date(epoch)/` format) | | `GbTextareaComponent` | `gb-textarea` | Multi-line text input | | `GbTimeComponent` | `gb-time` | HH:MM time input with ngx-mask | | `GbToggleComponent` | `gb-toggle` | Simple slide toggle | | `GbpicklistComponent` | `gb-picklist` | Picklist with dialog grid (old) | | `GbNewPicklistComponent` | `gb-newpicklist` | Picklist with dialog grid (new/refactored) | | `GbFormGridComponent` | `gb-formgrid` | Editable grid row component (huge) | | `GbaddonComponent` | `gb-addon` | Dynamic add-on/deduction fields | | `GbdynamiccomboboxComponent` | `gb-dynamiccombobox` | Combobox with live HTTP picklist fetch | | `PicklistGridComponent` | `gb-picklistgrid` | AG Grid dialog for picklist selection | | `ExampleHeader` | `example-header` | Custom datepicker header (same file as GbDate) | **Services:** `GbdirectivesService`, `PicklistService`, `PicklistDbService` **Model:** `Idirectives.model.ts` (IField, IGrid, IPickListDetail, IPickListValue, IPicklistObject) All components implement `ControlValueAccessor` and integrate with the form-field system via `NgControl`. --- ## P0 — Critical Issues (Fix Immediately) ### P0-1: Missing `ChangeDetectionStrategy.OnPush` on virtually all components **Impact:** Every Angular CD cycle re-evaluates ALL instances of these components across the page. In forms with 20–50 fields, this multiplies CD work 20–50×. | Component | Has OnPush? | |---|---| | `GbinputComponent` | ❌ No | | `GbcomboboxComponent` | ❌ No | | `GbradiobuttonComponent` | ❌ No | | `GbcheckboxComponent` | ❌ No | | `GbDateComponent` | ❌ No | | `GbTextareaComponent` | ❌ No | | `GbTimeComponent` | ❌ No | | `GbToggleComponent` | ❌ No | | `GbpicklistComponent` | ✅ Yes | | `GbNewPicklistComponent` | ❌ No | | `GbFormGridComponent` | ❌ No | | `GbaddonComponent` | ❌ No | | `GbdynamiccomboboxComponent` | ❌ No | | `PicklistGridComponent` | ❌ No | Only `GbpicklistComponent` (the old, less-used version) has `OnPush`. The new `GbNewPicklistComponent` does not. Since these components are embedded in every form in the application (~830 components), the CD overhead is application-wide. **Fix:** Add `changeDetection: ChangeDetectionStrategy.OnPush` to every `@Component` decorator listed above. --- ### P0-2: `sessionStorage.getItem('LoginDTO')` in `PicklistDbService` **File:** `libs/gbdirectives/src/lib/gbpicklist/dbservice/gbpicklist.db.service.ts:17` ```typescript let LoginDTODetail: any = sessionStorage.getItem('LoginDTO') this.LoginDTO = JSON.parse(LoginDTODetail) ``` - Direct sessionStorage read for authentication data — P0 security violation per CLAUDE.md - No null-check: if `LoginDTO` is not in session (e.g., SSO session expired), `JSON.parse(null)` returns `null` and the downstream `this.LoginDTO.WorkOUId` throws - Called every picklist API invocation — this code path runs hundreds of times per session **Fix:** Inject and use the auth service signal instead of raw sessionStorage reads. --- ### P0-3: Multiple untracked `setTimeout` calls — memory leaks The following `setTimeout` calls are never stored or cleared in `ngOnDestroy`, creating leaks when components are destroyed before the timer fires: | Component | Location | Detail | |---|---|---| | `GbpicklistComponent` | constructor | `setTimeout(() => { ... ngControl.field ...}, 1)` | | `GbDateComponent` | constructor | `setTimeout(() => { ... this.Field ...}, 1)` | | `GbTimeComponent` | constructor | `setTimeout(() => { ... this.Field ...}, 1)` | | `GbdynamiccomboboxComponent` | constructor | `setTimeout(() => { ... ngControl.field ...}, 1)` | | `GbdynamiccomboboxComponent` | `writeValue()` | `setTimeout(() => { this.FetchPicklistData() }, 500)` — fires HTTP every time the form writes a value! | | `GbNewPicklistComponent` | constructor | `setTimeout(...)` | | `GbNewPicklistComponent` | `ngOnChanges` | 5× `setTimeout(...)` across different change branches | | `GbNewPicklistComponent` | `ngOnInit` | `setTimeout(() => { focus }, 100)` | | `GbaddonComponent` | constructor | `setTimeout(() => { ... ngControl.field ...}, 1)` | | `GbpicklistComponent` | `OnPicklistClick` | Untracked `setTimeout` in DexieDB path | | `GbTimeComponent` | `onTimeChange()` | `setTimeout(() => this.isRemoveTriggered = false, 100)` — repeated on every keystroke | | `PicklistGridComponent` | `applyDialogWidth()` | `setTimeout(() => { DOM query ...}, 0)` | | `PicklistGridComponent` | `onGridReady()` | `setTimeout(() => { this.preselectRows() }, 100)` | | `PicklistGridComponent` | `setupScrollSync()` | `setTimeout(() => { DOM query ...}, 150)` | | `PicklistGridComponent` | `PicklistServiceCall()` | `setTimeout(() => { re-select rows }, 200)` — inside subscribe | **Worst case:** `GbdynamiccomboboxComponent.writeValue()` fires an HTTP call with a 500ms delay on every `patchValue()`. In a form with 5 dynamic comboboxes, a single `form.reset()` triggers 5 HTTP calls. **Fix:** Store all timeout handles, cancel in `ngOnDestroy`. Use `takeUntilDestroyed()` (Angular 16+) or RxJS `timer()` for time-based logic. --- ### P0-4: Raw `.subscribe()` without cleanup | Component | Location | Issue | |---|---|---| | `GbinputComponent` | `Translate()` | `.subscribe()` without `takeUntil` | | `GbTextareaComponent` | `Translate()` | `.subscribe()` without `takeUntil` | | `GbNewPicklistComponent` | `Translate()` | `.subscribe()` without `takeUntil` | | `GbDateComponent` | `ngAfterViewInit()` | `this.pickers.changes.subscribe()` — never unsubscribed (no `ngOnDestroy`) | | `GbDateComponent` | `initTodayButton()` | `picker.openedStream.subscribe()` inside a loop — never unsubscribed | | `GbaddonComponent` | `fetchPicklistData()` | `Promise` wrapper around `.subscribe()` — underlying subscription never cleaned | `GbDateComponent` does not implement `OnDestroy` at all despite having subscriptions. --- ### P0-5: `console.log` in production code Per CLAUDE.md, `GbConsoleService` must be used instead. | File | Line | Statement | |---|---|---| | `gbdate.component.ts` | ~64 | `console.log("Date Field:", this.Field)` — logs field config on every component init | | `gbdate.component.ts` | ~300 | `console.log("Date Event:", event)` — logs every date change event | | `picklistgrid.component.ts` | ~690 | `console.log("this.searchText:", this.searchText)` | | `picklistgrid.component.ts` | ~692 | `console.log("Is Empty Search:", isEmptySearch)` | | `picklistgrid.component.ts` | ~375 | `console.warn('Scroll sync: viewports not found')` | --- ### P0-6: All `MatDialog` opens use fixed pixel widths Per CLAUDE.md, dialog widths must be `min(Xpx, 95vw)` with `maxWidth: '95vw'`. | Component | Width used | |---|---| | `GbinputComponent.formatmail()` | `width: '600px'` | | `GbinputComponent.onInputFocusOutChange()` | `width: '600px'` | | `GbinputComponent.Translate()` | `width: '600px'` | | `GbTextareaComponent.Translate()` | `width: '600px'` | | `GbTimeComponent.OnBlurTimeChange()` | `width: '400px'` | | `GbTimeComponent.onTimeChange()` | `width: '400px'` | | `GbTimeComponent.ngOnChanges()` (3 calls) | `width: '400px'` | | `GbNewPicklistComponent.OnPicklistClick()` | `width: '665px'`, `height: '560px'` | | `GbpicklistComponent.OnPicklistClick()` | `width: '665px'`, `height: '550px'` | | `PicklistGridComponent.onSelectionChanged()` | `width: '600px'` | | `PicklistGridComponent` | `width: '600px'` (IsInitAlpha warn) | All must be changed to `width: 'min(Xpx, 95vw)'` and add `maxWidth: '95vw'`. --- ### P0-7: `GbdynamiccomboboxComponent.writeValue()` fires HTTP on every form write ```typescript writeValue(value: string): void { setTimeout(() => { this.FetchPicklistData() // ← HTTP call every time the form sets this field's value }, 500); this.Fieldvalue = value this.cdr.detectChanges(); } ``` `writeValue()` is called by Angular's form system every time the field's value changes programmatically (e.g., `patchValue`, `reset`, `setValue`). This means every picklist load, form reset, or navigation fires HTTP requests for every dynamic combobox on the page. **Fix:** Load data only once in `ngOnInit`, or on first dropdown open via `onDropdownOpened`. Never inside `writeValue()`. --- ### P0-8: `GbaddonComponent` circular import via `GbDirectivesModule` `gbaddon.component.ts:24`: ```typescript import { GbDirectivesModule } from '../gbdirectives.module'; ``` `GbDirectivesModule` exports `GbaddonComponent` (if it were included). `GbaddonComponent` imports `GbDirectivesModule`. This is a circular dependency that can cause runtime initialization issues. **Fix:** Import specific standalone components directly (e.g., `GbinputComponent`, `GbpicklistComponent`) instead of the NgModule. --- ## P1 — High Priority ### P1-1: Constructor injection mixed with `inject()` — violates CLAUDE.md All components use `@Optional() @Self() public ngControl: NgControl` in the constructor. This is required for `ControlValueAccessor` to work correctly with Angular's form system. However, other services like `FormActionservice`, `MatDialog`, `ChangeDetectorRef` etc. are injected both ways — some via constructor parameters, some via `inject()`. This inconsistency creates confusion. **Components still using constructor injection for non-NgControl services:** - `GbpicklistComponent`: `PicklistService`, `MatDialog`, `FormActionservice`, `DexieService` all in constructor - `GbTimeComponent`: `FormActionservice` in constructor - `GbaddonComponent`: `FormActionservice`, `FormBuilder` in constructor - `PicklistGridComponent`: `MatDialogRef`, `DexieService`, `MatDialog`, `PicklistService`, `ElementRef` all in constructor **Fix:** Move non-NgControl dependencies to `inject()` calls at class body level. `NgControl` must stay in the constructor signature due to Angular DI requirements. --- ### P1-2: Pervasive use of `any` type — zero type safety | Location | Count | Examples | |---|---|---| | `Idirectives.model.ts` | 12 `any` fields | `ExpandedView?: any`, `CustomStyle?: any`, `IsFormGroup?: any` | | `IGrid` interface | 15+ `any` fields | `IsDependValue?: any`, `PicklistDetail?: any`, `Buttons?: any` | | `PicklistDbService.getPicklistData()` | 20+ `any` vars | All criteria building uses `any` | | `PicklistGridComponent` | 30+ `any` vars | All grid params, cell renderers, row data | | `GbaddonComponent` | 15+ `any` vars | Form group config, picklist data | | `GbFormGridComponent` | Widespread | Every @Input except typed ones | **Fix:** Define interfaces for all API payloads (picklist criteria, response data), grid column definitions, and cell renderer params. --- ### P1-3: Dead import creates cross-library coupling in the model file `Idirectives.model.ts:1`: ```typescript import { DayBook } from "projects/finance/model/idaybook"; ``` `DayBook` is imported but never used in the file. This dead import creates a compile-time dependency from the `gbdirectives` shared library on the `finance` MFE — breaking the library's independence. **Fix:** Remove the unused import. --- ### P1-4: `GbdirectivesService` contains finance domain methods The shared service mixes generic HTTP helpers with finance-specific business logic: ```typescript public GetAccountDetails(id: any) { ... } public GetTDSCategoryDetails(id: any) { ... } public CallCurrencyDetails(id: any) { ... } public GetAccountBalanceCheckNegative(criteria: any, id: any) { ... } public GetCurrencyDetail(CurrencyId: number, BaseCurrencyId: number, ...) { ... } public CostCenterDetails(criteria: any) { ... } ``` These methods belong in a finance-specific DBService, not in a library-wide shared service used by HR, Admin, and other non-finance modules. **Fix:** Move finance-specific methods to `projects/finance/service/` or the appropriate domain service. --- ### P1-5: `GbdirectivesService.GetAccountDetails()` API misuse ```typescript public GetAccountDetails(id: any){ let url = '/as/Account.svc/GetAccountDetailsSQL/?AccountId=' + id; return this.http.gbhttpget(url, id); // 'id' passed as 'isStoreRequired: boolean' } ``` The second parameter of `gbhttpget` is `isStoreRequired: boolean`, but a numeric `id` is passed. This likely causes the wrong caching behaviour silently. --- ### P1-6: All hardcoded English strings — zero Transloco usage in error messages Per CLAUDE.md, all display strings must use Transloco keys: | Component | Hardcoded String | |---|---| | `GbinputComponent` | `"Please provide a valid email address"` | | `GbinputComponent` | `"Please Enter Input for Translation"` | | `GbTextareaComponent` | `"Please Enter Remarks for Translation"` | | `GbTimeComponent` | `"Please enter Valid Time Format"` (×6 occurrences) | | `GbinputComponent` | `"Please enter a value between ${min} and ${max}."` | | `PicklistGridComponent` | `"First Character Should be a Alphabet"` (also grammatically wrong — should be "an") | | `PicklistGridComponent` | `"Selected Datas Exceeds MaxLength"` (grammatically wrong) | | `GbNewPicklistComponent` | `"First Character Should be a Alphabet"` | | `GbNewPicklistComponent` | `"Please Enter Input for Translation"` | --- ### P1-7: `GbDateComponent` has zero cleanup — `OnDestroy` not implemented Despite having subscriptions via `ngAfterViewInit`: ```typescript this.pickers.changes.subscribe(() => { // leaked this.initTodayButton(); }); this.initTodayButton(); // → registers picker.openedStream.subscribe() for each picker (leaked) ``` `GbDateComponent` does not implement `OnDestroy`. Every calendar instance that opens leaks a subscription to the picker's open stream. --- ### P1-8: `GbdynamiccomboboxComponent.ngOnDestroy()` never completes `destroy$` ```typescript private destroy$ = new Subject(); // ... used in: .pipe(takeUntil(this.destroy$)).subscribe(...) ngOnDestroy(): void { // ← body is empty! destroy$ is never called! } ``` The `takeUntil` protection is completely ineffective — subscriptions will not be cleaned up on component destroy. --- ### P1-9: `PicklistDbService` — magic number array with no explanation `gbpicklist.db.service.ts` lines 444–455: ```typescript const allowedIds = [ -1399999876, -1399999877, -1399999994, -1399999987, -1399999995, -1399999992, -1399999848, -1399999993, -1399999990, -1399999912, -1399999983, -1399999991, -1399999988, -1399999964, -1399999986, -1399999985, -1399999961, -1399999960, -1399999959, -1799999993 ]; ``` 20 magic negative integers with no names, comments, or documentation explaining what they represent or why they affect `JoinType`. **Fix:** Extract as named constants with documentation, ideally in a dedicated constants file. --- ### P1-10: `RemoveSpace()` utility duplicated 8+ times Identical implementation in every component: ```typescript public RemoveSpace(OriginalText: string): string { return OriginalText.toString().replace(/\s+/g, ''); } ``` Present in: `GbcomboboxComponent`, `GbradiobuttonComponent`, `GbpicklistComponent`, `GbNewPicklistComponent`, `GbdynamiccomboboxComponent`, `PicklistGridComponent`, `GbinputComponent`, `GbTextareaComponent`. **Fix:** Create a single utility function in a shared utils file. --- ### P1-11: `PicklistDbService.getPicklistData()` — 614 lines, deeply nested The single method covers all cases (filter/non-filter, grid/non-grid, various criteria formats, 4–6 element arrays). Cyclomatic complexity is extremely high. Nesting reaches 6 levels deep. This is impossible to unit test. **Fix:** Extract sub-methods: `buildSearchCriteria()`, `buildDependentCriteria()`, `buildFilterCriteria()`, `resolveFieldValue()`. --- ## P2 — Medium Priority ### P2-1: `GbinputComponent` stores every input in `localStorage` — privacy violation ```typescript private loadSuggestionsFromStorage(): void { const stored = localStorage.getItem(this.STORAGE_KEY); // ... } private saveSuggestionToStorage(value: string): void { localStorage.setItem(this.STORAGE_KEY, JSON.stringify(history)); } ``` Every `gb-input` with a `gbName` persists user input history to `localStorage`. This includes sensitive data (employee IDs, names, codes). Per CLAUDE.md, sensitive data must not be stored in localStorage. Users have no opt-out, no TTL on the data, and it persists across sessions. --- ### P2-2: `GbTimeComponent` time validation inconsistency - `OnBlurTimeChange()`: `parseInt(minutes) > 60` — allows "60:00" as valid - `onTimeChange()`: `parseInt(minutes) >= 60` — correctly rejects "60" - `ngOnChanges()` for `Date` type: `parseInt(minutes) > 60` — same bug as `OnBlurTimeChange` Minute validation is inconsistent across the 3 code paths. --- ### P2-3: `GbDateComponent` has two identical `setToday()` methods `setToday()` (line ~167) and `selectToday()` (line ~253) do identical operations. One is dead code. --- ### P2-4: `PicklistGridComponent.openPicklistDialog()` is dead code Lines 217–237: A full method to open the picklist dialog that is never called from the template or lifecycle hooks. Likely copy-paste left from a refactor. --- ### P2-5: `GbDirectivesModule` — anti-pattern in Angular 17+ All components in `GbDirectivesModule` are standalone (they have `imports: []` arrays). Wrapping standalone components in an NgModule prevents proper tree-shaking and is an Angular 17+ anti-pattern. The module should be removed; consumers should import components directly. --- ### P2-6: Two duplicate picklist components — `GbpicklistComponent` vs `GbNewPicklistComponent` Both components solve the same problem (picklist field with dialog grid). `GbNewPicklistComponent` is more refactored. The older `GbpicklistComponent` has more technical debt. Having two nearly identical components creates maintenance overhead and confusion. **Recommendation:** Audit consumers, migrate all to `GbNewPicklistComponent`, remove `GbpicklistComponent`. --- ### P2-7: `GbDateComponent.initTodayButton()` uses direct DOM manipulation with XSS risk **File:** `libs/gbdirectives/src/lib/gbdate/gbdate.component.ts` — `initTodayButton()` ```typescript const calendarContent = document.querySelector('.mat-datepicker-content'); const view = document.createElement('div'); view.innerHTML = `
`; view.querySelector('button')?.addEventListener('click', () => this.setToday()); calendarContent.appendChild(view); ``` **Security (reclassify from P2 → P0):** `this.Field.Name` and `this.gbName` come from `@Input()` or API-loaded field config. If an API response contains `" onmouseover="alert(1)` in a field name, it is injected directly into `innerHTML` and executes. Use `createElement`/`setAttribute`/`textContent` which are always safe: ```typescript const btn = document.createElement('button'); btn.setAttribute('data-cy', `${fieldName}-DatePickerToday`); btn.textContent = translocoService.translate('form.date.today'); ``` Additional issues: - `document.querySelector('.mat-datepicker-content')` is global — returns the wrong element when multiple date pickers are on the same page - `addEventListener` on DOM element is not cleaned up on destroy - The `ExampleHeader` component (in the same file, lines ~345–467) already provides a proper Angular datepicker header with "Go to Today" — `initTodayButton()` is completely redundant and should be removed --- ### P2-8: `PicklistGridComponent` imports unused `color` from d3 ```typescript import { color } from 'd3'; ``` `color` is never used in the file. This import bundles a portion of d3 into the picklist dialog module unnecessarily. --- ### P2-9: `GbdynamiccomboboxComponent` fires HTTP on every dropdown open ```typescript onDropdownOpened(isOpen: boolean): void { if (isOpen) { this.FetchPicklistData(true); // HTTP call every time dropdown opens ... } } ``` No caching check — if the data was already fetched, it still fires an HTTP call. In a form with multiple dynamic comboboxes, every time the user opens any dropdown, all other dropdowns that were previously opened will also re-fetch. **Fix:** Cache results and only fetch if data is stale or not yet loaded. --- ### P2-10: `GbaddonComponent.fetchPicklistData()` uses anti-pattern Promise wrapper ```typescript fetchPicklistData(url: string): Promise { return new Promise((resolve, reject) => { this.service.CommonGetService(url).subscribe({ next: (data) => resolve(data), error: (err) => reject(err), }); }); } ``` This is the "deferred anti-pattern". Use `firstValueFrom(observable)` from rxjs instead. --- ### P2-11: `new DatePipe("en-US")` instantiated inline multiple times | File | Location | |---|---| | `gbdate.component.ts` | `ngOnInit()`, `ngOnChanges()`, `formattedDate()` getter — 3 separate instantiations | | `gbaddon.component.ts` | `getDateValue()` — new instance on every call | Should be injected via `inject(DatePipe)` after providing it, or used as a singleton. --- ### P2-12: `GbpicklistComponent` / `GbNewPicklistComponent` have no `ChangeDetectionStrategy.OnPush` but call `cdr.detectChanges()` manually Both components inject `ChangeDetectorRef` and call `this.cdr.detectChanges()` at multiple points inside async callbacks. This is the wrong pattern — manual `detectChanges()` calls combined with Default CD means changes are detected twice (once automatically, once manually). --- ### P2-13: No RTL CSS across any gbdirectives SCSS files Despite Arabic language support being required, none of the SCSS files in `gbdirectives` contain `[dir="rtl"]` selectors. --- ### P2-14: All spec files are empty — zero test coverage Every component has a `.spec.ts` file. All contain empty `describe` blocks. The shared form field library — used by the entire application — has zero unit tests. --- ### P2-15: `GbInputComponent` `@HostListener('document:click')` fires for every click in the app ```typescript @HostListener('document:click', ['$event']) handleClickOutside(event: MouseEvent) { ... } ``` - Fires on every single click anywhere in the document regardless of component visibility - Not guarded by `Gbvisible()` - Runs even when the autocomplete dropdown has nothing to close - With many `gb-input` instances on a page, this multiplies the overhead per click **Fix:** Use `@HostListener` only when the suggestions dropdown is open (`showSuggestions()`). Alternatively, use a `clickOutside` directive pattern. --- ## Functional Issues ### F-1: `GbpicklistComponent.ngOnChanges()` wraps all logic in `setTimeout(100ms)` ```typescript ngOnChanges(changes: SimpleChanges): void { if (changes['FormData'] || ...) { setTimeout(() => { // all display update logic }, 100) } } ``` A 100ms async delay on every `FormData` change means visible lag in picklist field updates, especially in grids where rows change rapidly. --- ### F-2: `GbNewPicklistComponent.ngOnChanges()` has overlapping change handlers ```typescript if (changes['GridIndex']) { this.handleFormChanges() } if (changes['FormData'] && changes['GridIndex']) { setTimeout(() => this.handleFormChanges(), 300); // runs handleFormChanges TWICE } if (changes['FormData']) { setTimeout(() => this.handleFormChanges(), 100); // runs THIRD time } ``` When both `FormData` and `GridIndex` change simultaneously (common in grids), `handleFormChanges()` is called 3 times. --- ### F-3: `GbpicklistComponent.Showarchivefn()` deep-clones entire dialog data ```typescript public Showarchivefn(CheckBoxValue: any) { this.PicklistGridData = JSON.parse(JSON.stringify(this.PicklistGridData)) this.PicklistGridData.Field.ShowArchived = !this.PicklistGridData.Field.ShowArchived this.PicklistServiceCall() } ``` `JSON.parse(JSON.stringify(...))` on `PicklistGridData` loses any `Date` objects (converts to strings), functions, and undefined values. Use `structuredClone()` for shallow objects, or just mutate the specific property. --- ### F-4: `GbpicklistComponent` / `GbNewPicklistComponent` — `OnPicklistClose()` behavior is confusing ```typescript public OnPicklistClose(): void { console.log("this.searchText:", this.searchText) // P0 console.log const isEmptySearch = this.searchText === ""; if (isEmptySearch) { this.dialogRef.close(this.searchText); // closes with "" return; } if (this.PicklistGridData?.Field?.AddNew) { this.dialogRef.close(); // closes with undefined return; } this.dialogRef.close(this.searchText); // closes with current search text } ``` The close logic returns inconsistent values (`""`, `undefined`, or the search string) for the caller to handle, with no documented contract. --- ### F-5: `GbaddonComponent.InitiateFormAddon()` — async `forEach` anti-pattern ```typescript this.AddonDetails.forEach(async addon => { const PicklistData = await this.fetchPicklistData(picklistURL); // ... }); ``` `forEach` does not await async callbacks. All picklist fetches fire simultaneously but the code treats them as sequential. The `addonForm` is created immediately after `forEach` returns, before any fetch completes. --- ### F-6: `GbDateComponent` — the IST offset `19800000` is hardcoded `19800000` milliseconds = UTC+5:30 (India Standard Time). This offset is hardcoded in 6+ places in `gbdate.component.ts`: ```typescript this.value = "/Date(" + (this.convertToStartOfDay(GMTdate.getTime()) + 19800000).toString() + ")/"; ``` For users in other timezones this produces wrong date values. Should use `new Date().getTimezoneOffset()` dynamically, or let the server handle timezone normalization. --- ### F-7: `GbNewPicklistComponent.onFocusChange()` has hardcoded field name exclusions ```typescript if (...this.Field.Name != 'CostingNumberEnquiry' && this.Field.Name != 'EmployeeBloodGroup' && this.Field.Name != 'CallStatusName') { this.OnPicklistClick(false); } ``` Business logic ("don't auto-search for these specific field names") is hardcoded inside a shared library component. This creates tight coupling and is not discoverable. Should be an `@Input() skipAutoSearch: boolean` or part of the field config. --- ## Architecture / Maintainability ### A-1: `GbDirectivesModule` is a legacy pattern for Angular 17+ The `NgModule` wraps standalone components. This prevents tree-shaking since importing the module includes all 13+ components regardless of which ones are used. Consumers should import individual standalone components. --- ### A-2: `Idirectives.model.ts` has evolved into a God interface `IField` has 90+ properties covering every possible field configuration. Most are optional. There is no documentation explaining when each property applies. Suggest splitting into: - `IBaseField` — shared across all field types - `IPicklistFieldConfig` — picklist-specific - `IGridFieldConfig` — grid-specific - `IValidationConfig` — validation-specific --- ### A-3: `IGrid` has properties that duplicate `IField` Many properties exist in both `IField` and `IGrid` (`Required`, `ReadOnly`, `DefaultValue`, `MaxLength`, etc.) with slightly different semantics. This creates confusion about which applies where and opportunities for inconsistent behaviour. --- ### A-4: `PicklistURL` is imported as a module-level constant in multiple components ```typescript import { PicklistURL } from 'projects/gbhost/public/picklist/picklisturl'; ``` `PicklistURL` appears to be a large URL-mapping object imported into `GbpicklistComponent`, `GbNewPicklistComponent`, `GbdynamiccomboboxComponent`. This creates a compile-time coupling from the shared library to the host project's assets, making the library non-portable. --- ## Summary Table | # | Severity | Component / File | Issue | |---|---|---|---| | P0-1 | Critical | All components | Missing `OnPush` CD strategy | | P0-2 | Critical | `PicklistDbService:17` | `sessionStorage.getItem('LoginDTO')` | | P0-3 | Critical | 8 components | Untracked setTimeout — memory leaks | | P0-4 | Critical | 5 components | Raw `.subscribe()` without cleanup | | P0-5 | Critical | `GbDate`, `PicklistGrid` | `console.log` in production | | P0-6 | Critical | 11 dialog open sites | Fixed `width: 'Xpx'` — no `min()` | | P0-7 | Critical | `GbdynamiccomboboxComponent` | HTTP call in `writeValue()` | | P0-8 | Critical | `GbaddonComponent` | Circular import via `GbDirectivesModule` | | P1-1 | High | All components | Mixed constructor + `inject()` pattern | | P1-2 | High | Everywhere | `any` types — no type safety | | P1-3 | High | `Idirectives.model.ts:1` | Dead `DayBook` import from finance MFE | | P1-4 | High | `GbdirectivesService` | Finance methods in shared library service | | P1-5 | High | `GbdirectivesService` | `id` passed as `boolean` in `gbhttpget` | | P1-6 | High | All components | Hardcoded English error strings | | P1-7 | High | `GbDateComponent` | Missing `ngOnDestroy` — subscription leaks | | P1-8 | High | `Gbdynamiccombobox` | `ngOnDestroy` empty — `destroy$` never completed | | P1-9 | High | `PicklistDbService` | 20 magic numbers with no explanation | | P1-10 | High | 8 components | `RemoveSpace()` duplicated 8× | | P1-11 | High | `PicklistDbService` | 614-line method, impossible to test | | P2-1 | Medium | `GbinputComponent` | User input stored in `localStorage` | | P2-2 | Medium | `GbTimeComponent` | Inconsistent minute validation (`>60` vs `>=60`) | | P2-3 | Medium | `GbDateComponent` | Duplicate `setToday()` / `selectToday()` | | P2-4 | Medium | `PicklistGridComponent` | Dead `openPicklistDialog()` method | | P2-5 | Medium | `GbDirectivesModule` | NgModule wrapping standalone components | | P2-6 | Medium | Library-wide | Two duplicate picklist implementations | | P2-7 | **P0 Security** | `GbDateComponent` | `innerHTML` with `Field.Name` — XSS; also DOM manipulation, redundant vs ExampleHeader | | P2-8 | Medium | `PicklistGridComponent` | Unused `d3` import in picklist | | P2-9 | Medium | `Gbdynamiccombobox` | HTTP on every dropdown open | | P2-10 | Medium | `GbaddonComponent` | Promise/subscribe anti-pattern | | P2-11 | Medium | `GbDate`, `GbAddon` | `new DatePipe("en-US")` inline | | P2-12 | Medium | `GbpicklistComponent` | Manual `cdr.detectChanges()` in Default CD | | P2-13 | Medium | All SCSS files | No RTL CSS for Arabic | | P2-14 | Medium | All spec files | Zero unit tests | | P2-15 | Medium | `GbinputComponent` | `document:click` listener on every instance | | F-1 | Functional | `GbpicklistComponent` | 100ms delay in `ngOnChanges` | | F-2 | Functional | `GbNewPicklistComponent` | Triple-call `handleFormChanges()` on change | | F-3 | Functional | `PicklistGridComponent` | `JSON.parse/stringify` loses Dates | | F-4 | Functional | `PicklistGridComponent` | Inconsistent `OnPicklistClose` return values | | F-5 | Functional | `GbaddonComponent` | async `forEach` anti-pattern | | F-6 | Functional | `GbDateComponent` | IST offset `19800000` hardcoded (timezone bug) | | F-7 | Functional | `GbNewPicklistComponent` | Hardcoded field name exclusions | | A-1 | Architecture | `GbDirectivesModule` | Legacy NgModule anti-pattern | | A-2 | Architecture | `Idirectives.model.ts` | God interface — 90+ properties | | A-3 | Architecture | `IField` / `IGrid` | Property duplication between interfaces | | A-4 | Architecture | 3 components | Hard dependency on host project's picklist assets | --- ## Recommended Fix Priority Order 1. **Immediate (before next release):** - Remove `console.log` calls (GbDate, PicklistGrid) - Fix `ngOnDestroy` in `GbdynamiccomboboxComponent` (complete `destroy$`) - Add `ngOnDestroy` to `GbDateComponent` - Remove HTTP call from `GbdynamiccomboboxComponent.writeValue()` - Fix `GbdirectivesService.GetAccountDetails()` parameter bug 2. **Sprint 1:** - Add `ChangeDetectionStrategy.OnPush` to all 13 components - Fix all `MatDialog` widths to `min(Xpx, 95vw)` - Replace `sessionStorage.getItem('LoginDTO')` with auth service signal - Store and clear all `setTimeout` handles - Add `takeUntil` to all raw subscriptions 3. **Sprint 2:** - Translate all error strings to Transloco keys - Remove localStorage autocomplete feature from `GbinputComponent` (or make opt-in) - Fix IST timezone hardcoding in `GbDateComponent` - Remove dead `DayBook` import from model file - Fix the minute validation inconsistency in `GbTimeComponent` - Remove `openPicklistDialog()` dead code from `PicklistGridComponent` 4. **Sprint 3:** - Consolidate `GbpicklistComponent` into `GbNewPicklistComponent` - Break up `PicklistDbService.getPicklistData()` into smaller methods - Move finance methods out of `GbdirectivesService` - Replace `GbDirectivesModule` with direct standalone component exports - Add RTL CSS to all SCSS files - Add unit tests for service layer 5. **Long-term:** - Split `IField` God interface into focused interfaces - Name the magic number constants in `PicklistDbService` - Remove `PicklistURL` host-project coupling from library - Consolidate `RemoveSpace()` into shared utility