chore: Intacct connector page info box for fyle theme #991
Conversation
WalkthroughThe changes involve comprehensive updates to the configuration settings for various accounting integrations, including enhancements to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
|
PR title must start with "fix:", "feat:", "chore:", "refactor", or "test:" (case-insensitive) |
|
PR description must contain a link to a ClickUp (case-insensitive) |
DhaaraniCIT
changed the title
|
PR description must contain a link to a ClickUp (case-insensitive) |
2 similar comments
|
PR description must contain a link to a ClickUp (case-insensitive) |
|
PR description must contain a link to a ClickUp (case-insensitive) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/app/shared/components/si/core/intacct-connector/intacct-connector.component.html (2)
37-41: LGTM! Consider adding an aria-label for accessibility.The addition of this information box is a great improvement to the user interface, aligning well with the PR objective. The conditional styling based on the branding configuration ensures visual consistency.
Consider adding an
aria-labelto the div for improved accessibility:- <div class="tw-w-400-px tw-py-8-px tw-px-16-px tw-rounded-6-px" [ngClass]="{'tw-bg-bg-tertiary-lighter tw-border tw-border-separator' : brandingConfig.brandId === 'co', 'tw-bg-info-section' : brandingConfig.brandId !== 'co'}"> + <div class="tw-w-400-px tw-py-8-px tw-px-16-px tw-rounded-6-px" [ngClass]="{'tw-bg-bg-tertiary-lighter tw-border tw-border-separator' : brandingConfig.brandId === 'co', 'tw-bg-info-section' : brandingConfig.brandId !== 'co'}" aria-label="Integration credentials reminder">
Line range hint
1-41: Overall implementation looks great! Consider unifying error message structure.The Intacct connector component is well-structured with proper conditional rendering, branding configuration, and form validation. Great job on implementing the new info box and maintaining consistency with the existing code.
For improved consistency, consider unifying the structure of error messages. Currently, the Company ID and User ID error messages use "Invalid" while the Password error message uses "Incorrect". Standardizing these messages could enhance the user experience:
- Invalid {{brandingContent.intacct.common.companyId}}. Enter valid {{brandingContent.intacct.common.companyId}}. + Invalid {{brandingContent.intacct.common.companyId}}. Please enter a valid {{brandingContent.intacct.common.companyId}}. - Invalid {{brandingContent.intacct.common.userId}}. Enter valid {{brandingContent.intacct.common.userId}}. + Invalid {{brandingContent.intacct.common.userId}}. Please enter a valid {{brandingContent.intacct.common.userId}}. - Incorrect {{brandingContent.intacct.common.password}}. Enter correct {{brandingContent.intacct.common.password}}. + Invalid {{brandingContent.intacct.common.password}}. Please enter a valid {{brandingContent.intacct.common.password}}.This change would make the error messages more consistent and user-friendly.
|
PR description must contain a link to a ClickUp (case-insensitive) |
|
PR description must contain a link to a ClickUp (case-insensitive) |
1 similar comment
|
PR description must contain a link to a ClickUp (case-insensitive) |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
src/app/branding/c1-contents-config.ts (1)
187-187: Consider changing 'different than' to 'different from' for clarityIn standard English usage, 'different from' is preferred over 'different than' when making comparisons.
Apply this diff to improve the wording:
-connectorInfoLabel: 'Make sure you’re entering your integration credentials. These should be different than your Sage Intacct credentials.' +connectorInfoLabel: 'Make sure you’re entering your integration credentials. These should be different from your Sage Intacct credentials.'src/app/branding/fyle-contents-config.ts (3)
Line range hint
17-17: Change 'can not' to 'cannot' for correct usageIn the message, 'you can not change' should be 'you cannot change'.
Apply this diff to fix the grammatical error:
-subLabel: 'Expenses will be posted to the NetSuite Tenant Mapping selected here. Once configured, you can not change ' + brandingConfig.brandName + ' organization or Tenant Mapping.' +subLabel: 'Expenses will be posted to the NetSuite Tenant Mapping selected here. Once configured, you cannot change ' + brandingConfig.brandName + ' organization or Tenant Mapping.'
Line range hint
179-179: Change 'can not' to 'cannot' for correct usageIn the message, 'you can not change' should be 'you cannot change'.
Apply this diff to fix the grammatical error:
-locationSubLabel: 'Expenses will be posted to the Sage Intacct Location entity selected here. Once configured, you can not change ' + brandingConfig.brandName + ' Organization or Location Entity.', +locationSubLabel: 'Expenses will be posted to the Sage Intacct Location entity selected here. Once configured, you cannot change ' + brandingConfig.brandName + ' Organization or Location Entity.',
Line range hint
291-291: Add missing period after 'Sage Intacct' for proper punctuationThere is a missing period after 'Sage Intacct' in the sentence.
Apply this diff to fix the punctuation:
-customizeSectionSubLabel: 'In this section, you can customize the data that you\'d like to export from ' + brandingConfig.brandName + ' to Sage Intacct You can choose what data points need to be exported and what shouldn\'t be.' +customizeSectionSubLabel: 'In this section, you can customize the data that you\'d like to export from ' + brandingConfig.brandName + ' to Sage Intacct. You can choose what data points need to be exported and what shouldn\'t be.'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- src/app/branding/c1-contents-config.ts (1 hunks)
- src/app/branding/fyle-contents-config.ts (1 hunks)
- src/app/core/models/branding/content-configuration.model.ts (1 hunks)
- src/app/shared/components/si/core/intacct-connector/intacct-connector.component.html (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/shared/components/si/core/intacct-connector/intacct-connector.component.html
🔇 Additional comments (1)
src/app/core/models/branding/content-configuration.model.ts (1)
182-183: Properties added align with existing naming conventionsThe properties
locationSubLabelandconnectorInfoLabelare appropriately added to theconnectorconfiguration. They follow the existing structure and naming conventions, enhancing the configuration options for the Intacct connector.
| @@ -183,7 +183,8 @@ export const c1Contents = { | |||
| connector: { | |||
| stepName: 'Connect to Sage Intacct', | |||
| subLabel: 'Provide your credentials to establish a secure connection between your Expense Management and Sage Intacct account. ', | |||
| locationSubLabel: 'Expenses will be posted to the Sage Intacct location entity selected here. You can\'t change the location entity once they\'re configured.' | |||
| locationSubLabel: 'Expenses will be posted to the Sage Intacct location entity selected here. You can\'t change the location entity once they\'re configured.', | |||
There was a problem hiding this comment.
Fix pronoun agreement in 'locationSubLabel'
The term 'location entity' is singular, so the pronoun should be singular 'it' instead of 'they' in the sentence "You can't change the location entity once they're configured."
Apply this diff to correct the pronoun:
-locationSubLabel: 'Expenses will be posted to the Sage Intacct location entity selected here. You can\'t change the location entity once they\'re configured.',
+locationSubLabel: 'Expenses will be posted to the Sage Intacct location entity selected here. You can\'t change the location entity once it\'s configured.',📝 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.
| locationSubLabel: 'Expenses will be posted to the Sage Intacct location entity selected here. You can\'t change the location entity once they\'re configured.', | |
| locationSubLabel: 'Expenses will be posted to the Sage Intacct location entity selected here. You can\'t change the location entity once it\'s configured.', |
| @@ -183,7 +183,8 @@ export const fyleContents = { | |||
| connector: { | |||
| stepName: 'Connect to Sage Intacct', | |||
| locationSubLabel: 'Expenses will be posted to the Sage Intacct Location entity selected here. Once configured, you can not change ' + brandingConfig.brandName + ' Organization or Location Entity.', | |||
| subLabel: 'To connect your ' + brandingConfig.brandName + ' and Sage Intacct account, follow the detailed instructions provided in the article to generate the credentials and establish a secure connection.' | |||
| subLabel: 'To connect your ' + brandingConfig.brandName + ' and Sage Intacct account, follow the detailed instructions provided in the article to generate the credentials and establish a secure connection.', | |||
| connectorInfoLabel: 'Please use the integration credentials you received via email. These are different from the ones you use to login to your Sage Intacct account.' | |||
There was a problem hiding this comment.
Correct 'login' to 'log in' in the connectorInfoLabel
In the message, 'login' should be 'log in' when used as a verb.
Apply this diff to fix the grammatical error:
-connectorInfoLabel: 'Please use the integration credentials you received via email. These are different from the ones you use to login to your Sage Intacct account.'
+connectorInfoLabel: 'Please use the integration credentials you received via email. These are different from the ones you use to log in to your Sage Intacct account.'📝 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.
| connectorInfoLabel: 'Please use the integration credentials you received via email. These are different from the ones you use to login to your Sage Intacct account.' | |
| connectorInfoLabel: 'Please use the integration credentials you received via email. These are different from the ones you use to log in to your Sage Intacct account.' |
| @@ -183,7 +183,8 @@ | |||
| connector: { | |||
| stepName: 'Connect to Sage Intacct', | |||
| locationSubLabel: 'Expenses will be posted to the Sage Intacct Location entity selected here. Once configured, you can not change ' + brandingConfig.brandName + ' Organization or Location Entity.', | |||
| subLabel: 'To connect your ' + brandingConfig.brandName + ' and Sage Intacct account, follow the detailed instructions provided in the article to generate the credentials and establish a secure connection.' | |||
| subLabel: 'To connect your ' + brandingConfig.brandName + ' and Sage Intacct account, follow the detailed instructions provided in the article to generate the credentials and establish a secure connection.', | |||
There was a problem hiding this comment.
Change 'account' to 'accounts' for grammatical correctness
In the text, 'To connect your Fyle and Sage Intacct account...', 'account' should be plural to match 'Fyle and Sage Intacct accounts'.
Apply this diff to fix the grammatical error:
-subLabel: 'To connect your ' + brandingConfig.brandName + ' and Sage Intacct account, follow the detailed instructions provided in the article to generate the credentials and establish a secure connection.',
+subLabel: 'To connect your ' + brandingConfig.brandName + ' and Sage Intacct accounts, follow the detailed instructions provided in the article to generate the credentials and establish a secure connection.',📝 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.
| subLabel: 'To connect your ' + brandingConfig.brandName + ' and Sage Intacct account, follow the detailed instructions provided in the article to generate the credentials and establish a secure connection.', | |
| subLabel: 'To connect your ' + brandingConfig.brandName + ' and Sage Intacct accounts, follow the detailed instructions provided in the article to generate the credentials and establish a secure connection.', |
Description
Adding a Info box for the Intacct connector page similar to the C1 env
Clickup
https://app.clickup.com/t/86cwnfg30
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes