-
Notifications
You must be signed in to change notification settings - Fork 0
feat: migrate all dropdowns to a central component #1489
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Warning Rate limit exceeded@JustARatherRidiculouslyLongUsername has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 23 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (6)
WalkthroughRefactors the dropdown into a dual-mode ControlValueAccessor component (form-backed or standalone), adds many configurable inputs/outputs, dynamic templates (items, selected, filter, icons), tooltip/clear actions, brandable styling and size/error/loading variants, and a translation for empty results. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Form as Angular Form
participant Comp as DropdownComponent (CVA)
participant View as PrimeNG Dropdown
participant i18n as Transloco
Note over Comp,i18n: Init
Comp->>i18n: translate('dropdown.noResultsFound')
i18n-->>Comp: "No results found"
Comp->>View: bind inputs/templates, register NG_VALUE_ACCESSOR
alt Form-backed usage
Form->>Comp: registerOnChange/registerOnTouched
User->>View: select option
View-->>Comp: onSelectionChange(event)
Comp->>Form: propagate value via onChange
Comp-->>User: emit selectionChange
else Standalone usage
User->>View: select option
View-->>Comp: onSelectionChange(event)
Comp->>Comp: update internal value
Comp-->>User: emit selectionChange
end
rect rgba(220,240,255,0.4)
User->>View: type filter query
View-->>Comp: onFilter(query)
Comp-->>User: emit filterChange
end
opt Clear action
User->>Comp: click clear icon
Comp->>Form: onChange(null) / update internal value
Comp-->>User: emit clearSelection
end
opt Open/Close
User->>View: open or close panel
View-->>Comp: onShow/onHide
Comp-->>User: emit dropdownShow/dropdownHide
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/assets/i18n/en.json (1)
2348-2355: Add a translatable placeholder to fix pipeline warnings ('Search...')The HTML hard-codes 'Search...'. Add a pipes.search.placeholder key and use it in the template.
Apply this diff:
"pipes": { "search": { - "noResultFound": "No result found" + "noResultFound": "No result found", + "placeholder": "Search..." },Then update dropdown.component.html to bind this key (see HTML comments).
</blockquote></details> </blockquote></details>🧹 Nitpick comments (7)
src/app/shared/components/input/dropdown/dropdown.component.scss (2)
114-116: Typo in legacy class name: .dropdown-monthyIf safe, rename to .dropdown-monthly in CSS and corresponding templates; otherwise add a duplicate rule for the correct spelling to avoid future mistakes.
-:host ::ng-deep .dropdown-monthy .p-dropdown { +:host ::ng-deep .dropdown-monthly .p-dropdown, +:host ::ng-deep .dropdown-monthy .p-dropdown { @apply lg:tw-w-176-px md:tw-w-120-px #{!important}; }
33-46: Overuse of !important may cause brittle overridesWhere possible, prefer increasing selector specificity over !important to reduce maintenance risk.
Also applies to: 118-128
src/app/shared/components/input/dropdown/dropdown.component.html (2)
29-31: Drop unused parameter from isOverflowing usageTS method doesn’t use option; simplify the call.
- [pTooltip]="isOverflowing(textElement, option) ? option[displayKey] : null" + [pTooltip]="isOverflowing(textElement) ? option[displayKey] : null"Apply in both templates and update TS signature (see TS comment).
Also applies to: 106-108
33-37: Consolidate multi-line option rendering flagsBoth multiLine and isMultiLineOption control the same UI path. Prefer a single input to avoid drift.
- <p *ngIf="multiLine && option.subLabel" + <p *ngIf="isMultiLineOption && option.subLabel" class="item-subtext">{{ option.subLabel }}</p> - <p *ngIf="isMultiLineOption && option['subLabel']" + <p *ngIf="false && option['subLabel']" class="tw-text-text-muted tw-text-12-px">{{ option['subLabel'] }}{{ 'dropdown.subLabelSuffix' | transloco }}</p>Or remove one of the conditions and keep a single block with consistent styling.
Also applies to: 110-114
src/app/shared/components/input/dropdown/dropdown.component.ts (3)
174-179: Remove unused parameter from isOverflowingThe option parameter isn’t used.
- isOverflowing(element: HTMLElement, option: any): boolean { + isOverflowing(element: HTMLElement): boolean { if (!this.tooltipEnabled || !element) { return false; } return element.offsetWidth < element.scrollWidth; }Ensure HTML templates drop the second argument (see HTML comments).
30-33: Make form input optional to match usageOptional typing avoids strict-mode noise and clarifies intent.
- @Input() form: FormGroup; + @Input() form?: FormGroup;
11-18: Do you actually need NG_VALUE_ACCESSOR here?Since the inner p-dropdown binds directly to the provided FormGroup/FormControl and this component isn’t used as a form control itself (no [formControlName] on ), NG_VALUE_ACCESSOR might be redundant. Keeping it is fine if you plan to support <app-dropdown [(ngModel)]> or . Otherwise, consider removing the provider to reduce complexity.
Also applies to: 30-35
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/app/shared/components/input/dropdown/dropdown.component.html(1 hunks)src/app/shared/components/input/dropdown/dropdown.component.scss(1 hunks)src/app/shared/components/input/dropdown/dropdown.component.ts(1 hunks)src/assets/i18n/en.json(1 hunks)🧰 Additional context used
📓 Path-based instructions (4)
src/assets/i18n/*.json
📄 CodeRabbit inference engine (.cursor/rules/atomics-i18n-key-pattern.mdc)
src/assets/i18n/*.json: Group i18n keys under the correct atomic root: 'services', 'pipes', or 'directives' in the translation JSON files
Add i18n keys to every supported language file in 'src/assets/i18n/{lang}.json'
Keep child keys alphabetically sorted within each group in translation JSON files to reduce merge conflicts
Use only the three roots: services, pipes, directives for i18n key groupingFiles:
src/assets/i18n/en.json**/*.component.{ts,html}
📄 CodeRabbit inference engine (.cursor/rules/component-i18n-key-naming.mdc)
**/*.component.{ts,html}: Top-level object in i18n files must be the component or feature folder name, converted from the file name by removing the prefixes 'feature', 'ui', or 'component' (in that order) and converting from kebab-case to camelCase.
Keys inside the top-level i18n object must not be nested beyond the first level; only one level of keys is allowed.
Keys inside the top-level i18n object must use meaningful, semantic, and context-aware names (e.g., 'title', 'accountDisabledError', 'submitButton'); avoid generic names like 'label1', 'text1', 'message1'.
Do not use prefixes like 'ui', 'feature', or 'component' in the top-level i18n object name; the object name should be the context.
Do not translate strings that are already translation keys, tracking/analytics strings, strings with only special characters, comments, non-user-visible text, or variable bindings and expressions.
One i18n object per component or feature; do not combine multiple components/features into a single i18n object.Files:
src/app/shared/components/input/dropdown/dropdown.component.htmlsrc/app/shared/components/input/dropdown/dropdown.component.tssrc/**/*.component.{ts,html}
📄 CodeRabbit inference engine (.cursor/rules/component-i18n-key-naming.mdc)
For files in 'src/**', add or update i18n keys in the corresponding translation files located at 'src/assets/i18n/{lang}.json', and ensure the key is present in every supported language file.
Files:
src/app/shared/components/input/dropdown/dropdown.component.htmlsrc/app/shared/components/input/dropdown/dropdown.component.ts**/*.component.html
📄 CodeRabbit inference engine (.cursor/rules/component-i18n-key-naming.mdc)
Prefer property binding for user-facing attributes in Angular templates (e.g., [placeholder], [title]) when using translation keys.
Files:
src/app/shared/components/input/dropdown/dropdown.component.html🧬 Code graph analysis (1)
src/app/shared/components/input/dropdown/dropdown.component.ts (2)
src/app/shared/components/onboarding/clone-setting/clone-setting-field/clone-setting-field.component.ts (1)
Component(5-58)src/app/shared/components/input/toggle/toggle.component.ts (1)
Component(4-19)🪛 GitHub Actions: TypeScript Lint Check
src/app/shared/components/input/dropdown/dropdown.component.html
[warning] 60-60: Hard-coded string 'Search...' should be replaced with a translation key. (fyle/no-hardcoded-strings)
[warning] 137-137: Hard-coded string 'Search...' should be replaced with a translation key. (fyle/no-hardcoded-strings)
src/app/shared/components/input/dropdown/dropdown.component.ts
[error] 15-15: ESLint: 'DropdownComponent' was used before it was defined. (no-use-before-define)
🪛 GitHub Check: lint
src/app/shared/components/input/dropdown/dropdown.component.ts
[failure] 15-15:
'DropdownComponent' was used before it was defined🔇 Additional comments (2)
src/assets/i18n/en.json (1)
1-2356: Ensure the new/updated keys exist in every supported language fileAdd pipes.search.placeholder (and avoid dropdown.noResultsFound entirely) across all language JSONs to keep i18n consistent.
Would you like me to generate diffs for the other locale files as well?
src/app/shared/components/input/dropdown/dropdown.component.ts (1)
167-172: CSS hook for brand icon should target inner icon elementgetDropdownIcon appends the brandId class to the icon element; combined with the SCSS fix, this will now work. No change needed here, just confirming intent.
Please verify in the DOM that the class lands on .p-dropdown-trigger-icon and the icons render with the updated SCSS.
src/app/shared/components/input/dropdown/dropdown.component.html
Outdated
Show resolved
Hide resolved
src/app/shared/components/input/dropdown/dropdown.component.html
Outdated
Show resolved
Hide resolved
| import { Component, Input, Output, EventEmitter, OnInit, ViewChild, forwardRef, inject } from '@angular/core'; | ||
| import { FormGroup, ControlValueAccessor, NG_VALUE_ACCESSOR } from '@angular/forms'; | ||
| import { TranslocoService } from '@jsverse/transloco'; | ||
| import { Dropdown } from 'primeng/dropdown'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Import OnDestroy and rxjs utilities for safe subscription cleanup
We’ll use these to fix the statusChanges subscription leak below.
-import { Component, Input, Output, EventEmitter, OnInit, ViewChild, forwardRef, inject } from '@angular/core';
+import { Component, Input, Output, EventEmitter, OnInit, OnDestroy, ViewChild, forwardRef, inject } from '@angular/core';
+import { Subject } from 'rxjs';
+import { takeUntil } from 'rxjs/operators';📝 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 { Component, Input, Output, EventEmitter, OnInit, ViewChild, forwardRef, inject } from '@angular/core'; | |
| import { FormGroup, ControlValueAccessor, NG_VALUE_ACCESSOR } from '@angular/forms'; | |
| import { TranslocoService } from '@jsverse/transloco'; | |
| import { Dropdown } from 'primeng/dropdown'; | |
| import { Component, Input, Output, EventEmitter, OnInit, OnDestroy, ViewChild, forwardRef, inject } from '@angular/core'; | |
| import { Subject } from 'rxjs'; | |
| import { takeUntil } from 'rxjs/operators'; | |
| import { FormGroup, ControlValueAccessor, NG_VALUE_ACCESSOR } from '@angular/forms'; | |
| import { TranslocoService } from '@jsverse/transloco'; | |
| import { Dropdown } from 'primeng/dropdown'; |
🤖 Prompt for AI Agents
In src/app/shared/components/input/dropdown/dropdown.component.ts around lines 1
to 4, the component is missing OnDestroy and RXJS utilities needed to safely
clean up the statusChanges subscription; import OnDestroy from '@angular/core'
and import Subject and takeUntil (or takeWhile) from 'rxjs'/'rxjs/operators',
implement OnDestroy on the component, add a private destroy$ = new
Subject<void>(), pipe the existing FormControl.statusChanges subscription with
takeUntil(this.destroy$), and in ngOnDestroy call this.destroy$.next() and
this.destroy$.complete() to prevent memory leaks.
| export class DropdownComponent implements OnInit, ControlValueAccessor { | ||
| @ViewChild('dropdown') dropdown!: Dropdown; | ||
|
|
||
| private translocoService: TranslocoService = inject(TranslocoService); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Prevent memory leaks from statusChanges subscription
Add a destroy$ subject, pipe statusChanges through takeUntil, and implement ngOnDestroy.
-export class DropdownComponent implements OnInit, ControlValueAccessor {
+export class DropdownComponent implements OnInit, OnDestroy, ControlValueAccessor {
@ViewChild('dropdown') dropdown!: Dropdown;
private translocoService: TranslocoService = inject(TranslocoService);
+ private destroy$ = new Subject<void>();
@@
private setupFormValidation() {
if (this.form && this.formControllerName) {
const control = this.form.get(this.formControllerName);
if (control) {
- control.statusChanges.subscribe(status => {
+ control.statusChanges.pipe(takeUntil(this.destroy$)).subscribe(status => {
this.errorState = (status === 'INVALID' && control.touched) && !this.isSearchFocused;
});
}
}
}
+
+ ngOnDestroy(): void {
+ this.destroy$.next();
+ this.destroy$.complete();
+ }Also applies to: 84-89, 101-110, 197-199
🤖 Prompt for AI Agents
In src/app/shared/components/input/dropdown/dropdown.component.ts around lines
20-24 (and also apply to ranges 84-89, 101-110, 197-199), the component
subscribes to formControl.statusChanges without teardown causing potential
memory leaks; add a private destroy$ = new Subject<void>(), have the component
implement OnDestroy, pipe all statusChanges (and any other long-lived)
subscriptions through takeUntil(this.destroy$), and in ngOnDestroy call
this.destroy$.next(); this.destroy$.complete(); (also import Subject and
takeUntil and add OnDestroy to the class signature). Ensure no remaining raw
subscriptions remain unsubscribed.
| @Input() emptyMessage: string = this.translocoService.translate('dropdown.noResultsFound'); | ||
|
|
||
| @Input() loading: boolean = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use existing i18n key; avoid adding dropdown.noResultsFound
Align with i18n change by reusing pipes.search.noResultFound.
- @Input() emptyMessage: string = this.translocoService.translate('dropdown.noResultsFound');
+ @Input() emptyMessage: string = this.translocoService.translate('pipes.search.noResultFound');📝 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.
| @Input() emptyMessage: string = this.translocoService.translate('dropdown.noResultsFound'); | |
| @Input() loading: boolean = false; | |
| @Input() emptyMessage: string = this.translocoService.translate('pipes.search.noResultFound'); | |
| @Input() loading: boolean = false; |
🤖 Prompt for AI Agents
In src/app/shared/components/input/dropdown/dropdown.component.ts around lines
53 to 55, the default i18n key uses 'dropdown.noResultsFound' which must be
replaced with the existing key 'pipes.search.noResultFound'; update the @Input()
emptyMessage default to use
this.translocoService.translate('pipes.search.noResultFound') (or otherwise
obtain the same translation via the transloco pipe/service used in this
codebase) so the component reuses the existing i18n key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/app/shared/components/input/dropdown/dropdown.component.ts (2)
12-20: Fix ESLint no-use-before-define in providers by moving provider to a constantCreate a named constant and reference it from the decorator.
@@ - providers: [ - { - provide: NG_VALUE_ACCESSOR, - useExisting: forwardRef(() => DropdownComponent), - multi: true - } - ] + providers: [DROPDOWN_VALUE_ACCESSOR]Add near the imports:
export const DROPDOWN_VALUE_ACCESSOR = { provide: NG_VALUE_ACCESSOR, // eslint-disable-next-line no-use-before-define useExisting: forwardRef((): any => DropdownComponent), multi: true };
1-2: Prevent subscription leak; import OnDestroy and use takeUntil teardownWire statusChanges through a teardown and implement ngOnDestroy.
- import { Component, Input, Output, EventEmitter, OnInit, ViewChild, forwardRef, inject } from '@angular/core'; + import { Component, Input, Output, EventEmitter, OnInit, OnDestroy, ViewChild, forwardRef, inject } from '@angular/core'; + import { Subject } from 'rxjs'; + import { takeUntil } from 'rxjs/operators'; @@ -export class DropdownComponent implements OnInit, ControlValueAccessor { +export class DropdownComponent implements OnInit, OnDestroy, ControlValueAccessor { @@ private translocoService: TranslocoService = inject(TranslocoService); + private destroy$ = new Subject<void>(); @@ - control.statusChanges.subscribe(status => { + control.statusChanges.pipe(takeUntil(this.destroy$)).subscribe(status => { this.errorState = (status === 'INVALID' && control.touched) && !this.isSearchFocused; }); @@ } + + ngOnDestroy(): void { + this.destroy$.next(); + this.destroy$.complete(); + }Also applies to: 22-23, 98-111
🧹 Nitpick comments (4)
src/app/shared/components/input/dropdown/dropdown.component.ts (4)
56-56: Make emptyMessage reactive to language changes (avoid early translation at field init)Translating at field init won’t update on runtime locale change. Subscribe to selectTranslate instead.
- @Input() emptyMessage: string = this.translocoService.translate('dropdown.noResultsFound'); + @Input() emptyMessage?: string; @@ - ngOnInit() { - this.setupFormValidation(); - } + ngOnInit() { + this.setupFormValidation(); + if (!this.emptyMessage) { + this.translocoService + .selectTranslate('dropdown.noResultsFound') + .pipe(takeUntil(this.destroy$)) + .subscribe(t => (this.emptyMessage = t)); + } + }Also applies to: 98-100
124-127: Recompute error state on focus/blur to avoid stale UIstatusChanges may not fire on focus toggles; recompute when focus changes.
onSearchFocus(focused: boolean) { this.isSearchFocused = focused; + if (this.form && this.formControllerName) { + const control = this.form.get(this.formControllerName); + if (control) { + this.errorState = (control.status === 'INVALID' && control.touched) && !this.isSearchFocused; + } + } }Also applies to: 102-111
171-176: Remove unused parameter from isOverflowingoption isn’t used.
- isOverflowing(element: HTMLElement, option: any): boolean { + isOverflowing(element: HTMLElement): boolean {
27-31: Type safety and API polishConsider generics for value type and stronger event typings to reduce any usage. Keep the public API crisp for app-wide reuse.
Also applies to: 39-63, 69-75, 86-96, 113-121, 137-144, 145-162, 164-170, 178-196
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/app/shared/components/input/dropdown/dropdown.component.html(1 hunks)src/app/shared/components/input/dropdown/dropdown.component.scss(1 hunks)src/app/shared/components/input/dropdown/dropdown.component.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/shared/components/input/dropdown/dropdown.component.scss
- src/app/shared/components/input/dropdown/dropdown.component.html
🧰 Additional context used
📓 Path-based instructions (2)
**/*.component.{ts,html}
📄 CodeRabbit inference engine (.cursor/rules/component-i18n-key-naming.mdc)
**/*.component.{ts,html}: Top-level object in i18n files must be the component or feature folder name, converted from the file name by removing the prefixes 'feature', 'ui', or 'component' (in that order) and converting from kebab-case to camelCase.
Keys inside the top-level i18n object must not be nested beyond the first level; only one level of keys is allowed.
Keys inside the top-level i18n object must use meaningful, semantic, and context-aware names (e.g., 'title', 'accountDisabledError', 'submitButton'); avoid generic names like 'label1', 'text1', 'message1'.
Do not use prefixes like 'ui', 'feature', or 'component' in the top-level i18n object name; the object name should be the context.
Do not translate strings that are already translation keys, tracking/analytics strings, strings with only special characters, comments, non-user-visible text, or variable bindings and expressions.
One i18n object per component or feature; do not combine multiple components/features into a single i18n object.
Files:
src/app/shared/components/input/dropdown/dropdown.component.ts
src/**/*.component.{ts,html}
📄 CodeRabbit inference engine (.cursor/rules/component-i18n-key-naming.mdc)
For files in 'src/**', add or update i18n keys in the corresponding translation files located at 'src/assets/i18n/{lang}.json', and ensure the key is present in every supported language file.
Files:
src/app/shared/components/input/dropdown/dropdown.component.ts
🧬 Code graph analysis (1)
src/app/shared/components/input/dropdown/dropdown.component.ts (2)
src/app/shared/components/onboarding/clone-setting/clone-setting-field/clone-setting-field.component.ts (1)
Component(5-58)src/app/branding/branding-config.ts (1)
brandingConfig(11-11)
🔇 Additional comments (2)
src/app/shared/components/input/dropdown/dropdown.component.ts (2)
164-170: Confirm PrimeNG accepts multiple classes in dropdownIconYou return "pi pi-chevron-down ${brandingConfig.brandId}". Ensure p-dropdown’s dropdownIcon supports multiple classes; otherwise, brand via CSS on the trigger container.
56-56: Add missingdropdown.noResultsFoundentries in module-specific i18n files
- Present in
src/assets/i18n/en.json(OK)- Missing in
src/assets/i18n/co/en.json- Missing in
src/assets/i18n/fyle/en.jsonAdd the
dropdown.noResultsFoundkey with its translated value to the two missing files.⛔ Skipped due to learnings
Learnt from: CR PR: fylein/fyle-integrations-app#0 File: .cursor/rules/atomics-i18n-key-pattern.mdc:0-0 Timestamp: 2025-07-28T07:37:10.809Z Learning: Applies to src/assets/i18n/*.json : Add i18n keys to every supported language file in 'src/assets/i18n/{lang}.json'Learnt from: CR PR: fylein/fyle-integrations-app#0 File: .cursor/rules/component-i18n-key-naming.mdc:0-0 Timestamp: 2025-07-28T07:37:26.776Z Learning: Applies to src/**/*.component.{ts,html} : For files in 'src/**', add or update i18n keys in the corresponding translation files located at 'src/assets/i18n/{lang}.json', and ensure the key is present in every supported language file.
| @Input() form: FormGroup; | ||
|
|
||
| @Input() formControllerName: string; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Mark optional @input()s to avoid strict initialization errors
These fields aren’t required in all modes; make them optional.
- @Input() form: FormGroup;
+ @Input() form?: FormGroup;
@@
- @Input() formControllerName: string;
+ @Input() formControllerName?: string;
@@
- @Input() optionLabel: string;
+ @Input() optionLabel?: string;
@@
- @Input() optionValue: string;
+ @Input() optionValue?: string;Also applies to: 65-68
🤖 Prompt for AI Agents
In src/app/shared/components/input/dropdown/dropdown.component.ts around lines
32-35 and 65-68, the @Input() properties are not always provided and cause
strict initialization errors; mark them optional by adding the ? to the property
names and allow undefined in their types (e.g., form?: FormGroup | undefined;
formControllerName?: string | undefined), and then update any usages in the
component to guard against undefined (or use safe chaining/defaults) so runtime
access is protected.
| * dropdown is associated with. | ||
| */ | ||
| export class DropdownComponent implements OnInit, ControlValueAccessor { | ||
| @ViewChild('dropdown') dropdown!: Dropdown; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can remove if unused
| @Input() isMultiLineOption: boolean = false; | ||
|
|
||
| // New enhanced inputs | ||
| @Input() size: 'small' | 'medium' | 'large' = 'medium'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enum
|
|
||
| @Input() showClearIcon: boolean = false; | ||
|
|
||
| @Input() customClass: string = ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
diff b/w additionalClass and customClass - can we keep 1
| [filterFields]="filterFields.length > 0 ? filterFields : [displayKey]" | ||
| [emptyFilterMessage]="emptyMessage" | ||
| [dropdownIcon]="getDropdownIcon()" | ||
| [optionLabel]="optionLabel" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can plan to remove this since we have template
* feat: migrate advanced search dropdowns to app-dropdown * fix: comments * feat: migrate regular config page dropdowns (#1492) * fix: text overflow ellipsis for long options * feat: migrate all regular config dropdowns * feat: migrate qbd schedule export dropdowns (#1493) * feat: migrate qbd schedule export dropdowns * feat: migrate remaining edge-case config dropdowns (#1495) * fix: tooltip render logic * refactor: remove formControlName for standalone dropdowns
🔁 Code Duplication Report - Angular
📊 No significant change in code duplication. |
Description
Create a shiny new
app-dropdowncomponent and migrate all uses of the PrimeNG dropdown to this central componentClickup
https://app.clickup.com/t/86czy8qez
Summary by CodeRabbit
New Features
Style
Localization