WEB-511 Update login page logo to use tenant-specific branding#2926
WEB-511 Update login page logo to use tenant-specific branding#2926IOhacker merged 1 commit intoopenMF:devfrom
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Environment & Configuration env.sample, src/assets/env.js, src/assets/env.template.js, src/environments/environment.prod.ts, src/environments/environment.ts |
Added new environment variable tenantLogoUrl for configurable tenant logo URL, with fallback to default 'assets/images/mifos_lg-logo.png'. Defined across all configuration layers (sample, runtime, and TypeScript environments). |
Login Component Logic & Template src/app/login/login.component.ts, src/app/login/login.component.html |
Introduced logoPath property and updateLogo() method to dynamically resolve logo from environment or derive tenant-specific path. Added onLogoError() fallback handler. Updated template to bind image source dynamically with error handler. |
Tenant Selection Integration src/app/shared/tenant-selector/tenant-selector.component.ts |
Integrated AlertService to emit "Tenant Changed" alert when tenant selection is modified, triggering logo update in login component. |
Dark Theme Styling src/theme/_dark_content.scss |
Extended dark theme logo selector to target both legacy and new image sources ([src*='mifos_lg-logo.png'] and [src$='_home.png']). |
Sequence Diagram
sequenceDiagram
participant User
participant TenantSelector as TenantSelector<br/>Component
participant AlertService
participant LoginComponent
participant ImageElement as Image<br/>Element
User->>TenantSelector: Select tenant
TenantSelector->>AlertService: emit alert<br/>(type: "Tenant Changed")
AlertService->>LoginComponent: receive alert
LoginComponent->>LoginComponent: updateLogo()
Note over LoginComponent: Check env.tenantLogoUrl<br/>or derive from tenantId
LoginComponent->>ImageElement: [src]="logoPath"
ImageElement-->>LoginComponent: load image
alt Image loads successfully
ImageElement->>User: display logo
else Image fails to load
LoginComponent->>LoginComponent: onLogoError()
LoginComponent->>ImageElement: fallback to<br/>default_home.png
ImageElement->>User: display fallback
end
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- WEB-457 Tenant aware feature #2854: Implements complementary tenant-specific branding by updating Home/Footer components to use tenant-aware image paths aligned with this PR's tenant logo infrastructure.
Suggested reviewers
- IOhacker
Pre-merge checks and finishing touches
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately summarizes the main change: updating the login page logo to support tenant-specific branding, which is reflected across all modified files. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/environments/environment.ts (2)
59-59: Consider consistent property access notation.The
tenantLogoUrluses dot notation (loadedEnv.tenantLogoUrl) while the corresponding line inenvironment.prod.tsuses bracket notation (loadedEnv['tenantLogoUrl']). Both work correctly, but consistency across environment files improves maintainability.🔎 Suggested consistency improvement
Consider standardizing to bracket notation to match the majority of property accesses in
environment.prod.ts:- tenantLogoUrl: loadedEnv.tenantLogoUrl || 'assets/images/mifos_lg-logo.png', + tenantLogoUrl: loadedEnv['tenantLogoUrl'] || 'assets/images/mifos_lg-logo.png',
59-59: URL validation for external tenant logos should be more explicit.The environment supports external URLs for
tenantLogoUrl, but the login component only validates that the string is non-empty. While Angular's property binding[src]provides built-in XSS protection and img tags are inherently safe from content injection, consider adding explicit URL validation to ensure only valid HTTPS URLs are accepted. This prevents potential issues like malformed URLs or unencrypted resource loading. The existing error fallback to the default logo provides a good safety net if the URL fails to load.src/app/login/login.component.ts (2)
120-131: Consider encapsulation and minor optimization.The three-tier fallback logic (environment → tenant-specific → default) is well-structured and provides robust branding support.
Two optional refinements:
Method visibility: Since
updateLogo()is only called internally, consider making itprivateto improve encapsulation.Redundant trim check: Line 121 checks both truthiness and
trim() !== ''. Since you're already checkingenvironment.tenantLogoUrl &&, the trim check is redundant—a truthy non-empty string won't become empty after trimming whitespace.🔎 Proposed refinements
- updateLogo(): void { + private updateLogo(): void { if (environment.tenantLogoUrl && environment.tenantLogoUrl.trim() !== '') { - this.logoPath = environment.tenantLogoUrl; + this.logoPath = environment.tenantLogoUrl.trim(); return; }Or simply:
- if (environment.tenantLogoUrl && environment.tenantLogoUrl.trim() !== '') { - this.logoPath = environment.tenantLogoUrl; + if (environment.tenantLogoUrl) { + this.logoPath = environment.tenantLogoUrl.trim(); return; }
75-90: Consider RxJS operators for cleaner observable handling.The current subscription management with manual unsubscribe is correct and prevents memory leaks. However, you could improve readability and follow Angular best practices by using RxJS operators.
🔎 Alternative approach using takeUntil pattern
+import { Subject } from 'rxjs'; +import { takeUntil } from 'rxjs/operators'; export class LoginComponent implements OnInit, OnDestroy { - alert$: Subscription; + private destroy$ = new Subject<void>(); ngOnInit() { this.updateLogo(); - this.alert$ = this.alertService.alertEvent.subscribe((alertEvent: Alert) => { + this.alertService.alertEvent + .pipe(takeUntil(this.destroy$)) + .subscribe((alertEvent: Alert) => { const alertType = alertEvent.type; // ... alert handling logic }); } ngOnDestroy() { - this.alert$.unsubscribe(); + this.destroy$.next(); + this.destroy$.complete(); }This pattern scales better when managing multiple subscriptions and is more idiomatic in Angular applications.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
env.samplesrc/app/login/login.component.htmlsrc/app/login/login.component.tssrc/app/shared/tenant-selector/tenant-selector.component.tssrc/assets/env.jssrc/assets/env.template.jssrc/environments/environment.prod.tssrc/environments/environment.tssrc/theme/_dark_content.scss
🧰 Additional context used
📓 Path-based instructions (1)
src/app/**
⚙️ CodeRabbit configuration file
src/app/**: For Angular code: verify component separation, trackBy on *ngFor,
strict type safety, and clean observable patterns.
Files:
src/app/login/login.component.htmlsrc/app/login/login.component.tssrc/app/shared/tenant-selector/tenant-selector.component.ts
🧬 Code graph analysis (1)
src/app/login/login.component.ts (2)
src/environments/environment.prod.ts (1)
environment(7-81)src/environments/environment.ts (1)
environment(12-85)
🔇 Additional comments (6)
src/app/login/login.component.html (1)
53-53: Dynamic logo binding implemented correctly.The image binding uses proper Angular syntax with
[src]="logoPath"for dynamic logo resolution and(error)="onLogoError()"for graceful fallback handling. The alt text remains accessible.src/theme/_dark_content.scss (1)
274-277: Dark theme logo selector correctly extended.The attribute selector now covers both the main tenant logo (
mifos_lg-logo.png) and the fallback logo (images ending with_home.png), ensuring consistent white logo rendering in dark mode for all logo sources.src/app/shared/tenant-selector/tenant-selector.component.ts (1)
4-4: Alert integration looks correct.The AlertService is properly imported (line 4), injected (line 21), and invoked after the tenant identifier is set (line 45). The alert provides the tenant value as the message, which aligns with the login component's need to update the logo when tenants change. The login component properly subscribes to the "Tenant Changed" alert type (line 87 in login.component.ts) and the AlertService signature matches this usage pattern, with the Alert interface defining both type and message properties as expected.
src/app/login/login.component.ts (3)
68-68: Good foundation for tenant-specific branding!The
logoPathproperty is well-initialized with a sensible default. The overall approach of dynamic logo selection based on tenant configuration is sound and provides good flexibility for multi-tenant deployments.
74-74: LGTM! Proper initialization and reactive updates.The logo is correctly initialized on component load and updates dynamically when the tenant changes. This ensures the branding stays in sync with tenant selection throughout the user session.
Also applies to: 87-89
133-135: LGTM! Good defensive error handling.The fallback to the default image when logo loading fails provides a robust user experience and prevents broken image display.
Description
Describe the changes made and why they were made instead of how they were made. List any dependencies that are required for this change.
Related issues and discussion
#{Issue Number}
Screenshots, if any
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
If you have multiple commits please combine them into one commit by squashing them.
Read and understood the contribution guidelines at
web-app/.github/CONTRIBUTING.md.Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.