-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: common main page padding style conversion #1163
base: branding-css
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request introduces a comprehensive update to the branding configuration across multiple components in the application. Two new properties, Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
✨ Finishing Touches
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 (
|
|
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
🧹 Nitpick comments (16)
src/app/integrations/business-central/business-central-main/business-central-export-log/business-central-export-log.component.ts (1)
4-4
: Great work on maintaining consistency across components!The implementation effectively centralizes the padding styles while maintaining a consistent pattern across all components. The use of readonly modifiers ensures proper immutability.
Consider documenting the brandingStyle structure in the README or creating a separate documentation file to help other developers understand the available styling options and their intended usage.
Also applies to: 26-29
src/app/integrations/netsuite/netsuite-main/netsuite-configuration/netsuite-configuration.component.ts (2)
Line range hint
32-35
: Remove commented code if no longer needed.There's commented code in the ngOnInit method that should be removed if it's no longer required. If it's kept for future reference, consider documenting the reason in a TODO comment or moving it to documentation.
1-1
: Consider documenting the branding configuration architecture.The changes implement a consistent pattern for branding styles across components, which is good for maintainability. Consider:
- Documenting the purpose and usage of different branding configuration files (branding-config.ts vs c1-contents-config.ts)
- Adding examples of how to use the new brandingStyle in component templates
- Creating a migration guide for converting existing components
This will help future developers understand and correctly implement the branding system.
src/app/integrations/qbo/qbo-main/qbo-mapping/qbo-mapping.component.ts (1)
5-5
: LGTM! Consider consolidating branding-related properties.The changes are consistent with other components. However, since this component uses multiple branding-related properties (brandingConfig, brandingFeatureConfig, brandingStyle), consider consolidating them into a single branding service in a future refactor to improve maintainability.
Also applies to: 31-32
src/app/integrations/xero/xero-main/xero-mapping/xero-mapping.component.ts (1)
5-5
: LGTM! Consider adding JSDoc comments.The addition of
brandingStyle
import and readonly property aligns with the centralized styling approach. Consider adding JSDoc comments to document the purpose of this property./** * Centralized branding styles for consistent component styling * @readonly */ readonly brandingStyle = brandingStyle;Also applies to: 31-31
src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-pre-requisite/qbd-direct-onboarding-pre-requisite.component.ts (1)
67-67
: LGTM! Consider grouping branding-related properties.The addition of readonly
brandingStyle
property is correct. Consider grouping all branding-related properties together for better code organization.- brandingContent = brandingContent.qbd_direct.configuration.preRequisite; - brandingConfig: BrandingConfiguration = brandingConfig; - readonly brandingStyle = brandingStyle; + // Branding configurations + readonly brandingContent = brandingContent.qbd_direct.configuration.preRequisite; + readonly brandingConfig: BrandingConfiguration = brandingConfig; + readonly brandingStyle = brandingStyle;src/app/integrations/qbd/qbd-main/qbd-mapping/qbd-generic-mapping/qbd-generic-mapping.component.ts (1)
4-4
: LGTM! Consider grouping related properties.The addition of readonly brandingStyle property improves type safety. Consider grouping it with the other branding-related property (brandingConfig) for better code organization.
readonly brandingConfig = brandingConfig; + readonly brandingStyle = brandingStyle; readonly AppName = AppName; - readonly brandingStyle = brandingStyle;Also applies to: 56-57
src/app/integrations/travelperk/travelperk-shared/travelperk-advanced-settings/travelperk-advanced-settings.component.ts (1)
65-65
: Consider regrouping configuration properties.The readonly brandingStyle property should be grouped with other configuration-related properties (brandingConfig) for better code organization and maintainability.
readonly brandingConfig = brandingConfig; + readonly brandingStyle = brandingStyle; defaultCategories: TravelperkDestinationAttribuite[]; destinationFieldOptions: SelectFormOption[] = TravelperkAdvancedSettingModel.getDefaultCategory(); defaultMemoOptions: string[] = ['trip_id', 'trip_name', 'traveler_name', 'booker_name', 'merchant_name']; memoPreviewText: string; memoStructure: string[] = []; destinationAttributeNames: SelectFormLabel = { label: 'label', value: '' }; sourceAttributeNames: SelectFormLabel = { label: 'value', value: '' }; lineItems: SelectFormOption[] = TravelperkAdvancedSettingModel.getExpenseGroup(); - readonly brandingStyle = brandingStyle;src/app/integrations/xero/xero-shared/xero-advanced-settings/xero-advanced-settings.component.ts (1)
74-76
: Remove extra blank line after brandingStyle property.The property is correctly grouped with other branding-related properties, but has an unnecessary extra blank line after it.
readonly brandingContent = brandingContent.xero.configuration.advancedSettings; readonly brandingStyle = brandingStyle; - constructor(
src/app/integrations/qbo/qbo-onboarding/qbo-onboarding-connector/qbo-onboarding-connector.component.ts (1)
76-76
: LGTM! Part of consistent styling centralization.The readonly brandingStyle property follows the same pattern implemented across other components, contributing to a more maintainable styling approach.
This change is part of a broader architectural improvement that:
- Centralizes branding styles
- Ensures configuration immutability
- Makes styling more consistent across components
src/app/integrations/qbd/qbd-main/qbd-dashboard/qbd-dashboard.component.ts (1)
75-76
: Remove unnecessary blank line after brandingStyle declaration.While the readonly brandingStyle property is correctly implemented, there's an extra blank line that can be removed to maintain consistent spacing with other property declarations.
readonly brandingStyle = brandingStyle; -
src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-advanced-settings/qbd-direct-advanced-settings.component.ts (1)
Line range hint
85-85
: Well-structured approach to branding style management.The systematic addition of readonly brandingStyle across components demonstrates good architectural practices:
- Ensures consistent styling implementation across different parts of the application
- Prevents accidental modifications to branding styles during runtime
- Centralizes branding configuration, making it easier to maintain and update
Also applies to: 73-73, 75-76, 98-98
src/app/integrations/intacct/intacct-main/intacct-mapping/intacct-mapping.component.html (1)
3-3
: Consider consolidating padding styles.Similar to other mapping components, this combines fixed
tw-py-40-px
with dynamicbrandingStyle.common.mainPaddingStyle
. Consider consolidating all padding into the centralized style configuration for better maintainability.src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-configuration/qbd-direct-configuration.component.html (1)
3-7
: Consider standardizing the padding structure across components.The padding structure here differs from other components:
- Other components: Single div with both
tw-py-40-px
andmainPaddingStyle
- This component: Nested divs with separate padding styles
This structural inconsistency could make maintenance more challenging.
Consider adopting the same structure as other components for consistency:
-<div class="tw-py-40-px"> - <div [ngClass]="brandingStyle.common.mainPaddingStyle"> +<div class="tw-py-40-px" [ngClass]="brandingStyle.common.mainPaddingStyle"> - </div>src/app/shared/components/dashboard/dashboard-export-section/dashboard-export-section.component.html (1)
1-2
: LGTM! Consider standardizing shadow styles in future.The padding style change looks good. However, notice that shadow styles are still conditionally applied based on brandId. Consider standardizing these in a future PR for better maintainability.
src/app/shared/components/dashboard/dashboard-error-section/dashboard-error-section.component.html (1)
1-2
: LGTM! Consider comprehensive styling standardization.The padding style change is good. However, there are multiple brand-specific styles still in use:
- Shadow styles based on brandId
- Text case transformations based on brandId (e.g.,
sentenceCase
vstitlecase
)Consider creating a comprehensive styling strategy that standardizes these variations through the branding configuration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (82)
src/app/branding/c1-style-config.ts
(1 hunks)src/app/branding/fyle-style-config.ts
(1 hunks)src/app/integrations/business-central/business-central-main/business-central-export-log/business-central-export-log.component.html
(1 hunks)src/app/integrations/business-central/business-central-main/business-central-export-log/business-central-export-log.component.ts
(2 hunks)src/app/integrations/business-central/business-central-main/business-central-mapping/business-central-mapping.component.html
(1 hunks)src/app/integrations/business-central/business-central-main/business-central-mapping/business-central-mapping.component.ts
(2 hunks)src/app/integrations/business-central/business-central-onboarding/business-central-onboarding-connector/business-central-onboarding-connector.component.ts
(1 hunks)src/app/integrations/business-central/business-central-shared/business-central-advanced-settings/business-central-advanced-settings.component.ts
(1 hunks)src/app/integrations/business-central/business-central-shared/business-central-export-settings/business-central-export-settings.component.ts
(1 hunks)src/app/integrations/business-central/business-central-shared/business-central-import-settings/business-central-import-settings.component.ts
(1 hunks)src/app/integrations/intacct/intacct-main/intacct-configuration/intacct-configuration.component.html
(1 hunks)src/app/integrations/intacct/intacct-main/intacct-configuration/intacct-configuration.component.ts
(2 hunks)src/app/integrations/intacct/intacct-main/intacct-export-log/intacct-export-log.component.html
(1 hunks)src/app/integrations/intacct/intacct-main/intacct-export-log/intacct-export-log.component.ts
(2 hunks)src/app/integrations/intacct/intacct-main/intacct-mapping/intacct-mapping.component.html
(1 hunks)src/app/integrations/intacct/intacct-main/intacct-mapping/intacct-mapping.component.ts
(2 hunks)src/app/integrations/intacct/intacct-shared/intacct-advanced-settings/intacct-advanced-settings.component.ts
(1 hunks)src/app/integrations/intacct/intacct-shared/intacct-c1-import-settings/intacct-c1-import-settings.component.ts
(1 hunks)src/app/integrations/intacct/intacct-shared/intacct-export-settings/intacct-export-settings.component.ts
(1 hunks)src/app/integrations/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.component.ts
(1 hunks)src/app/integrations/netsuite/netsuite-main/netsuite-configuration/netsuite-configuration.component.html
(1 hunks)src/app/integrations/netsuite/netsuite-main/netsuite-configuration/netsuite-configuration.component.ts
(2 hunks)src/app/integrations/netsuite/netsuite-main/netsuite-export-log/netsuite-export-log.component.html
(1 hunks)src/app/integrations/netsuite/netsuite-main/netsuite-export-log/netsuite-export-log.component.ts
(2 hunks)src/app/integrations/netsuite/netsuite-main/netsuite-mapping/netsuite-mapping.component.html
(1 hunks)src/app/integrations/netsuite/netsuite-main/netsuite-mapping/netsuite-mapping.component.ts
(2 hunks)src/app/integrations/netsuite/netsuite-shared/netsuite-advanced-settings/netsuite-advanced-settings.component.ts
(1 hunks)src/app/integrations/netsuite/netsuite-shared/netsuite-export-settings/netsuite-export-settings.component.ts
(1 hunks)src/app/integrations/netsuite/netsuite-shared/netsuite-import-settings/netsuite-import-settings.component.ts
(1 hunks)src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-configuration/qbd-direct-configuration.component.html
(1 hunks)src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-configuration/qbd-direct-configuration.component.ts
(2 hunks)src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-export-log/qbd-direct-export-log.component.html
(1 hunks)src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-export-log/qbd-direct-export-log.component.ts
(2 hunks)src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-mapping/qbd-direct-mapping.component.html
(1 hunks)src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-mapping/qbd-direct-mapping.component.ts
(2 hunks)src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-connector/qbd-direct-onboarding-connector.component.ts
(1 hunks)src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-pre-requisite/qbd-direct-onboarding-pre-requisite.component.ts
(1 hunks)src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-advanced-settings/qbd-direct-advanced-settings.component.ts
(1 hunks)src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-import-settings/qbd-direct-import-settings.component.ts
(1 hunks)src/app/integrations/qbd/qbd-main/qbd-dashboard/qbd-dashboard.component.html
(1 hunks)src/app/integrations/qbd/qbd-main/qbd-dashboard/qbd-dashboard.component.ts
(2 hunks)src/app/integrations/qbd/qbd-main/qbd-mapping/qbd-generic-mapping/qbd-generic-mapping.component.html
(1 hunks)src/app/integrations/qbd/qbd-main/qbd-mapping/qbd-generic-mapping/qbd-generic-mapping.component.ts
(2 hunks)src/app/integrations/qbd/qbd-shared/qbd-advanced-setting/qbd-advanced-setting.component.ts
(1 hunks)src/app/integrations/qbd/qbd-shared/qbd-export-setting/qbd-export-setting.component.ts
(1 hunks)src/app/integrations/qbd/qbd-shared/qbd-field-mapping/qbd-field-mapping.component.ts
(1 hunks)src/app/integrations/qbo/qbo-main/qbo-configuration/qbo-configuration.component.html
(1 hunks)src/app/integrations/qbo/qbo-main/qbo-configuration/qbo-configuration.component.ts
(2 hunks)src/app/integrations/qbo/qbo-main/qbo-export-log/qbo-export-log.component.html
(1 hunks)src/app/integrations/qbo/qbo-main/qbo-export-log/qbo-export-log.component.ts
(2 hunks)src/app/integrations/qbo/qbo-main/qbo-mapping/qbo-mapping.component.html
(1 hunks)src/app/integrations/qbo/qbo-main/qbo-mapping/qbo-mapping.component.ts
(2 hunks)src/app/integrations/qbo/qbo-onboarding/qbo-clone-settings/qbo-clone-settings.component.ts
(1 hunks)src/app/integrations/qbo/qbo-onboarding/qbo-onboarding-connector/qbo-onboarding-connector.component.ts
(1 hunks)src/app/integrations/qbo/qbo-shared/qbo-advanced-settings/qbo-advanced-settings.component.ts
(1 hunks)src/app/integrations/qbo/qbo-shared/qbo-export-settings/qbo-export-settings.component.ts
(1 hunks)src/app/integrations/qbo/qbo-shared/qbo-import-settings/qbo-import-settings.component.ts
(1 hunks)src/app/integrations/sage300/sage300-main/sage300-export-log/sage300-export-log.component.html
(1 hunks)src/app/integrations/sage300/sage300-main/sage300-export-log/sage300-export-log.component.ts
(2 hunks)src/app/integrations/sage300/sage300-main/sage300-mapping/sage300-mapping.component.html
(1 hunks)src/app/integrations/sage300/sage300-main/sage300-mapping/sage300-mapping.component.ts
(2 hunks)src/app/integrations/sage300/sage300-onboarding/sage300-onboarding-connector/sage300-onboarding-connector.component.ts
(1 hunks)src/app/integrations/sage300/sage300-shared/sage300-advanced-settings/sage300-advanced-settings.component.ts
(1 hunks)src/app/integrations/sage300/sage300-shared/sage300-export-settings/sage300-export-settings.component.ts
(1 hunks)src/app/integrations/sage300/sage300-shared/sage300-import-settings/sage300-import-settings.component.ts
(1 hunks)src/app/integrations/travelperk/travelperk-shared/travelperk-advanced-settings/travelperk-advanced-settings.component.ts
(1 hunks)src/app/integrations/travelperk/travelperk-shared/travelperk-payment-profile-settings/travelperk-payment-profile-settings.component.ts
(1 hunks)src/app/integrations/xero/xero-main/xero-configuration/xero-configuration.component.html
(1 hunks)src/app/integrations/xero/xero-main/xero-configuration/xero-configuration.component.ts
(2 hunks)src/app/integrations/xero/xero-main/xero-export-log/xero-export-log.component.html
(1 hunks)src/app/integrations/xero/xero-main/xero-export-log/xero-export-log.component.ts
(2 hunks)src/app/integrations/xero/xero-main/xero-mapping/xero-mapping.component.html
(1 hunks)src/app/integrations/xero/xero-main/xero-mapping/xero-mapping.component.ts
(2 hunks)src/app/integrations/xero/xero-onboarding/xero-clone-settings/xero-clone-settings.component.ts
(1 hunks)src/app/integrations/xero/xero-onboarding/xero-onboarding-connector/xero-onboarding-connector.component.ts
(1 hunks)src/app/integrations/xero/xero-shared/xero-advanced-settings/xero-advanced-settings.component.ts
(1 hunks)src/app/integrations/xero/xero-shared/xero-export-settings/xero-export-settings.component.ts
(1 hunks)src/app/integrations/xero/xero-shared/xero-import-settings/xero-import-settings.component.ts
(1 hunks)src/app/shared/components/dashboard/dashboard-error-section/dashboard-error-section.component.html
(1 hunks)src/app/shared/components/dashboard/dashboard-error-section/dashboard-error-section.component.ts
(2 hunks)src/app/shared/components/dashboard/dashboard-export-section/dashboard-export-section.component.html
(1 hunks)src/app/shared/components/dashboard/dashboard-export-section/dashboard-export-section.component.ts
(2 hunks)
⛔ Files not processed due to max files limit (6)
- src/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.html
- src/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.ts
- src/app/shared/components/netsuite/core/netsuite-connector/netsuite-connector.component.ts
- src/app/shared/components/netsuite/core/netsuite-subsidiary-mapping/netsuite-subsidiary-mapping.component.ts
- src/app/shared/components/si/core/intacct-connector/intacct-connector.component.ts
- src/app/shared/components/si/core/intacct-location-entity/intacct-location-entity.component.ts
✅ Files skipped from review due to trivial changes (19)
- src/app/integrations/qbd/qbd-shared/qbd-export-setting/qbd-export-setting.component.ts
- src/app/integrations/intacct/intacct-shared/intacct-advanced-settings/intacct-advanced-settings.component.ts
- src/app/integrations/xero/xero-shared/xero-import-settings/xero-import-settings.component.ts
- src/app/integrations/sage300/sage300-shared/sage300-import-settings/sage300-import-settings.component.ts
- src/app/integrations/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.component.ts
- src/app/integrations/business-central/business-central-shared/business-central-advanced-settings/business-central-advanced-settings.component.ts
- src/app/integrations/qbd/qbd-shared/qbd-advanced-setting/qbd-advanced-setting.component.ts
- src/app/integrations/qbo/qbo-shared/qbo-advanced-settings/qbo-advanced-settings.component.ts
- src/app/integrations/netsuite/netsuite-shared/netsuite-import-settings/netsuite-import-settings.component.ts
- src/app/integrations/netsuite/netsuite-shared/netsuite-export-settings/netsuite-export-settings.component.ts
- src/app/integrations/qbo/qbo-onboarding/qbo-clone-settings/qbo-clone-settings.component.ts
- src/app/integrations/business-central/business-central-shared/business-central-export-settings/business-central-export-settings.component.ts
- src/app/integrations/sage300/sage300-shared/sage300-export-settings/sage300-export-settings.component.ts
- src/app/integrations/qbo/qbo-shared/qbo-import-settings/qbo-import-settings.component.ts
- src/app/integrations/intacct/intacct-shared/intacct-c1-import-settings/intacct-c1-import-settings.component.ts
- src/app/integrations/qbd/qbd-shared/qbd-field-mapping/qbd-field-mapping.component.ts
- src/app/integrations/qbo/qbo-shared/qbo-export-settings/qbo-export-settings.component.ts
- src/app/integrations/business-central/business-central-onboarding/business-central-onboarding-connector/business-central-onboarding-connector.component.ts
- src/app/integrations/intacct/intacct-shared/intacct-export-settings/intacct-export-settings.component.ts
🔇 Additional comments (54)
src/app/integrations/sage300/sage300-main/sage300-mapping/sage300-mapping.component.ts (2)
5-5
: LGTM! Clean import statement addition.The import statement follows Angular best practices by grouping related branding imports together.
31-31
: LGTM! Good use of readonly modifier.The property declaration follows the existing pattern in the component and correctly uses the readonly modifier for configuration data.
src/app/integrations/netsuite/netsuite-main/netsuite-mapping/netsuite-mapping.component.ts (2)
5-5
: LGTM! Import statement updated correctly.The addition of
brandingStyle
to the existing branding-related imports is consistent with the PR's objective of centralizing padding styles.
31-31
: LGTM! Property declaration follows established patterns.The new readonly property follows the same pattern as other branding-related properties in the component, maintaining consistency in the codebase.
Let's verify the usage of this new property in the template file:
✅ Verification successful
Property declaration and usage verified successfully
The
brandingStyle
property is correctly used in the template withngClass
directive, confirming proper implementation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of brandingStyle in the template # Expected: Should find usage of brandingStyle.common.mainPaddingStyle in the HTML template rg "brandingStyle\.common\.mainPaddingStyle" "src/app/integrations/netsuite/netsuite-main/netsuite-mapping/netsuite-mapping.component.html"Length of output: 234
src/app/integrations/intacct/intacct-main/intacct-mapping/intacct-mapping.component.ts (2)
5-5
: LGTM! Clean import addition.The
brandingStyle
import is properly grouped with other branding-related imports.
33-33
: LGTM! Consistent property declaration.The
brandingStyle
property is correctly declared as readonly, maintaining consistency with other branding-related properties in the class.Let's verify that the template is properly using this new property:
✅ Verification successful
Template correctly uses the brandingStyle property
The template properly binds the
brandingStyle.common.mainPaddingStyle
through thengClass
directive, confirming correct implementation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the template usage of brandingStyle # Expected: The template should use brandingStyle.common.mainPaddingStyle rg -t html "brandingStyle\.common\.mainPaddingStyle" "src/app/integrations/intacct/intacct-main/intacct-mapping/intacct-mapping.component.html"Length of output: 238
src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-mapping/qbd-direct-mapping.component.ts (2)
5-5
: LGTM! Clean import addition.The import statement is correctly placed and follows Angular conventions.
35-35
: LGTM! Good use of readonly property.The declaration follows the established pattern in the file for branding-related configurations and ensures immutability.
src/app/branding/fyle-style-config.ts (1)
8-10
: LGTM! Good approach to centralizing styles.The addition of
mainPaddingStyle
andmainShadowStyle
properties follows a consistent pattern with the existing configuration. Using Tailwind CSS classes helps maintain consistency and makes the styles more maintainable.src/app/branding/c1-style-config.ts (1)
8-10
: Verify shadow class naming consistency across brands.While the different padding values between brands make sense, the shadow classes use different naming patterns:
- Fyle:
tw-shadow-app-card
- C1:
tw-shadow-shadow-level-1
Please verify if this difference is intentional or if we should maintain consistent class naming across brands.
✅ Verification successful
Shadow class naming difference is intentional and properly configured
The different shadow naming patterns reflect each brand's design system approach:
- Fyle uses semantic naming (
tw-shadow-app-card
) with deeper, brand-colored shadows- C1 uses systematic naming (
tw-shadow-shadow-level-1
) with lighter, neutral shadowsBoth are properly defined in the Tailwind configuration and the difference should be maintained.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for shadow class usage across the codebase echo "Searching for shadow classes..." rg -l "tw-shadow-(app-card|shadow-level-1)" echo -e "\nChecking Tailwind config for shadow definitions..." fd -g "tailwind.config.*" -x cat {} \; | grep -A 5 "boxShadow"Length of output: 3680
Script:
#!/bin/bash # Search for shadow-level-1 definition in configuration files echo "Searching for shadow-level-1 definition..." rg -p "shadow-level-1" --type ts --type js --type json echo -e "\nSearching for shadow configuration files..." fd -g "*shadow*.{ts,js,json}" echo -e "\nSearching for additional Tailwind theme files..." fd -g "*theme*.{ts,js,json}" -x cat {} \;Length of output: 768
src/app/integrations/xero/xero-main/xero-export-log/xero-export-log.component.ts (1)
4-4
: LGTM! Good use of readonly modifier.The addition of the
brandingStyle
property follows the existing pattern of other branding-related properties and maintains immutability with thereadonly
modifier.Also applies to: 25-26
src/app/integrations/qbo/qbo-main/qbo-export-log/qbo-export-log.component.ts (1)
4-4
: LGTM! Consistent implementation across components.The changes maintain consistency with other integration components, following the same pattern of importing and declaring the
brandingStyle
property as readonly.Also applies to: 26-27
src/app/integrations/sage300/sage300-main/sage300-export-log/sage300-export-log.component.ts (1)
4-4
: LGTM! Clean addition of brandingStyle.The changes align well with the PR's objective of centralizing padding styles. The readonly modifier ensures proper immutability.
Also applies to: 26-26
src/app/integrations/netsuite/netsuite-main/netsuite-export-log/netsuite-export-log.component.ts (1)
4-4
: LGTM! Consistent implementation.The changes maintain consistency with other components in the codebase.
Also applies to: 26-26
src/app/integrations/intacct/intacct-main/intacct-export-log/intacct-export-log.component.ts (1)
4-4
: LGTM! Maintains consistent pattern.The implementation follows the established pattern across components.
Also applies to: 28-28
src/app/integrations/business-central/business-central-main/business-central-export-log/business-central-export-log.component.ts (1)
4-4
: LGTM! Consistent implementation with other components.The changes maintain consistency with the established pattern.
Let's verify that the corresponding template files have been updated to use the new brandingStyle:
Also applies to: 26-26
✅ Verification successful
Template correctly implements brandingStyle padding
The template file has been properly updated to use
brandingStyle.common.mainPaddingStyle
and no legacy padding styles remain.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if all export-log component templates use brandingStyle.common.mainPaddingStyle echo "Checking template files for brandingStyle usage..." rg -l "brandingStyle\.common\.mainPaddingStyle" \ src/app/integrations/*/*/export-log/*.html echo -e "\nChecking for any remaining ngClass conditional padding..." rg "ngClass.*padding" \ src/app/integrations/*/*/export-log/*.htmlLength of output: 571
Script:
#!/bin/bash # First, verify the template file exists echo "Checking for template file..." fd -e html . src/app/integrations/business-central echo -e "\nChecking template for brandingStyle usage..." rg "brandingStyle\.common\.mainPaddingStyle" \ src/app/integrations/business-central/business-central-main/business-central-export-log/ echo -e "\nChecking for any remaining ngClass conditional padding..." rg "ngClass.*padding" \ src/app/integrations/business-central/business-central-main/business-central-export-log/Length of output: 3543
src/app/integrations/qbo/qbo-main/qbo-configuration/qbo-configuration.component.ts (1)
3-3
: LGTM! Clean implementation of branding style.The changes follow the established pattern in the codebase, correctly importing and declaring the brandingStyle property as readonly.
Also applies to: 26-26
src/app/integrations/xero/xero-main/xero-configuration/xero-configuration.component.ts (1)
3-3
: LGTM! Consistent implementation across components.The changes mirror the implementation in other configuration components, maintaining consistency in the codebase.
Also applies to: 26-26
src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-export-log/qbd-direct-export-log.component.ts (1)
5-7
: Verify the different source for brandingConfig import.There seems to be an inconsistency in the import sources:
brandingStyle
is imported from 'src/app/branding/branding-config'brandingConfig
is imported from 'src/app/branding/c1-contents-config'Other components import all branding-related configs from 'branding-config'.
src/app/integrations/netsuite/netsuite-main/netsuite-configuration/netsuite-configuration.component.ts (1)
3-3
: LGTM! Consistent implementation of branding style.The changes align with the implementation in other configuration components.
Also applies to: 26-26
src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-configuration/qbd-direct-configuration.component.ts (1)
5-5
: LGTM! Clean addition of brandingStyle.The changes properly integrate the brandingStyle property while maintaining immutability with readonly modifier.
Also applies to: 31-32
src/app/integrations/intacct/intacct-main/intacct-configuration/intacct-configuration.component.ts (1)
4-4
: LGTM! Consistent implementation of brandingStyle.The changes align with the pattern seen in other components, maintaining immutability and proper organization.
Also applies to: 21-22
src/app/integrations/business-central/business-central-main/business-central-mapping/business-central-mapping.component.ts (1)
5-5
: LGTM! Clean integration of brandingStyle.The changes maintain consistency with other components while preserving the existing mapping functionality.
Also applies to: 30-31
src/app/shared/components/dashboard/dashboard-export-section/dashboard-export-section.component.ts (1)
2-2
: LGTM! Consistent with branding configuration pattern.The addition of
brandingStyle
property follows the existing pattern of branding configuration properties in the component.Also applies to: 47-47
src/app/integrations/sage300/sage300-onboarding/sage300-onboarding-connector/sage300-onboarding-connector.component.ts (1)
37-37
: LGTM! Clean integration of branding styles.The addition of readonly
brandingStyle
property is clean and doesn't interfere with existing component functionality.src/app/integrations/travelperk/travelperk-shared/travelperk-payment-profile-settings/travelperk-payment-profile-settings.component.ts (1)
58-58
: LGTM! Property placement is consistent.The readonly brandingStyle property is correctly placed with other configuration properties.
src/app/integrations/xero/xero-shared/xero-export-settings/xero-export-settings.component.ts (1)
93-93
: LGTM! Good practice using readonly for branding styles.Making the branding style property readonly prevents accidental modifications and aligns with the centralization of styling configurations.
src/app/integrations/business-central/business-central-shared/business-central-import-settings/business-central-import-settings.component.ts (1)
67-67
: LGTM! Consistent with the centralized styling approach.The readonly modifier ensures immutability of the branding style configuration.
src/app/shared/components/dashboard/dashboard-error-section/dashboard-error-section.component.ts (1)
2-2
: LGTM! Appropriate addition of branding styles to shared component.The changes follow the consistent pattern of centralizing styles, with good code formatting.
Also applies to: 108-109
src/app/integrations/xero/xero-onboarding/xero-onboarding-connector/xero-onboarding-connector.component.ts (1)
85-85
: LGTM! Good practice using readonly modifier.The addition of the readonly modifier for brandingStyle property ensures immutability and prevents accidental modifications during the component's lifecycle.
src/app/integrations/sage300/sage300-shared/sage300-advanced-settings/sage300-advanced-settings.component.ts (1)
73-73
: LGTM! Consistent with the application-wide pattern.The readonly brandingStyle property follows the same pattern used across other components, maintaining consistency in the codebase.
src/app/integrations/qbd/qbd-main/qbd-dashboard/qbd-dashboard.component.ts (1)
12-12
: LGTM! Import added correctly.The brandingStyle import is properly added alongside other branding-related imports.
src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-advanced-settings/qbd-direct-advanced-settings.component.ts (1)
98-98
: LGTM! Follows established pattern.The readonly brandingStyle property is correctly implemented, maintaining consistency with other components in the application.
src/app/integrations/netsuite/netsuite-shared/netsuite-advanced-settings/netsuite-advanced-settings.component.ts (1)
106-106
: LGTM! Follows TypeScript best practices.Making brandingStyle readonly aligns with TypeScript best practices for configuration properties and ensures consistent behavior across components.
src/app/integrations/xero/xero-onboarding/xero-clone-settings/xero-clone-settings.component.ts (1)
146-146
: LGTM! Maintains consistency across components.The readonly modifier on brandingStyle property maintains consistency with other components and ensures immutability of the branding configuration.
src/app/integrations/xero/xero-main/xero-configuration/xero-configuration.component.html (1)
4-4
: LGTM! Verify brandingStyle initialization.The change aligns with the centralized styling approach. Please ensure that the brandingStyle property is properly initialized in the component.
✅ Verification successful
✓ brandingStyle is properly initialized
The component correctly imports and initializes the readonly brandingStyle property from the centralized branding configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify brandingStyle initialization in component ast-grep --pattern 'class $_ { $$$ readonly brandingStyle = $_; $$$ }' src/app/integrations/xero/xero-main/xero-configuration/xero-configuration.component.tsLength of output: 165
Script:
#!/bin/bash # Check component file content cat src/app/integrations/xero/xero-main/xero-configuration/xero-configuration.component.ts # Search for brandingStyle declarations with broader pattern ast-grep --pattern 'brandingStyle = $_' # Search for constructor patterns ast-grep --pattern 'constructor($$$) { $$$ }'Length of output: 1367
src/app/integrations/qbo/qbo-main/qbo-configuration/qbo-configuration.component.html (1)
4-4
: LGTM! Consistent implementation.The change maintains consistency with other components in the centralized styling approach.
src/app/integrations/netsuite/netsuite-main/netsuite-configuration/netsuite-configuration.component.html (1)
4-4
: LGTM! Maintains consistent styling approach.The change follows the established pattern of centralizing padding styles.
src/app/integrations/xero/xero-main/xero-mapping/xero-mapping.component.html (1)
3-3
: Verify padding combination.The combination of fixed
tw-py-40-px
class with dynamicbrandingStyle.common.mainPaddingStyle
might lead to conflicting paddings. Consider moving the vertical padding to the centralized style if it's meant to be consistent across components.src/app/integrations/netsuite/netsuite-main/netsuite-mapping/netsuite-mapping.component.html (1)
3-3
: Same padding style conflict as in qbo-mapping.component.htmlsrc/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-mapping/qbd-direct-mapping.component.html (1)
3-3
: Same padding style conflict as in qbo-mapping.component.htmlsrc/app/integrations/intacct/intacct-main/intacct-configuration/intacct-configuration.component.html (1)
3-7
: Same structural inconsistency as in qbd-direct-configuration.component.htmlsrc/app/integrations/sage300/sage300-main/sage300-mapping/sage300-mapping.component.html (1)
3-3
: LGTM! Verify brandingStyle property initialization.The change successfully centralizes the padding style configuration, improving maintainability.
✅ Verification successful
Property is correctly initialized and used ✅
The
brandingStyle
property is properly imported from the branding config and correctly initialized as a readonly property in the component.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that brandingStyle is properly initialized in the component ast-grep --pattern 'class Sage300MappingComponent { $$$ readonly brandingStyle = $_; $$$ }'Length of output: 99
Script:
#!/bin/bash # Find the component's TypeScript file fd -e ts sage300-mapping.component # Search for brandingStyle property with context rg -A 5 -B 5 "brandingStyle" --type tsLength of output: 69529
Script:
#!/bin/bash # Check the specific component file for brandingStyle cat src/app/integrations/sage300/sage300-main/sage300-mapping/sage300-mapping.component.tsLength of output: 2283
src/app/integrations/business-central/business-central-main/business-central-mapping/business-central-mapping.component.html (1)
5-5
: LGTM! Consistent implementation.The change maintains consistency with other components in using the centralized padding style.
src/app/integrations/qbo/qbo-main/qbo-export-log/qbo-export-log.component.html (1)
4-4
: LGTM! Verify feature flag configuration.The change successfully implements the centralized padding style while maintaining the feature flag functionality.
✅ Verification successful
Feature flag configuration verified and correctly implemented
The component properly imports and initializes both
brandingStyle
andbrandingFeatureConfig
as readonly properties, ensuring consistent branding behavior.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that both brandingStyle and brandingFeatureConfig are properly initialized ast-grep --pattern 'class QboExportLogComponent { $$$ readonly brandingStyle = $_; readonly brandingFeatureConfig = $_; $$$ }'Length of output: 136
Script:
#!/bin/bash # First, find the component file fd "qbo-export-log.component.ts" --type f # Then examine its content echo "=== Component Content ===" rg -A 5 "class QboExportLogComponent" $(fd "qbo-export-log.component.ts" --type f) # Look for branding-related property declarations echo "=== Branding Properties ===" rg "brandingStyle|brandingFeatureConfig" $(fd "qbo-export-log.component.ts" --type f)Length of output: 1104
src/app/integrations/netsuite/netsuite-main/netsuite-export-log/netsuite-export-log.component.html (1)
4-4
: LGTM! Consistent implementation.The change follows the established pattern for centralizing padding styles while maintaining feature flag functionality.
src/app/integrations/xero/xero-main/xero-export-log/xero-export-log.component.html (1)
4-4
: LGTM! Style conversion looks good.The padding style has been successfully centralized using
brandingStyle.common.mainPaddingStyle
while maintaining the vertical padding.src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-export-log/qbd-direct-export-log.component.html (1)
4-4
: LGTM! Consistent with other export log components.The padding style conversion matches the pattern used in other export log components.
src/app/integrations/business-central/business-central-main/business-central-export-log/business-central-export-log.component.html (1)
9-9
: LGTM! Style conversion maintains loading state handling.The padding style has been centralized while properly maintaining the loading state conditional rendering.
src/app/integrations/sage300/sage300-main/sage300-export-log/sage300-export-log.component.html (1)
9-9
: LGTM! Consistent with other export log components.The padding style conversion follows the same pattern as other components while maintaining loading state handling.
src/app/integrations/qbd/qbd-main/qbd-dashboard/qbd-dashboard.component.html (1)
8-8
: LGTM! Padding style standardization looks good.The change to use
brandingStyle.common.mainPaddingStyle
aligns with the PR objective and maintains consistency across components.src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-import-settings/qbd-direct-import-settings.component.ts (1)
111-111
: LGTM! Good practice adding readonly modifier.Making
brandingStyle
readonly prevents accidental modifications and ensures consistent branding across the application.src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-connector/qbd-direct-onboarding-connector.component.ts (1)
82-82
: LGTM! Consistent with codebase patterns.Making
brandingStyle
readonly aligns with the immutability pattern being implemented across components.src/app/integrations/qbo/qbo-main/qbo-mapping/qbo-mapping.component.html (1)
3-3
: LGTM! Improved template maintainability.Centralizing padding styles in
brandingStyle.common.mainPaddingStyle
reduces template complexity and ensures consistent styling across components.
@@ -1,7 +1,7 @@ | |||
<div *ngIf="isGradientAllowed" class="tw-absolute tw-bg-header-1 tw-h-84-px tw-w-[100%]"></div> | |||
<div *ngIf="isGradientAllowed" class="tw-absolute tw-bg-header-2 tw-h-84-px tw-w-[100%]"></div> | |||
|
|||
<div class="tw-py-40-px" [ngClass]="{'tw-px-120-px': brandingConfig.brandId === 'fyle', 'tw-px-60-px': brandingConfig.brandId === 'co'}"> | |||
<div class="tw-py-40-px" [ngClass]="brandingStyle.common.mainPaddingStyle"> |
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.
💡 Codebase verification
Extend padding style pattern to child components
Several child components still use the old conditional padding style and should be updated to use brandingStyle.common.mainPaddingStyle
:
- Export log child components in:
- Xero (complete/skipped)
- QBO (complete/skipped)
- NetSuite (complete/skipped)
- QBD Direct (complete/skipped)
- Intacct (complete/skipped)
🔗 Analysis chain
LGTM! Consider extending this pattern.
The change successfully implements the centralized padding style. Consider searching for similar patterns in other components that could benefit from this refactoring.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other components that might benefit from this refactoring
echo "Searching for components with conditional padding styles..."
rg -l "ngClass.*brandingConfig\.brandId.*'fyle'.*'co'" --type html
echo "Searching for components already using the new pattern..."
rg -l "ngClass.*brandingStyle\.common\.mainPaddingStyle" --type html
Length of output: 4646
@@ -1,7 +1,7 @@ | |||
<div *ngIf="isLoading" class="tw-flex tw-justify-center tw-items-center tw-h-screen"> | |||
<app-loader></app-loader> | |||
</div> | |||
<div *ngIf="!isLoading" class="tw-py-16-px" [ngClass]="{'tw-px-120-px': brandingConfig.brandId === 'fyle', 'tw-px-60-px': brandingConfig.brandId === 'co'}"> | |||
<div *ngIf="!isLoading" class="tw-py-16-px" [ngClass]="brandingStyle.common.mainPaddingStyle"> |
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.
💡 Codebase verification
Inconsistent vertical padding detected
The qbd-generic-mapping
component uses tw-py-16-px
while all other mapping components across different integrations (QBO, Xero, Intacct, etc.) consistently use tw-py-40-px
. Consider aligning with the standard tw-py-40-px
padding for consistency.
🔗 Analysis chain
Verify if different vertical padding is intentional.
While the horizontal padding has been centralized using brandingStyle.common.mainPaddingStyle
, this component uses tw-py-16-px
for vertical padding, whereas other components use tw-py-40-px
. Please confirm if this difference is intentional.
Run this script to check vertical padding consistency across similar components:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check vertical padding patterns in mapping components
# Look for tw-py-{n}-px patterns in mapping component templates
echo "Checking vertical padding patterns in mapping components..."
rg -g '**/*mapping*.component.html' 'tw-py-\d+-px'
Length of output: 2053
mainPaddingStyle: 'tw-px-60-px', | ||
mainShadowStyle: 'tw-shadow-shadow-level-1' |
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.
better names, may be we can huddle and brainstorm tomo
Description
feat: common main page padding style conversion
Clickup
https://app.clickup.com/t/
Summary by CodeRabbit
Based on the comprehensive summary, here are the release notes:
New Features
mainPaddingStyle
andmainShadowStyle
for consistent stylingImprovements
Technical Updates
brandingStyle.common
propertiesPerformance