feat: Change to sage logo #3986
Conversation
WalkthroughI replaced Fyle logo assets with Sage logo in templates and icon registry, and updated header/logo dimensions in two SCSS files. No logic or control-flow changes, only assets and styling. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
src/assets/svg/fyle-logo-light.svgis excluded by!**/*.svgsrc/assets/svg/fyle-logo.svgis excluded by!**/*.svgsrc/assets/svg/sage-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (5)
src/app/auth/switch-org/switch-org.page.html(1 hunks)src/app/auth/switch-org/switch-org.page.scss(1 hunks)src/app/shared/components/fy-opt-in/fy-opt-in.component.html(1 hunks)src/app/shared/components/fy-opt-in/fy-opt-in.component.scss(1 hunks)src/app/shared/icon/icon.providers.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.component.{ts,html}
📄 CodeRabbit inference engine (.cursor/rules/component-i18n-key-naming.mdc)
**/*.component.{ts,html}: Top-level object in i18n translation files must be the component or feature folder name, derived from the file name by converting kebab-case to camelCase and removing the first matching prefix from: 'feature', 'ui', 'component'.
Keys inside the top-level object in i18n translation files must be free-form, not prefixed, and there must be no nested objects beyond the first level.
Use meaningful, semantic, and context-aware key names for i18n keys; avoid generic names like 'label1', 'text1', or 'message1'.
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 object per component or feature in i18n translation files; avoid deep nesting and avoid prefixes like 'ui*' or 'feature*' in object names.
Files:
src/app/shared/components/fy-opt-in/fy-opt-in.component.html
src/**/*.component.{ts,html}
📄 CodeRabbit inference engine (.cursor/rules/component-i18n-key-naming.mdc)
Translation keys for Angular components and templates located in 'src/**' must be placed in 'src/assets/i18n/{lang}.json' for every supported language.
Files:
src/app/shared/components/fy-opt-in/fy-opt-in.component.html
**/*.component.html
📄 CodeRabbit inference engine (.cursor/rules/component-i18n-key-naming.mdc)
Prefer property binding for user-facing attributes in Angular templates when using translation keys.
Files:
src/app/shared/components/fy-opt-in/fy-opt-in.component.html
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
src/app/shared/components/fy-opt-in/fy-opt-in.component.scss (1)
20-21: Logo dimensions updated perfectly, boss!Width expanded from 70px to 200px and height from 24px to 26px - style is mass! These new dimensions are now consistent with switch-org header (both 200x26), which is the right way to maintain uniformity across the app, thalaiva style!
src/app/auth/switch-org/switch-org.page.scss (1)
9-10: Header dimensions adjusted with perfect timing!Width increased from 84px to 200px and height adjusted from 30px to 26px. Now both switch-org and fy-opt-in components share the same logo dimensions (200x26) - that's how you maintain consistency, macha! Sage logo fitting like a boss!
src/app/shared/components/fy-opt-in/fy-opt-in.component.html (1)
9-9: Icon switched with style, macha!Changed from "fyle-logo-dark" to "sage-logo" using mat-icon with svgIcon - that's the Angular Material way! Clean implementation using the icon registry, mass!
| <ion-icon | ||
| class="switch-org__toolbar__header-container__header" | ||
| src="../../../assets/svg/fyle-logo-dark.svg" | ||
| src="../../../assets/svg/sage-logo.svg" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Logo asset path updated, but pattern is different, da!
You've switched from "../../../assets/svg/fyle-logo-dark.svg" to "../../../assets/svg/sage-logo.svg" using direct path with ion-icon. The change works, but notice that fy-opt-in.component.html uses the icon registry approach (mat-icon with svgIcon="sage-logo") while this file uses direct SVG path. Both are valid, but mixing patterns can make maintenance harder, boss!
Consider using the icon registry pattern here too for consistency:
<div class="switch-org__toolbar__header-container">
- <ion-icon
+ <mat-icon
class="switch-org__toolbar__header-container__header"
- src="../../../assets/svg/sage-logo.svg"
- ></ion-icon>
+ svgIcon="sage-logo"
+ ></mat-icon>
</div>But if ion-icon is required for Ionic framework compatibility, the current approach is fine, thalaiva!
📝 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.
| src="../../../assets/svg/sage-logo.svg" | |
| <div class="switch-org__toolbar__header-container"> | |
| <mat-icon | |
| class="switch-org__toolbar__header-container__header" | |
| svgIcon="sage-logo" | |
| ></mat-icon> | |
| </div> |
🤖 Prompt for AI Agents
In src/app/auth/switch-org/switch-org.page.html around line 11, the logo was
changed to a direct SVG path (src="../../../assets/svg/sage-logo.svg") which
mixes patterns with other components that use the registered SVG icon approach;
update this file to use the app's icon registry pattern (e.g., replace the
direct src usage with the component/style that uses the registered name such as
mat-icon svgIcon="sage-logo" or the equivalent registered icon usage in your
Ionic setup) so it uses the same registered SVG icon as
fy-opt-in.component.html; if ion-icon is strictly required for this page, leave
as-is but add a short inline comment explaining why this page differs from the
registry pattern.
|
| 'duplicate.svg', | ||
| 'edit.svg', | ||
| 'envelope.svg', | ||
| 'fyle-logo-dark.svg', |
🔁 Code Duplication Report - Angular
📊 No significant change in code duplication. |
2 participants
Clickup
https://app.clickup.com/t/86d0k46y9
Code Coverage
Please add code coverage here
UI Preview
Please add screenshots for UI changes
Summary by CodeRabbit