feat: theme upgrade to primeng_18 #1399
Conversation
WalkthroughThis update systematically migrates the codebase from using PrimeNG's Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AngularComponent
participant PrimeNGSelect
participant FormControl
participant ManualChipHandler
User->>AngularComponent: Interacts with <p-select> (was <p-dropdown>)
AngularComponent->>PrimeNGSelect: Binds options, handles selection
PrimeNGSelect-->>AngularComponent: Emits selection event
AngularComponent->>FormControl: Updates form value
User->>ManualChipHandler: Types value, presses comma/Enter or blurs field
ManualChipHandler->>FormControl: Adds chip value to form array
ManualChipHandler->>InputElement: Clears input
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–25 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
🔁 Code Duplication Report - Angular
🎉 This PR reduces duplicated code by 0.34%! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (4)
src/app/shared/components/input/dropdown/dropdown.component.ts (1)
5-9: Inconsistent naming between selector and component class.The selector has been updated to
app-selectbut the component class remainsDropdownComponent, and the template/style file paths still referencedropdown.component.*. This creates naming inconsistency that could confuse developers.Consider renaming the component class and files for consistency:
-export class DropdownComponent { +export class SelectComponent {Also consider renaming the files:
dropdown.component.html→select.component.htmldropdown.component.scss→select.component.scssAnd update the component decorator accordingly:
@Component({ selector: 'app-select', - templateUrl: './dropdown.component.html', - styleUrls: ['./dropdown.component.scss'] + templateUrl: './select.component.html', + styleUrls: ['./select.component.scss'] })src/app/shared/components/menu/main-menu/main-menu.component.ts (1)
78-78: Remove invalid.clear()call on PrimeNG Select componentThe PrimeNG
<p-select>component does not expose aclear()method. You have two options to restore the intended “clear” behavior:• Replace imperatively clearing with a model reset
- File:
src/app/shared/components/menu/main-menu/main-menu.component.ts
Line: ~78- Remove:
this.pDropdown()?.clear();- Add (for template-driven or reactive forms):
// If using NgModel this.selectedValue = null; // Or if using a FormControl this.myForm.get('selectControl')?.setValue(null);- In your template, enable the clear icon:
<p-select [options]="dropdownOptions" [(ngModel)]="selectedValue" [showClear]="true"> </p-select>• Or switch to the PrimeNG Dropdown component
- Update your ViewChild to import and reference
Dropdownfrom'primeng/dropdown'- Use
@ViewChild(Dropdown)and thenthis.pDropdown?.clear()will work as expected.Please choose the approach that best fits your use case.
src/app/shared/components/helper/mapping/generic-mapping-table/generic-mapping-table.component.html (1)
18-30: Replace invalid resetFilter() call in p-selectThe
resetFilter()method no longer exists on PrimeNG v18’s<p-select>and will throw at runtime. You’ll need to remove that handler and use the officially supported clear mechanism.• File: src/app/shared/components/helper/mapping/generic-mapping-table/generic-mapping-table.component.html
– Remove line 28:
(onFocus)="destinationAttributes.resetFilter()"
– Add[showClear]="true"to your<p-select>tag to expose a clear icon that resets both the selection and the filter input.
– If you still need to auto-clear the filter when the panel opens, you can use an (onShow) workaround (undocumented):
(onShow)="destinationAttributes.filterValue = ''"<p-select #destinationAttributes appendTo="body" (onShow)="tableDropdownWidth()" - (onFocus)="destinationAttributes.resetFilter()" + [showClear]="true" [ngModel]="getDropdownValue(mapping)" [options]="destinationOptions" …>After making this change, please smoke-test the dropdown to confirm filtering and clear behavior.
src/app/shared/components/helper/mapping/mapping-filter/mapping-filter.component.html (1)
10-20: Inconsistent i18n key namespace – violates component-level guidelineMost strings in this template use the
mappingFilter.*namespace, but the select’s placeholder usesmapping.filterPlaceholder. Guidelines require a single top-level object per component; mixingmappingandmappingFilterbreaks that rule and complicates translation maintenance.Rename the key (and update translation files) so that all keys live under
mappingFilter.- [placeholder]="'mapping.filterPlaceholder' | transloco" + [placeholder]="'mappingFilter.filterPlaceholder' | transloco"
♻️ Duplicate comments (5)
src/app/shared/components/si/helper/dashboard-mapping-resolve/dashboard-mapping-resolve.component.html (1)
40-47: Duplicate of previous event-API concernThe employee mapping select uses the same
(onShow)output; audit and update as above.src/app/shared/components/configuration/configuration-import-field/configuration-import-field.component.html (1)
133-142: Duplicate selector issue already raisedSame nested-selector pattern (
.p-select-label .p-placeholder .p-select-trigger) as in the mapping-fields stylesheet – once fixed there, consider applying a shared mixin/utility to avoid drift.src/app/shared/components/menu/main-menu/main-menu.component.scss (1)
11-13: Duplicate selector issueThe nested selector chain suffers from the same DOM-mismatch highlighted in
configuration-mapping-fields.component.scss. Please adjust accordingly.src/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.scss (1)
49-63: Duplicate selector issueSame over-specific selector; see earlier comment for suggested correction.
src/app/shared/components/si/helper/skip-export/skip-export.component.html (1)
119-131: Same form-API mixup for the second conditionReplicate the fix applied to the first autocomplete.
🧹 Nitpick comments (17)
src/app/integrations/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.component.ts (1)
460-461: Method name is now misleading—consider renaming to match<p-select>usageOnly the comment was updated, but the handler is still called
onDropdownChange, which can confuse future readers now that the UI element is a<p-select>.
Renaming the method (and its template binding) toonSelectChangekeeps the code self-describing.src/app/shared/components/menu/main-menu/main-menu.component.ts (1)
45-45: Consider renaming property for clarity.The ViewChild has been correctly updated to use
Select, but the property namepDropdownis now misleading since it references a Select component.-private pDropdown = viewChild(Select); +private pSelect = viewChild(Select);And update the usage on line 78:
-this.pDropdown()?.clear(); +this.pSelect()?.clear();src/app/shared/components/helper/paginator/paginator.component.scss (2)
1-3: Consider reducing reliance on::ng-deepfor long-term maintainability
::ng-deepis already deprecated and may be removed in future Angular versions. Whenever possible, migrate these deep selectors to:• A global stylesheet (e.g. styles.scss)
• An explicit host class exported by the componentThis keeps style encapsulation intact and avoids future breakage.
13-15: Hard-coded right-padding may truncate long labelsThe
tw-pr-10-pxutility forces a fixed padding on the selected label. If labels can be longer in certain locales (i18n) this may clip text when combined with the fixed width in line 2. Consider usingtw-pr-xxonly when truncation is acceptable or switch totw-pr-with responsive breakpoints.src/stories/CloneSettingField.stories.ts (1)
30-30: Import array ordering – move heavy modules last for faster HMRPlacing
BrowserAnimationsModuleandSharedModulebefore feature modules lets Storybook’s module federation tree-shake unused PrimeNG code faster. Minor but free perf win.src/app/integrations/qbd/qbd-main/qbd-mapping/qbd-generic-mapping/qbd-generic-mapping.component.scss (1)
6-16: Avoid relying on::ng-deep– consider a safer scoping strategy
::ng-deepis deprecated and may be removed in future Angular versions. Relying on it for all PrimeNG-specific overrides jeopardises forward-compatibility and leaks styles globally. Prefer one of:
- Move these rules to a global stylesheet (e.g.
src/styles.scss) and drop::ng-deep.- Wrap the overrides with the new
:host-context()selector if the styles must stay component-scoped.- Leverage PrimeNG’s style APIs (CSS variables, theming) to avoid deep selectors altogether.
Upgrading to PrimeNG 18 is a good moment to pay this debt down.
src/app/integrations/netsuite/netsuite-shared/netsuite-import-settings/netsuite-custom-segment-dialog/netsuite-custom-segment-dialog.component.scss (1)
1-20: Same::ng-deepcaveat applies here
::ng-deepis again used for every.p-selectoverride. Consider migrating to a safer scoping strategy (global stylesheet,:host-context, or PrimeNG theme variables) to avoid future breakage.src/app/shared/components/si/helper/dashboard-mapping-resolve/dashboard-mapping-resolve.component.ts (1)
48-50: DOM query selector is brittle after component renameThe hard-coded selector
.p-select-panel.p-component.ng-star-insertedassumes PrimeNG’s internal class names and that the overlay is already in the DOM at method call time. Any future CSS class change or timing issue will silently break the width adjustment.Consider subscribing to
onShowfromp-select, and use the overlay element passed in the callback instead ofdocument.querySelector, or use PrimeNG’sOverlayServiceto obtain the panel reference.-const element = document.querySelector('.p-select-panel.p-component.ng-star-inserted') as HTMLElement; +const overlay = event?.overlay as HTMLElement; // triggered via (onShow) +if (overlay) { + overlay.style.width = '300px'; +}src/app/shared/components/onboarding/clone-setting/clone-setting-field/clone-setting-field.component.html (1)
11-20: Align enum terminology with the new component nameThe template now renders
<app-select>but still checks forInputType.DROPDOWN. Keeping the old enum label after the migration is a small cognitive mismatch and can trip up future readers.- *ngIf="inputType === InputType.DROPDOWN" + *ngIf="inputType === InputType.SELECT"(or rename the enum value itself) – either way, tidying up the naming keeps intent crystal-clear.
src/styles.scss (1)
77-85: Keep legacy selector until full rollout is confirmedThe icon override now targets
.p-select-trigger. If any screens still render a legacy.p-dropdown-trigger(e.g. in unfinished feature flags) the chevron will disappear. Consider a short-lived transition rule that covers both classes:.p-dropdown-trigger, .p-select-trigger { … }Remove once the migration is 100 % rolled out.
src/app/shared/components/input/dropdown/dropdown.component.scss (1)
1-48: File/component naming is now misleadingAll style rules moved to
.p-select, yet the component is still called dropdown. A simple rename toselect.component.*(class, selector & folder) will prevent future confusion and improve searchability.src/app/shared/components/configuration/configuration-connector/configuration-connector.component.html (1)
25-41: Avoid binding a constant string tooptionLabel
[optionLabel]="'value'"incurs the cost of change-detection with zero benefit.
Use a plain attribute instead:-<p-select appendTo="body" [options]="accountingCompanyList" [optionLabel]="'value'" +<p-select appendTo="body" [options]="accountingCompanyList" optionLabel="value"Same applies to every other
p-selectinstance that binds a hard-coded string.src/app/integrations/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.component.html (1)
78-80: StaticoptionLabel/optionValueshould be literal attributesMultiple
p-selecttags bind a constant string:[p-select ...] [optionLabel]="'display_name'"Prefer plain attributes to cut unnecessary bindings:
- [optionLabel]="'display_name'" + optionLabel="display_name"Apply to every occurrence in this template (tax group, Sage Intacct fields, Fyle fields, cost code/type selectors).
Also applies to: 106-108, 116-118, 134-135, 155-165, 174-182
src/app/shared/components/helper/mapping/generic-mapping-table/generic-mapping-table.component.ts (1)
100-110: Accessing internal PrimeNG DOM structure is brittleHard-coding
.p-select-panel.p-component.ng-star-insertedcouples the app to PrimeNG’s private markup. Minor library updates can silently break this selector.Prefer the
onShowoutput of<p-select>with a template reference variable, oroverlayServiceevents, to obtain the panel element without query-selecting internal class names.At minimum, drop the
.ng-star-insertedclass from the selector:-const element = document.querySelector('.p-select-panel.p-component.ng-star-inserted') as HTMLElement; +const element = document.querySelector('.p-select-panel.p-component') as HTMLElement;src/app/shared/components/configuration/configuration-mapping-fields/configuration-mapping-fields.component.scss (1)
5-19: Over-specific selector is likely ineffective with the newp-selectDOM
p-selectrenders.p-select-labeland.p-select-triggeras siblings inside the root.p-select.
The selector chain.p-select .p-select-label .p-placeholder .p-select-triggerassumes the trigger sits inside the placeholder element, which is no longer true and therefore never matches.-:host ::ng-deep .p-select .p-select-label .p-placeholder .p-select-trigger { … } +:host ::ng-deep .p-select.p-placeholder .p-select-trigger { … }Same pattern is repeated in the file for hidden/visible chevron rules – consider revising all of them to match the actual markup to avoid bloated CSS and broken visuals.
src/app/shared/components/si/helper/skip-export/skip-export.component.html (1)
63-65: Minor: empty body tagThe closing
</p-select>tag is on the next line but has no template body; consider self-closing<p-select ... />to keep markup terse.src/app/shared/components/configuration/configuration-skip-export/configuration-skip-export.component.ts (1)
341-350: Consider improving type safety and event handling.The keyboard event handling logic is correct, but there are opportunities for improvement:
- Use more specific TypeScript types instead of
any- Add null checks for target element
- Consider extracting the key list to a constant
+ private readonly CHIP_SEPARATOR_KEYS = [',', 'Enter'] as const; + - onKeyDown(event: any, fieldNumber: number): void { + onKeyDown(event: KeyboardEvent, fieldNumber: number): void { - const keyboardEvent = event as KeyboardEvent; - const target = keyboardEvent.target as HTMLInputElement; + const target = event.target as HTMLInputElement; + if (!target) return; + - if ((keyboardEvent.key === ',' || keyboardEvent.key === 'Enter') && target.value.trim()) { + if (this.CHIP_SEPARATOR_KEYS.includes(event.key as any) && target.value.trim()) { - keyboardEvent.preventDefault(); + event.preventDefault(); this.addChip(target.value.trim(), target, fieldNumber); } }
| provideAnimationsAsync(), | ||
| providePrimeNG({ | ||
| theme: { | ||
| preset: Lara, | ||
| options: { | ||
| darkModeSelector: false, | ||
| cssLayer: false, | ||
| }, | ||
| } | ||
| }), |
There was a problem hiding this comment.
Fix trailing comma ESLint violations.
The PrimeNG configuration is functionally correct, but there are trailing comma issues that need to be resolved.
Apply this diff to fix the linting errors:
providePrimeNG({
theme: {
preset: Lara,
options: {
darkModeSelector: false,
- cssLayer: false,
- },
- }
+ cssLayer: false
+ }
+ }
}),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| provideAnimationsAsync(), | |
| providePrimeNG({ | |
| theme: { | |
| preset: Lara, | |
| options: { | |
| darkModeSelector: false, | |
| cssLayer: false, | |
| }, | |
| } | |
| }), | |
| provideAnimationsAsync(), | |
| providePrimeNG({ | |
| theme: { | |
| preset: Lara, | |
| options: { | |
| darkModeSelector: false, | |
| cssLayer: false | |
| } | |
| } | |
| }), |
🧰 Tools
🪛 GitHub Check: lint
[failure] 55-55:
Unexpected trailing comma
[failure] 54-54:
Unexpected trailing comma
🪛 GitHub Actions: TypeScript Lint Check
[error] 54-54: ESLint: Unexpected trailing comma (comma-dangle)
🤖 Prompt for AI Agents
In src/app/app.module.ts between lines 48 and 57, remove the trailing commas
after the last properties in the PrimeNG configuration object to fix ESLint
trailing comma violations. Ensure that no comma follows the last item in each
nested object or array within the providePrimeNG call.
| :host ::ng-deep .selectedItem .p-select-trigger, :host ::ng-deep .selectedAccItem .p-select-trigger{ | ||
| @apply tw-hidden #{!important}; | ||
| } | ||
|
|
||
| :host ::ng-deep .selectedAccItem .p-dropdown.p-disabled { | ||
| :host ::ng-deep .selectedAccItem .p-select.p-disabled { | ||
| @apply tw-bg-input-read-only-bg tw-opacity-100 tw-border tw-border-input-read-only-border #{!important} | ||
| } | ||
|
|
||
| :host ::ng-deep .selectedAccItem .p-dropdown.p-disabled .p-dropdown-label { | ||
| :host ::ng-deep .selectedAccItem .p-select.p-disabled .p-select-label { | ||
| @apply tw-text-input-read-only-text #{!important}; | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Selector change may miss the trigger element
p-select renders the caret inside .p-select-trigger inside the .p-select.
The previous rule hid .p-dropdown-trigger; the updated rule tries to hide .p-select-trigger, but the extra descendant space (.selectedItem .p-select-trigger) and the absence of the parent .p-select level can break specificity.
-:host ::ng-deep .selectedItem .p-select-trigger
+:host ::ng-deep .selectedItem .p-select .p-select-triggerTighten the selector to guarantee the trigger stays hidden for the selected rows.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| :host ::ng-deep .selectedItem .p-select-trigger, :host ::ng-deep .selectedAccItem .p-select-trigger{ | |
| @apply tw-hidden #{!important}; | |
| } | |
| :host ::ng-deep .selectedAccItem .p-dropdown.p-disabled { | |
| :host ::ng-deep .selectedAccItem .p-select.p-disabled { | |
| @apply tw-bg-input-read-only-bg tw-opacity-100 tw-border tw-border-input-read-only-border #{!important} | |
| } | |
| :host ::ng-deep .selectedAccItem .p-dropdown.p-disabled .p-dropdown-label { | |
| :host ::ng-deep .selectedAccItem .p-select.p-disabled .p-select-label { | |
| @apply tw-text-input-read-only-text #{!important}; | |
| } | |
| :host ::ng-deep .selectedItem .p-select .p-select-trigger, :host ::ng-deep .selectedAccItem .p-select-trigger { | |
| @apply tw-hidden #{!important}; | |
| } | |
| :host ::ng-deep .selectedAccItem .p-select.p-disabled { | |
| @apply tw-bg-input-read-only-bg tw-opacity-100 tw-border tw-border-input-read-only-border #{!important} | |
| } | |
| :host ::ng-deep .selectedAccItem .p-select.p-disabled .p-select-label { | |
| @apply tw-text-input-read-only-text #{!important}; | |
| } |
🤖 Prompt for AI Agents
In
src/app/shared/components/configuration/configuration-import-field/configuration-import-field.component.scss
between lines 81 and 92, the CSS selector for hiding the trigger element is too
loose and misses the parent .p-select level, which can cause the trigger to
remain visible. Update the selector to include the .p-select parent before
.p-select-trigger, removing the extra descendant space, to ensure the trigger
element is properly targeted and hidden for selected rows.
| onBlur(event: any, fieldNumber: number): void { | ||
| const target = event.target as HTMLInputElement; | ||
| // Replaces [addOnBlur]="true" functionality | ||
| if (target.value.trim()) { | ||
| this.addChip(target.value.trim(), target, fieldNumber); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null safety check for the target element.
The blur event handler should include a null check to prevent potential runtime errors.
- onBlur(event: any, fieldNumber: number): void {
+ onBlur(event: FocusEvent, fieldNumber: number): void {
const target = event.target as HTMLInputElement;
+ if (!target) return;
+
// Replaces [addOnBlur]="true" functionality
if (target.value.trim()) {
this.addChip(target.value.trim(), target, fieldNumber);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onBlur(event: any, fieldNumber: number): void { | |
| const target = event.target as HTMLInputElement; | |
| // Replaces [addOnBlur]="true" functionality | |
| if (target.value.trim()) { | |
| this.addChip(target.value.trim(), target, fieldNumber); | |
| } | |
| } | |
| onBlur(event: FocusEvent, fieldNumber: number): void { | |
| const target = event.target as HTMLInputElement; | |
| if (!target) return; | |
| // Replaces [addOnBlur]="true" functionality | |
| if (target.value.trim()) { | |
| this.addChip(target.value.trim(), target, fieldNumber); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/app/shared/components/configuration/configuration-skip-export/configuration-skip-export.component.ts
around lines 352 to 358, add a null safety check for the target element in the
onBlur method to prevent runtime errors. Before accessing target.value, verify
that event.target is not null or undefined, and only proceed if it is valid.
This ensures the method safely handles cases where the event target might be
missing.
| private addChip(value: string, inputElement: HTMLInputElement, fieldNumber: number): void { | ||
| const formControlName = fieldNumber === 1 ? 'value1' : 'value2'; | ||
| const currentValues = this.skipExportForm.get(formControlName)?.value || []; | ||
|
|
||
| // Avoid duplicates | ||
| if (!currentValues.includes(value)) { | ||
| const newValues = [...currentValues, value]; | ||
| this.skipExportForm.get(formControlName)?.setValue(newValues); | ||
| inputElement.value = ''; // Clear input after adding | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and validation for form control operations.
The addChip method should include error handling for form control operations and input validation.
private addChip(value: string, inputElement: HTMLInputElement, fieldNumber: number): void {
+ if (!value || !inputElement) return;
+
const formControlName = fieldNumber === 1 ? 'value1' : 'value2';
- const currentValues = this.skipExportForm.get(formControlName)?.value || [];
+ const formControl = this.skipExportForm.get(formControlName);
+ if (!formControl) return;
+
+ const currentValues = formControl.value || [];
// Avoid duplicates
if (!currentValues.includes(value)) {
const newValues = [...currentValues, value];
- this.skipExportForm.get(formControlName)?.setValue(newValues);
+ formControl.setValue(newValues);
inputElement.value = ''; // Clear input after adding
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private addChip(value: string, inputElement: HTMLInputElement, fieldNumber: number): void { | |
| const formControlName = fieldNumber === 1 ? 'value1' : 'value2'; | |
| const currentValues = this.skipExportForm.get(formControlName)?.value || []; | |
| // Avoid duplicates | |
| if (!currentValues.includes(value)) { | |
| const newValues = [...currentValues, value]; | |
| this.skipExportForm.get(formControlName)?.setValue(newValues); | |
| inputElement.value = ''; // Clear input after adding | |
| } | |
| } | |
| private addChip(value: string, inputElement: HTMLInputElement, fieldNumber: number): void { | |
| if (!value || !inputElement) return; | |
| const formControlName = fieldNumber === 1 ? 'value1' : 'value2'; | |
| const formControl = this.skipExportForm.get(formControlName); | |
| if (!formControl) return; | |
| const currentValues = formControl.value || []; | |
| // Avoid duplicates | |
| if (!currentValues.includes(value)) { | |
| const newValues = [...currentValues, value]; | |
| formControl.setValue(newValues); | |
| inputElement.value = ''; // Clear input after adding | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/app/shared/components/configuration/configuration-skip-export/configuration-skip-export.component.ts
around lines 360 to 370, the addChip method lacks error handling and input
validation for form control operations. Add checks to ensure the form control
exists before accessing or setting its value, and validate the input value to
prevent adding empty or invalid entries. Include try-catch blocks or conditional
guards to handle potential errors gracefully during form control operations.
| import { FormControl, FormGroup } from '@angular/forms'; | ||
| import { TranslocoService } from '@jsverse/transloco'; | ||
| import { DropdownFilterOptions } from 'primeng/dropdown'; | ||
| import { SelectFilterOptions } from 'primeng/select'; |
There was a problem hiding this comment.
SelectFilterOptions is imported but never used
This will raise a TypeScript unused-import error and break the build.
-import { SelectFilterOptions } from 'primeng/select';Remove the import or wire it where it is actually required.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { SelectFilterOptions } from 'primeng/select'; |
🤖 Prompt for AI Agents
In
src/app/shared/components/helper/mapping/generic-mapping-table/generic-mapping-table.component.ts
at line 4, the import SelectFilterOptions from 'primeng/select' is not used
anywhere in the file. To fix the TypeScript unused-import error, remove this
import statement entirely from the file.
| <p-select appendTo="body" (onChange)="patchFormValue($event)" class="tw-pl-7" *ngIf="!locationEntity?.location_entity_name" [placeholder]="'intacctLocationEntity.selectLocationEntity' | transloco: { locationEntity: locationEntityLabel }" formControllerName="locationEntity" [options]="locationEntityOptions" optionLabel="value"> | ||
| </p-select> |
There was a problem hiding this comment.
Typo – formControllerName should be formControlName
Angular’s reactive-forms directive is formControlName. The current typo prevents the control from registering, so validation and value updates will fail.
-<p-select … formControllerName="locationEntity" …>
+<p-select … formControlName="locationEntity" …>🤖 Prompt for AI Agents
In
src/app/shared/components/si/core/intacct-location-entity/intacct-location-entity.component.html
at lines 36 to 37, the attribute formControllerName is incorrectly used instead
of formControlName. Replace formControllerName with formControlName to correctly
bind the form control, enabling proper validation and value updates in the
Angular reactive form.
| <p-autocomplete | ||
| [multiple]="true" | ||
| [typeahead]="false" | ||
| [forceSelection]="false" | ||
| styleClass="tw-w-[296px]" | ||
| [(ngModel)]="valueOption1" | ||
| formControlName="value1" | ||
| [placeholder]="'skipExport.enterValue' | transloco" | ||
| [suggestions]="[]" | ||
| (onKeyDown)="onKeyDown($event, 1)" | ||
| (onBlur)="onBlur($event, 1)"> | ||
| </p-autocomplete> | ||
| <small class="chip-hint"><br>{{ 'skipExport.chipHint' | transloco }}</small> |
There was a problem hiding this comment.
ngModel and formControlName used together – this will throw NG0120
Angular prohibits using template-driven ([(ngModel)]) and reactive (formControlName) APIs on the same control. The new <p-autocomplete> has both:
[(ngModel)]="valueOption1" formControlName="value1"At runtime the form will error and the control won’t behave predictably.
Suggested fix – drop ngModel and drive everything through the reactive form:
- [(ngModel)]="valueOption1"
+ (ngModelChange)="valueOption1 = $event" ngModelOptions="{standalone: true}"or (preferred) remove ngModel entirely and patch the control inside addChip():
- [(ngModel)]="valueOption1"Remember to update addChip() to this.skipExportForm.get('value1')?.setValue([...]).
🤖 Prompt for AI Agents
In src/app/shared/components/si/helper/skip-export/skip-export.component.html
lines 48 to 60, the <p-autocomplete> component incorrectly uses both [(ngModel)]
and formControlName on the same control, causing Angular NG0120 errors. To fix
this, remove the [(ngModel)] binding entirely and manage the form control value
exclusively through the reactive form. Update the addChip() method to patch the
form control value using this.skipExportForm.get('value1')?.setValue([...])
instead of modifying the ngModel variable.
| import { ToggleSwitchModule } from 'primeng/toggleswitch'; | ||
| import { RadioButtonModule } from 'primeng/radiobutton'; | ||
| import { ToastModule } from 'primeng/toast'; | ||
| import { InputTextModule } from 'primeng/inputtext'; | ||
| import { DropdownModule } from 'primeng/dropdown'; | ||
| import { SelectModule } from 'primeng/select'; | ||
| import { DialogModule } from 'primeng/dialog'; |
There was a problem hiding this comment.
ToggleSwitchModule added but templates still use <p-inputSwitch> – build will break
All templates (e.g. intacct-import-settings.component.html lines 46, 60, 74, 139) still reference <p-inputSwitch>.
PrimeNG 18 replaces it with <p-toggleSwitch>. Either:
- Revert to
InputSwitchModuleor - Rename every
<p-inputSwitch>to<p-toggleSwitch>and adjust event names (onChange⇒onChange, API unchanged).
Failing to do so will cause a template compile error.
🤖 Prompt for AI Agents
In src/app/shared/shared.module.ts between lines 5 and 10, the code imports
ToggleSwitchModule but the templates still use the old <p-inputSwitch>
component, which will cause build errors. To fix this, either revert the import
to InputSwitchModule to match the existing template usage or update all template
files (e.g., intacct-import-settings.component.html lines 46, 60, 74, 139) to
replace <p-inputSwitch> with <p-toggleSwitch> and adjust any event bindings
accordingly. Ensure consistency between the imported module and the component
tags used in templates.
1 participant
Description
theme upgrade to primeng_18
Clickup
https://app.clickup.com/1864988/v/l/li/901607316229
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores