WEB-546 fix(images): correct mifos logo reference from png to jpg #2959
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Note
|
| Cohort / File(s) | Summary |
|---|---|
Environment Configurations src/environments/environment.prod.ts, src/environments/environment.ts |
Updated tenantLogoUrl property value from 'assets/images/mifos_lg-logo.png' to 'assets/images/mifos_lg-logo.jpg' |
Theme Styling src/theme/_dark_content.scss |
Updated CSS attribute selector for dark-theme home logo from [src*='mifos_lg-logo.png'] to [src*='mifos_lg-logo.jpg'] |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related PRs
- WEB-511 Update login page logo to use tenant-specific branding #2926: Introduced the
tenantLogoUrlenvironment property for tenant-specific logos, which is directly updated by this PR. - WEB-457 Tenant aware feature #2854: Also modifies references to the same logo asset name, transitioning from PNG to alternative formats.
Suggested reviewers
- IOhacker
Pre-merge checks and finishing touches
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately describes the main change: correcting mifos logo image references from PNG to JPG format across configuration and style files. |
✨ Finishing touches
- 📝 Generate docstrings
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 help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Pull request overview
This PR fixes incorrect logo image references in the Mifos application by changing the file extension from .png to .jpg to match the actual asset file that exists in the project. The changes ensure the Mifos large logo renders correctly across different parts of the application.
Key Changes:
- Updated logo file references from
mifos_lg-logo.pngtomifos_lg-logo.jpgin environment configurations - Updated CSS selector in dark theme to match the corrected
.jpgextension - Modified proxy configuration (appears unrelated to logo fix)
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/theme/_dark_content.scss | Updated attribute selector from .png to .jpg for dark mode logo replacement |
| src/environments/environment.ts | Corrected default tenant logo URL from .png to .jpg |
| src/environments/environment.prod.ts | Corrected default tenant logo URL from .png to .jpg in production environment |
| proxy.conf.js | Changed proxy target from Chuck Norris API to Mifos sandbox with additional logging (unrelated to PR purpose) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
proxy.conf.js
Outdated
| context: ['/fineract-provider'], | ||
| target: 'https://sandbox.mifos.community', | ||
| changeOrigin: true, | ||
| secure: false | ||
| secure: true, | ||
| logLevel: 'debug', | ||
| onProxyReq: function(proxyReq, req, res) { | ||
| console.log('[Proxy] Proxying:', req.method, req.url, '->', 'https://sandbox.mifos.community' + req.url); | ||
| } |
There was a problem hiding this comment.
The changes to proxy.conf.js appear to be unrelated to the PR's stated purpose of fixing logo image references. This PR changes the proxy configuration from a Chuck Norris API to the Mifos sandbox, adds debug logging, and changes security settings. These changes should either be removed from this PR and submitted separately with appropriate documentation, or the PR description should be updated to explain why these proxy configuration changes are being made.
There was a problem hiding this comment.
@priyans11 These changes in proxy.conf.js don’t seem related to the stated purpose of this PR (fixing logo image references). The proxy configuration update, debug logging, and security setting changes appear to be out of scope for this PR.
![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
🧹 Nitpick comments (1)
proxy.conf.js (1)
18-20: Avoid hardcoding the target URL in the logging function.The logging is helpful for debugging, but the target URL is hardcoded on Line 19 instead of referencing the
targetvariable defined on Line 14. This creates a maintenance risk if the target changes.🔎 Proposed refactor
- onProxyReq: function(proxyReq, req, res) { - console.log('[Proxy] Proxying:', req.method, req.url, '->', 'https://sandbox.mifos.community' + req.url); - } + onProxyReq: function(proxyReq, req, res, options) { + console.log('[Proxy] Proxying:', req.method, req.url, '->', options.target + req.url); + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
proxy.conf.jssrc/environments/environment.prod.tssrc/environments/environment.tssrc/theme/_dark_content.scss
⏰ 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: Upload results
🔇 Additional comments (3)
src/theme/_dark_content.scss (1)
274-274: LGTM!The CSS selector update is consistent with the environment configuration changes and correctly matches the new JPG asset.
src/environments/environment.prod.ts (1)
55-55: LGTM!The production environment logo reference is correctly updated to match the JPG asset, maintaining consistency with the development environment.
src/environments/environment.ts (1)
59-59: Asset path is correct. The JPG logo file exists atsrc/assets/images/mifos_lg-logo.jpg.
|
A Jira ticket Web-546 has been created for this please assign it to yourself before making any changes. Going forward, before creating any issue or PR, please ensure a Jira ticket is created first. |
|
@shubhamkumar9199 Ok understood. |
Correct incorrect references to `mifos_lg-logo.png`. The PNG asset does not exist; the correct file is `mifos_lg-logo.jpg`. Although the UI rendered correctly due to fallback logic, the incorrect reference was misleading and could cause issues in future refactors.
|
@shubhamkumar9199 Updated the PR |
|
LGTM. @IOhacker, could you please review? |
priyans11
changed the title
3 participants
Summary
This PR corrects incorrect references to
mifos_lg-logo.pngand replaces themwith the correct asset
mifos_lg-logo.jpg.The PNG file does not exist in the assets directory. Although the UI continued
to work due to an OR fallback (
tenantLogoUrl || assets/images/mifos_lg-logo.jpg),the incorrect reference was misleading and fragile.
Impact
Changes
.pngto.jpgin:environment.tsenvironment.prod.ts_dark_content.scssEvidence
Jira