fix: integrations app usability fixes#1642
Conversation
WalkthroughSmall UI and styling updates across Intacct templates and shared components: adjusted wrapping and vertical spacing, simplified some container classes, added chip separators, tweaked dropdown and mapped/unmapped styles, and updated a few color primitives and i18n text entries. No public APIs changed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.html (1)
177-177: Clarify conflicting ngClass conditions.This line contains an array of two ternary conditions that can produce conflicting class names. For example, when
formControllerName === 'dayOfWeek', both the first condition ('tw-pl-34-px') and second condition ('tw-pl-30-px') are applied as separate array elements, which creates CSS precedence confusion.Consider restructuring this to use an object notation or a single conditional to make the logic clearer and avoid class name conflicts:
- <div [ngClass]="[formControllerName !== 'dayOfMonth' ? 'tw-pl-34-px' : 'tw-pl-70-px', formControllerName === 'dayOfWeek' ? 'tw-pl-30-px' : 'tw-pl-34-px']"> + <div [ngClass]="formControllerName === 'dayOfMonth' ? 'tw-pl-70-px' : formControllerName === 'dayOfWeek' ? 'tw-pl-30-px' : 'tw-pl-34-px'">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/app/integrations/intacct/intacct-shared/intacct-advanced-settings/intacct-advanced-settings.component.html(2 hunks)src/app/integrations/intacct/intacct-shared/intacct-export-settings/intacct-export-settings.component.html(1 hunks)src/app/integrations/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.component.html(9 hunks)src/app/integrations/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.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-skip-export/configuration-skip-export.component.html(1 hunks)src/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.html(1 hunks)src/app/shared/components/si/core/intacct-location-entity/intacct-location-entity.component.html(2 hunks)src/app/shared/components/si/helper/skip-export/skip-export.component.html(1 hunks)src/assets/i18n/en.json(2 hunks)src/styles.scss(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/app/integrations/intacct/intacct-shared/intacct-export-settings/intacct-export-settings.component.html
🚧 Files skipped from review as they are similar to previous changes (3)
- src/styles.scss
- src/app/integrations/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.component.html
- src/app/shared/components/si/core/intacct-location-entity/intacct-location-entity.component.html
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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/si/helper/skip-export/skip-export.component.htmlsrc/app/shared/components/configuration/configuration-skip-export/configuration-skip-export.component.htmlsrc/app/integrations/intacct/intacct-shared/intacct-advanced-settings/intacct-advanced-settings.component.htmlsrc/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.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/si/helper/skip-export/skip-export.component.htmlsrc/app/shared/components/configuration/configuration-skip-export/configuration-skip-export.component.htmlsrc/app/integrations/intacct/intacct-shared/intacct-advanced-settings/intacct-advanced-settings.component.htmlsrc/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.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/si/helper/skip-export/skip-export.component.htmlsrc/app/shared/components/configuration/configuration-skip-export/configuration-skip-export.component.htmlsrc/app/integrations/intacct/intacct-shared/intacct-advanced-settings/intacct-advanced-settings.component.htmlsrc/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.htmlsrc/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.html
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 grouping
Files:
src/assets/i18n/en.json
🧠 Learnings (3)
📚 Learning: 2024-10-08T15:51:28.972Z
Learnt from: ashwin1111
Repo: fylein/fyle-integrations-app PR: 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.
Applied to files:
src/app/integrations/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.component.scss
📚 Learning: 2024-06-10T19:13:15.470Z
Learnt from: ashwin1111
Repo: fylein/fyle-integrations-app PR: 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.
Applied to files:
src/app/integrations/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.component.scss
📚 Learning: 2025-07-28T07:37:26.776Z
Learnt from: CR
Repo: fylein/fyle-integrations-app PR: 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.
Applied to files:
src/app/shared/components/configuration/configuration-skip-export/configuration-skip-export.component.html
🔇 Additional comments (7)
src/app/integrations/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.component.scss (1)
22-28: Responsive height adjustment improves layout flexibility.Changing
.label-contentheight from fixed toautois a solid UX improvement that allows the container to adapt to its content dynamically rather than being constrained to a fixed dimension. This aligns well with the overall usability focus of this PR.Please verify that this change renders correctly across related import settings components (particularly in HTML templates that use this
.label-contentclass) and that it integrates well with the broader UI layout updates mentioned in the PR summary.src/app/shared/components/si/helper/skip-export/skip-export.component.html (1)
108-108: Separator addition aligns chip field behavior across the form.The
separator=","attribute makes the second chip field consistent with the first one at line 48, enabling tokenized input. Property binding for the placeholder attribute follows best practices.src/app/shared/components/configuration/configuration-skip-export/configuration-skip-export.component.html (1)
128-128: Separator addition aligns chip field behavior across the form.The
separator=","attribute makes the second chip field consistent with the first one at line 58, enabling tokenized input. Property binding for all user-facing attributes (placeholder, tooltips) correctly follows the guidelines.src/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.html (1)
4-4: The review comment is based on a false premise. The TypeScript definition inconfiguration-select-field.component.tsline 81 explicitly defines the property as@Input() isimportSettings: boolean = false;(lowercase 'i'). The HTML file correctly uses the defined property name. No change is needed.Likely an incorrect or invalid review comment.
src/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.html (1)
39-39: Minor spacing refinement for conditional state.Adjusted vertical padding to use top-padding only (
tw-pt-10-px) when failed exports count is falsy, which appears to be part of the broader layout refinements in this PR.src/assets/i18n/en.json (2)
37-37: Text quality improvements for form labels and error messages.Minor refinements: added trailing periods to validation errors for consistency and lowercased "address" in label for proper sentence casing.
Also applies to: 38-38, 40-40
2085-2086: Dismiss the review comment—subdirectories are tenant-specific overrides, not language files.The
co/andfyle/subdirectories are not language variants but rather tenant/brand-specific or feature-specific translation overrides containing only selective keys. The rootsrc/assets/i18n/en.jsoncontains the complete translation set includingintacctImportSettings, while the subdirectories do not have this section at all. SinceintacctImportSettingsdoes not exist inco/en.jsonorfyle/en.json, the new keys are correctly placed in the root translation file only. The coding guideline refers to language files (en, es, fr variants), not tenant-specific overrides.Likely an incorrect or invalid review comment.
| [formControllerName]="'setTopMemoField'"></app-configuration-multi-select> | ||
| <div class="tw-pl-60-px tw-pr-24-px tw-pb-24-px"> | ||
| <p class="tw-text-form-label-text-color tw-text-14-px tw-mb-12-px">{{ 'intacctAdvancedSettings.preview' | transloco }} {{'intacct.common.descriptionText' | transloco}}</p> | ||
| <!-- <p class="tw-text-form-label-text-color tw-text-14-px tw-mb-12-px">{{ 'intacctAdvancedSettings.preview' | transloco }} {{'intacct.common.descriptionText' | transloco}}</p> --> |
There was a problem hiding this comment.
🧩 Analysis chain
Remove commented-out code or document the reason for keeping it.
Commented-out code should either be deleted or retained with an explanatory comment. If this is intentional (e.g., temporary toggle, feature flag), please add a comment explaining the intent.
Additionally, verify that the translation keys intacctAdvancedSettings.preview and intacct.common.descriptionText are still used elsewhere in the codebase; if not, they should be removed from the i18n files to maintain a clean translation catalog.
Also applies to: 153-153
🏁 Script executed:
#!/bin/bash
# Verify if translation keys are used elsewhere in the codebase
echo "=== Searching for 'intacctAdvancedSettings.preview' usage ==="
rg -n "intacctAdvancedSettings\.preview" --type html --type ts
echo ""
echo "=== Searching for 'intacct.common.descriptionText' usage ==="
rg -n "intacct\.common\.descriptionText" --type html --type tsLength of output: 1395
Delete commented-out code and remove unused translation keys from i18n files.
Both translation keys (intacctAdvancedSettings.preview and intacct.common.descriptionText) are used only in the commented-out code at lines 136 and 153. Since these keys are not used anywhere else in the codebase, the commented code should be deleted and the corresponding entries removed from the translation files.
🤖 Prompt for AI Agents
In
src/app/integrations/intacct/intacct-shared/intacct-advanced-settings/intacct-advanced-settings.component.html
around lines 136 and 153, remove the commented-out <p> lines that reference the
unused translation keys; then delete the corresponding keys
intacctAdvancedSettings.preview and intacct.common.descriptionText from the i18n
translation files (all locales) after confirming they are not referenced
elsewhere in the repo. Ensure you run a quick global search for those keys
before removal and update any translation snapshots or build artifacts if
required.
🔁 Code Duplication Report - Angular
|
Description
fix: integrations app usability fixes
Clickup
https://app.clickup.com
Summary by CodeRabbit