feat: common main pages shadow style conversion #1164
Conversation
WalkthroughThis pull request introduces a comprehensive refactoring of styling configurations across multiple components. The primary change involves centralizing style management by introducing a new Changes
Sequence DiagramsequenceDiagram
participant BrandingConfig
participant Component
participant Template
BrandingConfig->>BrandingConfig: Define brandingStyle
Component->>Component: Import brandingStyle
Component->>Template: Apply brandingStyle.common.mainShadowStyle
Template->>Template: Render with consistent styling
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
![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
🧹 Nitpick comments (10)
src/app/integrations/qbo/qbo-main/qbo-export-log/qbo-skipped-export-log/qbo-skipped-export-log.component.ts (1)
12-12: Consider standardizing import statement placement.While the implementation is correct, consider maintaining consistent import ordering across components for better maintainability. The
brandingConfigimports in other components are typically grouped with the Angular imports at the top.Also applies to: 57-57
src/app/shared/components/helper/mapping/generic-mapping-v2/generic-mapping-v2.component.ts (1)
1-1: Remove unused OnChanges import.The component imports OnChanges but doesn't implement it. Consider removing the unused import.
-import { Component, EventEmitter, Input, OnChanges, OnInit, Output } from '@angular/core'; +import { Component, EventEmitter, Input, OnInit, Output } from '@angular/core';src/app/integrations/qbo/qbo-main/qbo-export-log/qbo-complete-export-log/qbo-complete-export-log.component.ts (1)
13-13: LGTM! Consider grouping related properties.The addition of
brandingStylealigns with the standardization effort. Consider grouping all branding-related properties together for better code organization:- readonly brandingConfig = brandingConfig; - readonly brandingContent = brandingContent.exportLog; searchQuery: string | null; private searchQuerySubject = new Subject<string>(); - readonly brandingStyle = brandingStyle; + // Branding related properties + readonly brandingConfig = brandingConfig; + readonly brandingContent = brandingContent.exportLog; + readonly brandingStyle = brandingStyle;Also applies to: 63-63
src/app/integrations/netsuite/netsuite-main/netsuite-export-log/netsuite-complete-export-logs/netsuite-complete-export-logs.component.ts (1)
4-4: LGTM! Consider grouping related properties.The addition of
brandingStylealigns with the standardization effort. Consider grouping all branding-related properties together for better code organization:- readonly brandingConfig = brandingConfig; searchQuery: string | null; private searchQuerySubject = new Subject<string>(); - readonly brandingContent = brandingContent.exportLog; - readonly brandingStyle = brandingStyle; + // Branding related properties + readonly brandingConfig = brandingConfig; + readonly brandingContent = brandingContent.exportLog; + readonly brandingStyle = brandingStyle;Also applies to: 61-61
src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-export-log/qbd-direct-complete-export-log/qbd-direct-complete-export-log.component.ts (2)
5-6: Consider consolidating branding imports.The branding-related imports are split unnecessarily. Consider consolidating them:
-import { brandingContent, brandingStyle } from 'src/app/branding/branding-config'; -import { brandingConfig } from 'src/app/branding/c1-contents-config'; +import { brandingContent, brandingStyle, brandingConfig } from 'src/app/branding/branding-config';
67-67: LGTM! Consider grouping related properties.The addition of
brandingStylealigns with the standardization effort. Consider grouping all branding-related properties together for better code organization:- readonly brandingConfig = brandingConfig; - readonly brandingContent = brandingContent.exportLog; searchQuery: string | null; private searchQuerySubject = new Subject<string>(); - readonly brandingStyle = brandingStyle; + // Branding related properties + readonly brandingConfig = brandingConfig; + readonly brandingContent = brandingContent.exportLog; + readonly brandingStyle = brandingStyle;src/app/integrations/xero/xero-main/xero-export-log/xero-complete-export-log/xero-complete-export-log.component.ts (1)
4-4: LGTM! Consider grouping related properties.The addition of
brandingStylealigns with the standardization effort. Consider grouping all branding-related properties together for better code organization:- readonly brandingConfig = brandingConfig; - readonly brandingContent = brandingContent.exportLog; searchQuery: string | null; private searchQuerySubject = new Subject<string>(); xeroShortCode: string; hideCalendar: boolean; - readonly brandingStyle = brandingStyle; + // Branding related properties + readonly brandingConfig = brandingConfig; + readonly brandingContent = brandingContent.exportLog; + readonly brandingStyle = brandingStyle;Also applies to: 69-69
src/app/shared/components/helper/mapping/mapping-card-header/mapping-card-header.component.html (1)
Line range hint
3-14: Overall implementation of shadow style conversion looks great!The changes demonstrate:
- Consistent use of
brandingStyle.common.mainShadowStyleacross main pages- Appropriate use of specialized shadow styles where needed (e.g., mapping components)
- Clean implementation that improves maintainability by centralizing style management
Consider documenting the shadow style hierarchy (common vs. specialized) in the style guide to help other developers understand when to use each type.
src/app/integrations/intacct/intacct-main/intacct-export-log/intacct-completed-export-log/intacct-completed-export-log.component.html (1)
Line range hint
5-38: Consider adding documentation for the new style system.Since this is part of a larger style system refactoring, it would be helpful to document the available styles and their usage.
Consider adding a README.md in the branding directory explaining:
- Available style properties
- Usage examples
- Migration guide from the old conditional system
src/app/integrations/landing/landing.component.html (1)
23-23: LGTM! Comprehensive shadow style conversion.The changes successfully implement the centralized styling approach across all integration sections. The consistent use of
brandingStyle.common.mainShadowStyleandbrandingStyle.common.mainPaddingStyleimproves maintainability.Consider extracting the common integration app card structure into a reusable component to reduce repetition and further improve maintainability. This would help centralize not just the styling but also the structure of these similar sections.
Also applies to: 37-37, 44-44, 50-50, 56-56, 62-62, 69-69, 76-76, 83-83, 103-103, 122-122
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
src/app/branding/c1-style-config.ts(1 hunks)src/app/branding/fyle-style-config.ts(1 hunks)src/app/integrations/intacct/intacct-main/intacct-export-log/intacct-completed-export-log/intacct-completed-export-log.component.html(1 hunks)src/app/integrations/intacct/intacct-main/intacct-export-log/intacct-completed-export-log/intacct-completed-export-log.component.ts(2 hunks)src/app/integrations/intacct/intacct-main/intacct-export-log/intacct-skip-export-log/intacct-skip-export-log.component.html(1 hunks)src/app/integrations/intacct/intacct-main/intacct-export-log/intacct-skip-export-log/intacct-skip-export-log.component.ts(2 hunks)src/app/integrations/landing/landing.component.html(4 hunks)src/app/integrations/landing/landing.component.ts(2 hunks)src/app/integrations/netsuite/netsuite-main/netsuite-export-log/netsuite-complete-export-logs/netsuite-complete-export-logs.component.html(1 hunks)src/app/integrations/netsuite/netsuite-main/netsuite-export-log/netsuite-complete-export-logs/netsuite-complete-export-logs.component.ts(2 hunks)src/app/integrations/netsuite/netsuite-main/netsuite-export-log/netsuite-skipped-export-log/netsuite-skipped-export-log.component.html(1 hunks)src/app/integrations/netsuite/netsuite-main/netsuite-export-log/netsuite-skipped-export-log/netsuite-skipped-export-log.component.ts(2 hunks)src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-export-log/qbd-direct-complete-export-log/qbd-direct-complete-export-log.component.html(1 hunks)src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-export-log/qbd-direct-complete-export-log/qbd-direct-complete-export-log.component.ts(2 hunks)src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-export-log/qbd-direct-skipped-export-log/qbd-direct-skipped-export-log.component.html(1 hunks)src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-export-log/qbd-direct-skipped-export-log/qbd-direct-skipped-export-log.component.ts(2 hunks)src/app/integrations/qbo/qbo-main/qbo-export-log/qbo-complete-export-log/qbo-complete-export-log.component.html(1 hunks)src/app/integrations/qbo/qbo-main/qbo-export-log/qbo-complete-export-log/qbo-complete-export-log.component.ts(2 hunks)src/app/integrations/qbo/qbo-main/qbo-export-log/qbo-skipped-export-log/qbo-skipped-export-log.component.html(1 hunks)src/app/integrations/qbo/qbo-main/qbo-export-log/qbo-skipped-export-log/qbo-skipped-export-log.component.ts(2 hunks)src/app/integrations/xero/xero-main/xero-export-log/xero-complete-export-log/xero-complete-export-log.component.html(1 hunks)src/app/integrations/xero/xero-main/xero-export-log/xero-complete-export-log/xero-complete-export-log.component.ts(2 hunks)src/app/integrations/xero/xero-main/xero-export-log/xero-skipped-export-log/xero-skipped-export-log.component.html(1 hunks)src/app/integrations/xero/xero-main/xero-export-log/xero-skipped-export-log/xero-skipped-export-log.component.ts(2 hunks)src/app/shared/components/dashboard/dashboard-error-section/dashboard-error-section.component.html(1 hunks)src/app/shared/components/dashboard/dashboard-export-section/dashboard-export-section.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/helper/mapping/generic-mapping-v2/generic-mapping-v2.component.html(1 hunks)src/app/shared/components/helper/mapping/generic-mapping-v2/generic-mapping-v2.component.ts(2 hunks)src/app/shared/components/helper/mapping/mapping-card-header/mapping-card-header.component.html(2 hunks)src/app/shared/components/helper/mapping/mapping-card-header/mapping-card-header.component.ts(2 hunks)src/app/shared/components/helper/onboarding-done/onboarding-done.component.html(1 hunks)src/app/shared/components/helper/onboarding-done/onboarding-done.component.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/app/integrations/intacct/intacct-main/intacct-export-log/intacct-skip-export-log/intacct-skip-export-log.component.ts
🧰 Additional context used
🪛 GitHub Check: lint
src/app/integrations/intacct/intacct-main/intacct-export-log/intacct-completed-export-log/intacct-completed-export-log.component.ts
[failure] 72-72:
Trailing spaces not allowed
🪛 eslint
src/app/integrations/intacct/intacct-main/intacct-export-log/intacct-completed-export-log/intacct-completed-export-log.component.ts
[error] 72-72: Trailing spaces not allowed.
(no-trailing-spaces)
🪛 GitHub Actions: TypeScript Lint Check
src/app/integrations/intacct/intacct-main/intacct-export-log/intacct-completed-export-log/intacct-completed-export-log.component.ts
[error] 72-72: Trailing spaces not allowed (no-trailing-spaces)
🔇 Additional comments (27)
src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-export-log/qbd-direct-skipped-export-log/qbd-direct-skipped-export-log.component.ts (2)
5-5: LGTM!The import statement is correctly structured and follows Angular conventions.
59-60: LGTM! Good use of readonly.The
brandingStyleproperty is correctly declared as readonly, ensuring immutability and consistent with the existing pattern of branding-related properties in the component.src/app/branding/fyle-style-config.ts (1)
11-14: LGTM! Well-structured style configuration addition.The new
mappingproperty follows the established pattern and maintains consistency with other style configurations.src/app/branding/c1-style-config.ts (1)
11-14: LGTM! Consistent style implementation.The
mappingconfiguration maintains parity withfyle-style-config.tswhile using C1-specific shadow styling.src/app/shared/components/helper/onboarding-done/onboarding-done.component.ts (1)
2-2: LGTM! Clean implementation of branding style.The addition follows the component's established pattern for branding properties and supports the centralized styling approach.
Also applies to: 19-20
src/app/shared/components/helper/mapping/mapping-card-header/mapping-card-header.component.ts (1)
2-2: LGTM! Clean implementation of branding style.The addition follows the component's established pattern for branding properties and supports the centralized styling approach.
Also applies to: 25-26
src/app/integrations/netsuite/netsuite-main/netsuite-export-log/netsuite-skipped-export-log/netsuite-skipped-export-log.component.ts (1)
4-4: LGTM! Consistent with the application-wide styling refactor.The addition of
brandingStylefollows the established pattern and maintains immutability with the readonly modifier.Also applies to: 53-53
src/app/shared/components/helper/mapping/generic-mapping-v2/generic-mapping-v2.component.ts (1)
4-4: LGTM! Consistent implementation in shared component.The brandingStyle implementation follows the established pattern correctly.
Also applies to: 83-83
src/app/integrations/intacct/intacct-main/intacct-export-log/intacct-completed-export-log/intacct-completed-export-log.component.ts (1)
12-12: LGTM!The addition of
brandingStyleimport and property aligns with the PR's objective of centralizing shadow styles.Also applies to: 73-73
src/app/integrations/landing/landing.component.ts (1)
14-14: LGTM!The addition of
brandingStyleimport and property maintains consistency with the centralized styling approach.Also applies to: 95-96
src/app/shared/components/helper/onboarding-done/onboarding-done.component.html (1)
4-4: LGTM! Good simplification.Replacing conditional shadow classes with
brandingStyle.common.mainShadowStyleimproves template readability and maintainability.src/app/integrations/xero/xero-main/xero-export-log/xero-skipped-export-log/xero-skipped-export-log.component.html (1)
5-5: LGTM!The shadow style conversion maintains consistency with other components using the centralized
brandingStyle.common.mainShadowStyle.src/app/integrations/qbo/qbo-main/qbo-export-log/qbo-skipped-export-log/qbo-skipped-export-log.component.html (1)
5-5: LGTM! Shadow style implementation looks good.The change to use
brandingStyle.common.mainShadowStylealigns with the centralized styling approach.src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-export-log/qbd-direct-skipped-export-log/qbd-direct-skipped-export-log.component.html (1)
5-5: Implementation matches the standardized pattern.src/app/integrations/intacct/intacct-main/intacct-export-log/intacct-skip-export-log/intacct-skip-export-log.component.html (1)
5-5: Implementation follows the standard pattern.src/app/integrations/netsuite/netsuite-main/netsuite-export-log/netsuite-skipped-export-log/netsuite-skipped-export-log.component.html (1)
5-5: Implementation adheres to the standard pattern.src/app/shared/components/helper/mapping/mapping-card-header/mapping-card-header.component.html (1)
3-3: LGTM! Specialized shadow style implementation looks good.The use of
brandingStyle.mapping.mappingHeaderBoxShadowis appropriate for this specialized component, and it's consistently applied to both card sections.Let's verify the usage of this specialized shadow style:
Also applies to: 14-14
✅ Verification successful
Specialized shadow style implementation is correctly isolated
The
brandingStyle.mapping.mappingHeaderBoxShadowis intentionally used only in the mapping-card-header component, while other mapping components either use different shadow styles or none at all, as per their specific design requirements. This implementation is consistent with the component's specialized nature.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other mapping components that might need the same shadow style rg -l "mapping.*BoxShadow" --type htmlLength of output: 136
Script:
#!/bin/bash # Find all mapping-related component files fd mapping.*component.html # Check for any shadow-related classes or styles in these files rg "shadow|BoxShadow" $(fd mapping.*component.html)Length of output: 6197
src/app/integrations/intacct/intacct-main/intacct-export-log/intacct-completed-export-log/intacct-completed-export-log.component.html (1)
5-5: LGTM! Consistent style refactoring.The change improves maintainability by centralizing shadow styles and removing conditional logic.
Let's verify the proper initialization of
brandingStyleacross components:✅ Verification successful
Shadow styles properly centralized and initialized
The verification confirms that shadow styles are properly centralized in brand-specific style configurations and correctly initialized in components through the brandingStyle property. The implementation indeed improves maintainability as noted in the review.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for brandingStyle initialization in component files ast-grep --pattern 'readonly brandingStyle = brandingStyle;' # Verify the shadow style values match previous conditional values rg -A 2 'mainShadowStyle.*=.*{' src/app/branding/Length of output: 220
Script:
#!/bin/bash # Find brandingStyle property declarations in components ast-grep --pattern 'brandingStyle: $_' # Find brandingStyle property usage ast-grep --pattern 'readonly brandingStyle' # Search for shadow style definitions with proper escaping rg "mainShadowStyle.*\{" src/app/branding/ # Find component class declarations using brandingStyle ast-grep --pattern 'class $_ { $$$ brandingStyle $$$ }'Length of output: 18537
Script:
#!/bin/bash # Find branding style definition files fd "branding.*style" --type f # Search for mainShadowStyle definition rg -A 3 "mainShadowStyle.*=" --type tsLength of output: 72
Script:
#!/bin/bash # Search for style-related files fd "style|theme|branding" --type f # Look for shadow-related configurations rg -l "shadow" --type ts # Search for style objects or configurations ast-grep --pattern 'const $_ = { $$$ shadow $$$ }'Length of output: 626
Script:
#!/bin/bash # Show shadow style implementations from style config files rg -A 5 "mainShadowStyle" src/app/branding/c1-style-config.ts src/app/branding/fyle-style-config.ts # Show the entire style config files for context cat src/app/branding/c1-style-config.ts src/app/branding/fyle-style-config.tsLength of output: 1916
src/app/integrations/netsuite/netsuite-main/netsuite-export-log/netsuite-complete-export-logs/netsuite-complete-export-logs.component.html (1)
5-5: Changes match the established pattern.src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-export-log/qbd-direct-complete-export-log/qbd-direct-complete-export-log.component.html (1)
5-5: Changes match the established pattern.src/app/integrations/qbo/qbo-main/qbo-export-log/qbo-complete-export-log/qbo-complete-export-log.component.html (1)
5-5: Changes match the established pattern.src/app/integrations/xero/xero-main/xero-export-log/xero-complete-export-log/xero-complete-export-log.component.html (1)
5-5: Changes match the established pattern.src/app/shared/components/helper/mapping/generic-mapping-v2/generic-mapping-v2.component.html (1)
10-10: LGTM! Shadow style conversion looks good.The change successfully implements the centralized shadow styling approach using
brandingStyle.common.mainShadowStyle.Please ensure the
brandingStyleproperty is properly initialized in the component. Run this script to verify:✅ Verification successful
Property initialization verified and correct
The
brandingStyleproperty is properly initialized as a readonly property, imported from the branding configuration file. The implementation ensures the shadow styles are available when needed by the template.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify brandingStyle property initialization in the component ast-grep --pattern 'class GenericMappingV2Component { $$$ brandingStyle: $_ $$$ }'Length of output: 90
Script:
#!/bin/bash # Find the component file fd "generic-mapping-v2.component.ts" # Check property initialization in the component ast-grep --pattern 'class GenericMappingV2Component { $$$ brandingStyle$_ $$$ }' # Check constructor initialization ast-grep --pattern 'class GenericMappingV2Component { $$$ constructor($_) { $$$ this.brandingStyle$_ $$$ } $$$ }'Length of output: 353
Script:
#!/bin/bash # View the component file content cat src/app/shared/components/helper/mapping/generic-mapping-v2/generic-mapping-v2.component.ts # Alternative pattern for constructor injection ast-grep --pattern 'constructor( $$$ private $_: BrandingService$_ $$$ ) {'Length of output: 5706
src/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.html (1)
2-2: LGTM! Shadow style conversion is consistent.The change successfully implements the centralized shadow styling approach, maintaining consistency with other components.
src/app/shared/components/dashboard/dashboard-export-section/dashboard-export-section.component.html (1)
2-2: LGTM! Shadow style conversion maintains consistency.The change successfully implements the centralized shadow styling approach, aligning with the project-wide style conversion.
src/app/shared/components/dashboard/dashboard-error-section/dashboard-error-section.component.html (1)
2-2: LGTM! Shadow style conversion is properly implemented.The change successfully implements the centralized shadow styling approach, maintaining consistency with the project-wide style conversion.
src/app/integrations/xero/xero-main/xero-export-log/xero-skipped-export-log/xero-skipped-export-log.component.ts (1)
4-4: LGTM! Clean implementation of the branding style changes.The changes follow existing patterns in the codebase and align well with the broader styling refactoring effort. The readonly property ensures immutability which is a good practice.
Also applies to: 55-56
|
|
||
| readonly brandingStyle = brandingStyle; |
There was a problem hiding this comment.
Fix trailing whitespace.
Remove the trailing space on line 72 to resolve the linting error.
-
readonly brandingStyle = brandingStyle;📝 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.
| readonly brandingStyle = brandingStyle; | |
| readonly brandingStyle = brandingStyle; |
🧰 Tools
🪛 GitHub Check: lint
[failure] 72-72:
Trailing spaces not allowed
🪛 eslint
[error] 72-72: Trailing spaces not allowed.
(no-trailing-spaces)
🪛 GitHub Actions: TypeScript Lint Check
[error] 72-72: Trailing spaces not allowed (no-trailing-spaces)
|
2 participants
Description
feat: common main pages shadow style conversion
Clickup
https://app.clickup.com/t/
Summary by CodeRabbit
Based on the comprehensive summary, here are the updated release notes:
New Features
brandingStyleconfiguration for managing dynamic styling across multiple components.mappingproperty to branding configuration withmappingHeaderBoxShadowstyle.Improvements
Technical Updates
brandingStyleproperty.The changes primarily focus on enhancing the styling and branding configuration management across the application.