fix: dropdown in primeng #1401
fix: dropdown in primeng #1401
Conversation
WalkthroughThe updates simplify and standardize theming and styling for dropdown components and the application theme. The custom PrimeNG theme preset is removed in favor of directly using the Aura theme. Dropdown SCSS styles are drastically reduced to a minimal set. The Fyle theme receives new CSS variables for PrimeNG Select customization. A guard is simplified to always return true if a workspace ID exists. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/app/core/guard/intacct-token.guard.ts (1)
34-46: Remove unreachable code after early return.Lines 34-46 are unreachable due to the
return true;statement on line 32. This code handles token expiration and error scenarios but will never execute.- return true; - - return this.intacctConnectorService.checkIntacctTokenHealth(workspaceId).pipe( - map(() => true), - catchError(error => { - if (error.status === 400) { - globalCacheBusterNotifier.next(); - - if (error.error.message === "Intacct connection expired"){ - return this.router.navigateByUrl('integrations/intacct/token_expired/dashboard'); - } - } - return throwError(error); - }) - ); + return this.intacctConnectorService.checkIntacctTokenHealth(workspaceId).pipe( + map(() => true), + catchError(error => { + if (error.status === 400) { + globalCacheBusterNotifier.next(); + + if (error.error.message === "Intacct connection expired"){ + return this.router.navigateByUrl('integrations/intacct/token_expired/dashboard'); + } + } + return throwError(error); + }) + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/app.module.ts(2 hunks)src/app/core/guard/intacct-token.guard.ts(1 hunks)src/assets/scss/dropdowns.scss(1 hunks)src/assets/themes/fyle/fdl.scss(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#298
File: src/app/shared/components/export-log/skipped-export-log-table/skipped-export-log-table.component.html:21-22
Timestamp: 2024-06-10T19:13:15.470Z
Learning: The user ashwin1111 has requested the removal of a condition that was always evaluating to false (`*ngIf="false"`), which was making an `h5` element never visible. This indicates a preference for removing redundant or placeholder code that is not currently in use.
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#298
File: src/app/shared/components/export-log/skipped-export-log-table/skipped-export-log-table.component.html:21-22
Timestamp: 2024-10-08T15:51:28.972Z
Learning: The user ashwin1111 has requested the removal of a condition that was always evaluating to false (`*ngIf="false"`), which was making an `h5` element never visible. This indicates a preference for removing redundant or placeholder code that is not currently in use.
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#353
File: src/app/core/services/business-central/business-central-configuration/business-central-advanced-settings.service.ts:0-0
Timestamp: 2024-10-08T15:51:28.972Z
Learning: The user ashwin1111 has confirmed the correction of a typo in the method name from `getExpenseFilelds` to `getExpenseFields` in the `BusinessCentralAdvancedSettingsService` class.
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#353
File: src/app/core/services/business-central/business-central-configuration/business-central-advanced-settings.service.ts:0-0
Timestamp: 2024-06-10T19:13:15.470Z
Learning: The user ashwin1111 has confirmed the correction of a typo in the method name from `getExpenseFilelds` to `getExpenseFields` in the `BusinessCentralAdvancedSettingsService` class.
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#343
File: src/app/integrations/business-central/business-central-shared/business-central-export-settings/business-central-export-settings.component.html:0-0
Timestamp: 2024-06-10T19:13:15.470Z
Learning: The user ashwin1111 has confirmed the suggestion to replace the hardcoded reference to 'Sage Intacct' with a dynamic reference to the target system using the variable `targetSystemName` in the subLabel string within the `business-central-export-settings.component.html` file.
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#343
File: src/app/integrations/business-central/business-central-shared/business-central-export-settings/business-central-export-settings.component.html:0-0
Timestamp: 2024-10-08T15:51:28.972Z
Learning: The user ashwin1111 has confirmed the suggestion to replace the hardcoded reference to 'Sage Intacct' with a dynamic reference to the target system using the variable `targetSystemName` in the subLabel string within the `business-central-export-settings.component.html` file.
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#343
File: src/app/integrations/business-central/business-central-shared/business-central-export-settings/business-central-export-settings.component.html:0-0
Timestamp: 2024-06-10T19:13:15.470Z
Learning: The user ashwin1111 requested the removal of the "CRE" acronym from the subLabel string in the business-central-export-settings.component.html file, indicating a preference for not including this acronym in the user interface text.
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#343
File: src/app/integrations/business-central/business-central-shared/business-central-export-settings/business-central-export-settings.component.html:0-0
Timestamp: 2024-10-08T15:51:28.972Z
Learning: The user ashwin1111 requested the removal of the "CRE" acronym from the subLabel string in the business-central-export-settings.component.html file, indicating a preference for not including this acronym in the user interface text.
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#343
File: src/app/integrations/business-central/business-central-shared/business-central-export-settings/business-central-export-settings.component.html:0-0
Timestamp: 2024-10-08T15:51:28.972Z
Learning: The user ashwin1111 has confirmed the replacement of the 'intacct' keyword with the variable `targetSystemName` for dynamic system naming in the `business-central-export-settings.component.html` file.
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#343
File: src/app/integrations/business-central/business-central-shared/business-central-export-settings/business-central-export-settings.component.html:0-0
Timestamp: 2024-06-10T19:13:15.470Z
Learning: The user ashwin1111 has confirmed the replacement of the 'intacct' keyword with the variable `targetSystemName` for dynamic system naming in the `business-central-export-settings.component.html` file.
src/app/app.module.ts (2)
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 **/*.component.{ts,html} : Do not use prefixes like 'ui', 'feature', or 'component' in the top-level i18n object name; the object name should be the context.
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 : Use only the three roots: services, pipes, directives for i18n key grouping
🔇 Additional comments (3)
src/app/app.module.ts (1)
31-31: LGTM! Theme simplification using standard Aura preset.The change removes custom PrimeNG theme configuration in favor of the standard Aura theme, which simplifies maintenance and aligns with the theme standardization mentioned in the PR summary.
Also applies to: 50-50
src/assets/themes/fyle/fdl.scss (1)
408-443: LGTM! Comprehensive PrimeNG Select theming variables.The addition of extensive CSS custom properties for PrimeNG Select component provides consistent theming across all states. The variables properly reference existing theme tokens and use
!importantto ensure precedence over default PrimeNG styles.src/assets/scss/dropdowns.scss (1)
1-15: LGTM! Dropdown styles simplified in favor of CSS custom properties.The extensive simplification of dropdown styles makes sense given the comprehensive CSS variables added to the Fyle theme. This approach provides better maintainability and consistency while relying on the theme system for detailed styling.
| if (!workspaceId) { | ||
| return this.router.navigateByUrl(`workspaces`); | ||
| } | ||
| return true; |
There was a problem hiding this comment.
Critical: Guard logic bypasses token validation.
The guard now returns true immediately after workspace ID validation, completely bypassing the Intacct token health check. This could allow access with expired or invalid tokens.
🤖 Prompt for AI Agents
In src/app/core/guard/intacct-token.guard.ts at line 32, the guard returns true
immediately after validating the workspace ID, bypassing the Intacct token
health check. Modify the guard logic to perform the token validation check after
workspace ID validation and only return true if the token is valid and healthy.
Remove the premature return statement to ensure the token health check is
enforced before allowing access.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.html (2)
88-91: Add accessible label / role to the clear iconThe SVG is now plainly clickable (good ☑) but it is still announced as a decorative graphic to assistive tech.
Please add eitheraria-label="Clear selection"or wrap the icon in a<button>(preferred) so that screen-reader users can clear the value too.-<app-svg-icon *ngIf="showClearIcon && !isDisabled" class="tw-cursor-pointer" - [svgSource]="'cross-medium'" [c1SvgSource]="'grv-cross-filled-medium'" - (click)="removeFilter(form.controls[formControllerName])" - [height]="'18px'" [width]="'18px'" [styleClasses]="'tw-mt-4-px'"></app-svg-icon> +<button type="button" aria-label="Clear selection" + (click)="removeFilter(form.controls[formControllerName])" + class="tw-cursor-pointer tw-bg-transparent tw-border-none tw-p-0"> + <app-svg-icon [svgSource]="'cross-medium'" [c1SvgSource]="'grv-cross-filled-medium'" + [height]="'18px'" [width]="'18px'" [styleClasses]="'tw-mt-4-px'"></app-svg-icon> +</button>
176-181: Duplicate icon markup – consider extracting to a reusable templateThe clear-icon block appears twice with identical logic / styling. Extracting it into a small inline
ng-templateor a dedicated component (<app-clear-icon>) would reduce duplication and make future tweaks (like the a11y fix above) one-shot.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
src/app/integrations/intacct/intacct-shared/intacct-advanced-settings/intacct-advanced-settings.component.html(1 hunks)src/app/integrations/intacct/intacct-shared/intacct-advanced-settings/intacct-advanced-settings.component.scss(1 hunks)src/app/integrations/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.component.scss(1 hunks)src/app/integrations/qbd/qbd-main/qbd-mapping/qbd-generic-mapping/qbd-generic-mapping.component.scss(1 hunks)src/app/shared/components/configuration/configuration-import-field/configuration-import-field.component.scss(1 hunks)src/app/shared/components/configuration/configuration-mapping-fields/configuration-mapping-fields.component.scss(1 hunks)src/app/shared/components/configuration/configuration-multi-select/configuration-multi-select.component.html(1 hunks)src/app/shared/components/configuration/configuration-schedule-export/configuration-schedule-export.component.scss(1 hunks)src/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.html(2 hunks)src/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.scss(1 hunks)src/app/shared/components/configuration/configuration-skip-export/configuration-skip-export.component.scss(1 hunks)src/app/shared/components/configuration/email-multi-select-field/email-multi-select-field.component.html(1 hunks)src/app/shared/components/export-log/export-log-filter/export-log-filter.component.scss(1 hunks)src/app/shared/components/helper/mapping/generic-mapping-table/generic-mapping-table.component.scss(1 hunks)src/app/shared/components/helper/mapping/generic-mapping-v2/generic-mapping-v2.component.scss(1 hunks)src/app/shared/components/helper/paginator/paginator.component.scss(1 hunks)src/app/shared/components/input/dropdown/dropdown.component.scss(1 hunks)src/app/shared/components/menu/main-menu/main-menu.component.scss(1 hunks)src/app/shared/components/si/helper/skip-export/skip-export.component.scss(1 hunks)src/assets/scss/dropdowns.scss(1 hunks)src/assets/themes/fyle/fdl.scss(1 hunks)src/styles.scss(1 hunks)
✅ Files skipped from review due to trivial changes (14)
- src/app/shared/components/export-log/export-log-filter/export-log-filter.component.scss
- src/app/shared/components/configuration/configuration-skip-export/configuration-skip-export.component.scss
- src/app/shared/components/configuration/configuration-schedule-export/configuration-schedule-export.component.scss
- src/app/shared/components/configuration/configuration-import-field/configuration-import-field.component.scss
- src/app/integrations/intacct/intacct-shared/intacct-advanced-settings/intacct-advanced-settings.component.html
- src/app/shared/components/menu/main-menu/main-menu.component.scss
- src/app/shared/components/input/dropdown/dropdown.component.scss
- src/app/shared/components/si/helper/skip-export/skip-export.component.scss
- src/app/integrations/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.component.scss
- src/app/shared/components/helper/mapping/generic-mapping-v2/generic-mapping-v2.component.scss
- src/app/integrations/intacct/intacct-shared/intacct-advanced-settings/intacct-advanced-settings.component.scss
- src/app/shared/components/configuration/configuration-mapping-fields/configuration-mapping-fields.component.scss
- src/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.scss
- src/assets/themes/fyle/fdl.scss
🚧 Files skipped from review as they are similar to previous changes (1)
- src/assets/scss/dropdowns.scss
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/configuration/email-multi-select-field/email-multi-select-field.component.htmlsrc/app/shared/components/configuration/configuration-multi-select/configuration-multi-select.component.htmlsrc/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.html
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/configuration/email-multi-select-field/email-multi-select-field.component.htmlsrc/app/shared/components/configuration/configuration-multi-select/configuration-multi-select.component.htmlsrc/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.html
**/*.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/configuration/email-multi-select-field/email-multi-select-field.component.htmlsrc/app/shared/components/configuration/configuration-multi-select/configuration-multi-select.component.htmlsrc/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.html
🧠 Learnings (6)
📓 Common learnings
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#298
File: src/app/shared/components/export-log/skipped-export-log-table/skipped-export-log-table.component.html:21-22
Timestamp: 2024-06-10T19:13:15.470Z
Learning: The user ashwin1111 has requested the removal of a condition that was always evaluating to false (`*ngIf="false"`), which was making an `h5` element never visible. This indicates a preference for removing redundant or placeholder code that is not currently in use.
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#298
File: src/app/shared/components/export-log/skipped-export-log-table/skipped-export-log-table.component.html:21-22
Timestamp: 2024-10-08T15:51:28.972Z
Learning: The user ashwin1111 has requested the removal of a condition that was always evaluating to false (`*ngIf="false"`), which was making an `h5` element never visible. This indicates a preference for removing redundant or placeholder code that is not currently in use.
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#353
File: src/app/core/services/business-central/business-central-configuration/business-central-advanced-settings.service.ts:0-0
Timestamp: 2024-10-08T15:51:28.972Z
Learning: The user ashwin1111 has confirmed the correction of a typo in the method name from `getExpenseFilelds` to `getExpenseFields` in the `BusinessCentralAdvancedSettingsService` class.
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#353
File: src/app/core/services/business-central/business-central-configuration/business-central-advanced-settings.service.ts:0-0
Timestamp: 2024-06-10T19:13:15.470Z
Learning: The user ashwin1111 has confirmed the correction of a typo in the method name from `getExpenseFilelds` to `getExpenseFields` in the `BusinessCentralAdvancedSettingsService` class.
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#343
File: src/app/integrations/business-central/business-central-shared/business-central-export-settings/business-central-export-settings.component.html:0-0
Timestamp: 2024-10-08T15:51:28.972Z
Learning: The user ashwin1111 has confirmed the suggestion to replace the hardcoded reference to 'Sage Intacct' with a dynamic reference to the target system using the variable `targetSystemName` in the subLabel string within the `business-central-export-settings.component.html` file.
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#343
File: src/app/integrations/business-central/business-central-shared/business-central-export-settings/business-central-export-settings.component.html:0-0
Timestamp: 2024-06-10T19:13:15.470Z
Learning: The user ashwin1111 has confirmed the suggestion to replace the hardcoded reference to 'Sage Intacct' with a dynamic reference to the target system using the variable `targetSystemName` in the subLabel string within the `business-central-export-settings.component.html` file.
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#343
File: src/app/integrations/business-central/business-central-shared/business-central-export-settings/business-central-export-settings.component.html:0-0
Timestamp: 2024-06-10T19:13:15.470Z
Learning: The user ashwin1111 requested the removal of the "CRE" acronym from the subLabel string in the business-central-export-settings.component.html file, indicating a preference for not including this acronym in the user interface text.
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#343
File: src/app/integrations/business-central/business-central-shared/business-central-export-settings/business-central-export-settings.component.html:0-0
Timestamp: 2024-10-08T15:51:28.972Z
Learning: The user ashwin1111 requested the removal of the "CRE" acronym from the subLabel string in the business-central-export-settings.component.html file, indicating a preference for not including this acronym in the user interface text.
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#343
File: src/app/integrations/business-central/business-central-shared/business-central-export-settings/business-central-export-settings.component.html:0-0
Timestamp: 2024-10-08T15:51:28.972Z
Learning: The user ashwin1111 has confirmed the replacement of the 'intacct' keyword with the variable `targetSystemName` for dynamic system naming in the `business-central-export-settings.component.html` file.
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#343
File: src/app/integrations/business-central/business-central-shared/business-central-export-settings/business-central-export-settings.component.html:0-0
Timestamp: 2024-06-10T19:13:15.470Z
Learning: The user ashwin1111 has confirmed the replacement of the 'intacct' keyword with the variable `targetSystemName` for dynamic system naming in the `business-central-export-settings.component.html` file.
src/app/integrations/qbd/qbd-main/qbd-mapping/qbd-generic-mapping/qbd-generic-mapping.component.scss (1)
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 **/*.component.{ts,html} : Do not use prefixes like 'ui', 'feature', or 'component' in the top-level i18n object name; the object name should be the context.
src/app/shared/components/configuration/email-multi-select-field/email-multi-select-field.component.html (2)
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 **/*.component.html : Prefer property binding for user-facing attributes in Angular templates (e.g., [placeholder], [title]) when using translation keys.
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 **/*.component.{ts,html} : Do not use prefixes like 'ui', 'feature', or 'component' in the top-level i18n object name; the object name should be the context.
src/app/shared/components/configuration/configuration-multi-select/configuration-multi-select.component.html (4)
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 **/*.component.html : Prefer property binding for user-facing attributes in Angular templates (e.g., [placeholder], [title]) when using translation keys.
Learnt from: ashwin1111
PR: #298
File: src/app/shared/components/export-log/skipped-export-log-table/skipped-export-log-table.component.html:21-22
Timestamp: 2024-10-08T15:51:28.972Z
Learning: The user ashwin1111 has requested the removal of a condition that was always evaluating to false (*ngIf="false"), which was making an h5 element never visible. This indicates a preference for removing redundant or placeholder code that is not currently in use.
Learnt from: ashwin1111
PR: #298
File: src/app/shared/components/export-log/skipped-export-log-table/skipped-export-log-table.component.html:21-22
Timestamp: 2024-06-10T19:13:15.470Z
Learning: The user ashwin1111 has requested the removal of a condition that was always evaluating to false (*ngIf="false"), which was making an h5 element never visible. This indicates a preference for removing redundant or placeholder code that is not currently in use.
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 **/*.component.{ts,html} : Do not use prefixes like 'ui', 'feature', or 'component' in the top-level i18n object name; the object name should be the context.
src/app/shared/components/helper/mapping/generic-mapping-table/generic-mapping-table.component.scss (3)
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 **/*.component.{ts,html} : Do not use prefixes like 'ui', 'feature', or 'component' in the top-level i18n object name; the object name should be the context.
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 **/*.component.html : Prefer property binding for user-facing attributes in Angular templates (e.g., [placeholder], [title]) when using translation keys.
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 **/*.component.{ts,html} : 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.
src/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.html (3)
Learnt from: ashwin1111
PR: #298
File: src/app/shared/components/export-log/skipped-export-log-table/skipped-export-log-table.component.html:21-22
Timestamp: 2024-10-08T15:51:28.972Z
Learning: The user ashwin1111 has requested the removal of a condition that was always evaluating to false (*ngIf="false"), which was making an h5 element never visible. This indicates a preference for removing redundant or placeholder code that is not currently in use.
Learnt from: ashwin1111
PR: #298
File: src/app/shared/components/export-log/skipped-export-log-table/skipped-export-log-table.component.html:21-22
Timestamp: 2024-06-10T19:13:15.470Z
Learning: The user ashwin1111 has requested the removal of a condition that was always evaluating to false (*ngIf="false"), which was making an h5 element never visible. This indicates a preference for removing redundant or placeholder code that is not currently in use.
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 **/*.component.html : Prefer property binding for user-facing attributes in Angular templates (e.g., [placeholder], [title]) when using translation keys.
🔇 Additional comments (4)
src/app/integrations/qbd/qbd-main/qbd-mapping/qbd-generic-mapping/qbd-generic-mapping.component.scss (1)
10-16: Confirm class name matches current PrimeNG markupThe hide/show rules were moved from
.p-select-triggerto.p-select-dropdown.
PrimeNG 17+ indeed renamed the element, but projects on <=16 still use.p-select-trigger. Please verify the library version; otherwise these rules will no-op and the clear icon visibility logic will break.src/app/shared/components/helper/mapping/generic-mapping-table/generic-mapping-table.component.scss (1)
22-28: Visibility rule depends on library versionSame concern as above:
.p-select-dropdownis only present in PrimeNG ≥ 17.
If the application hasn’t upgraded yet, the hidden / visible arrows will stop working.src/app/shared/components/configuration/email-multi-select-field/email-multi-select-field.component.html (1)
12-13: Good switch to the officialplaceholderinput
defaultLabelis deprecated; using[placeholder]keeps the template future-proof. 🎉src/app/shared/components/configuration/configuration-multi-select/configuration-multi-select.component.html (1)
12-19: VerifyplaceholdervsdefaultLabelafter PrimeNG upgrade
p-multiSelectaccepteddefaultLabelup to v18 and switched toplaceholderin v19+.
If the library has not been bumped yet this will silently drop the label. Please confirm the PrimeNG version before merging.
| :host ::ng-deep .p-select .p-select-dropdown { | ||
| content: url('/assets/icons/co/grv-chevron-down-small.svg') !important; | ||
| @apply tw-w-12-px #{!important}; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Same pseudo-element issue as in global styles
content should live on .p-select-dropdown::before; otherwise the chevron replacement may silently fail.
-:host ::ng-deep .p-select .p-select-dropdown {
- content: url('/assets/icons/co/grv-chevron-down-small.svg') !important;
- @apply tw-w-12-px #{!important};
-}
+:host ::ng-deep .p-select .p-select-dropdown::before {
+ content: url('/assets/icons/co/grv-chevron-down-small.svg') !important;
+}
+:host ::ng-deep .p-select .p-select-dropdown {
+ @apply tw-w-12-px #{!important};
+}Repeat this pattern everywhere .p-select-dropdown is styled.
🤖 Prompt for AI Agents
In src/app/shared/components/helper/paginator/paginator.component.scss around
lines 18 to 21, the CSS rule sets the content property directly on
.p-select-dropdown, which is incorrect. Move the content property to the
::before pseudo-element of .p-select-dropdown (i.e., .p-select-dropdown::before)
to ensure the chevron icon displays properly. Apply this change consistently
wherever .p-select-dropdown is styled with content.
| .p-select-dropdown { | ||
| content: url('./assets/icons/arrow-down.svg') !important; | ||
| width: 18px !important; | ||
| } | ||
| } | ||
|
|
||
| [data-theme='co'] { | ||
| .p-select-trigger { | ||
| .p-select-dropdown { | ||
| content: url('./assets/icons/co/grv-chevron-down.svg') !important; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Override the dropdown icon on the correct pseudo-element
content only renders on pseudo-elements like ::before / ::after.
Applying it directly on .p-select-dropdown has no effect in most browsers, so the custom SVG arrow won’t show.
-[data-theme='fyle'] {
- .p-select-dropdown {
- content: url('./assets/icons/arrow-down.svg') !important;
- width: 18px !important;
- }
-}
+[data-theme='fyle'] {
+ .p-select-dropdown::before {
+ content: url('./assets/icons/arrow-down.svg') !important;
+ }
+ .p-select-dropdown {
+ @apply tw-w-18-px !important; /* keep width tweak */
+ }
+}
-[data-theme='co'] {
- .p-select-dropdown {
- content: url('./assets/icons/co/grv-chevron-down.svg') !important;
- }
-}
+[data-theme='co'] {
+ .p-select-dropdown::before {
+ content: url('./assets/icons/co/grv-chevron-down.svg') !important;
+ }
+}Benefits: keeps the icon visible across browsers, removes invalid CSS, and leverages Tailwind for width.
While touching this code, consider merging the two separate [data-theme='fyle'] / [data-theme='co'] blocks to avoid duplication.
Use ::before to Inject the Dropdown Icon
The CSS content property only takes effect on pseudo-elements, so applying it to .p-select-dropdown itself won’t render your SVG in most browsers. Update your SCSS as follows:
• File: src/styles.scss
– Replace content rules on .p-select-dropdown with ::before
– Keep the width tweak on the main selector (using Tailwind’s @apply or plain width)
– Optional: merge the two theme-specific blocks to DRY up the icons
-[data-theme='fyle'] {
- .p-select-dropdown {
- content: url('./assets/icons/arrow-down.svg') !important;
- width: 18px !important;
- }
-}
+[data-theme='fyle'] {
+ .p-select-dropdown::before {
+ content: url('./assets/icons/arrow-down.svg') !important;
+ }
+ .p-select-dropdown {
+ @apply tw-w-18-px !important; /* preserve width */
+ }
+}
-[data-theme='co'] {
- .p-select-dropdown {
- content: url('./assets/icons/co/grv-chevron-down.svg') !important;
- }
-}
+[data-theme='co'] {
+ .p-select-dropdown::before {
+ content: url('./assets/icons/co/grv-chevron-down.svg') !important;
+ }
+}Consider merging these two blocks if you want to centralize theme-based variables and reduce duplication.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/styles.scss around lines 91 to 100, the CSS uses the content property
directly on the .p-select-dropdown class, which does not render the SVG icon
because content only works on pseudo-elements. To fix this, move the content
property to a ::before pseudo-element on .p-select-dropdown, keep the width
styling on the main selector, and optionally merge the theme-specific blocks to
reduce duplication by centralizing the icon URLs.
1 participant
Description
Please add PR description here, add screenshots if needed
Clickup
https://app.clickup.com/
Summary by CodeRabbit
Style
Bug Fixes