fe: landing page UI#3665
Closed
iSimran2023 wants to merge 2 commits into
Closed
Conversation
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Login form validation behavior src/app/login/login-form/login-form.component.ts, src/app/login/login-form/login-form.component.html |
login() calls markAllAsTouched() and returns early if form is invalid. Submit button disabled binding changed to only respond to loading state; form validity is now enforced in the component. |
Login component template & wiring src/app/login/login.component.html, src/app/login/login.component.ts |
Hero panel replaced with static "Welcome to CoIMS" content and feature list. Login card simplified to subtitle, tenant selector, and conditional login/reset/2FA slots. Component imports trimmed to login-specific standalones. Logo defaults and error handlers point to debug-bg.png. |
Login component & form SCSS redesign src/app/login/login.component.scss, src/app/login/login-form/login-form.component.scss |
New SCSS palette variables introduced. Hero panel styled with gradient overlay and background image. Extensive ::ng-deep Angular Material MDC overrides for inputs, labels, icons, errors, submit button, and progress bar. Dark mode and responsive breakpoints rebuilt for ≤1024px, ≤768px, and ≤480px. |
Environment defaults, shell loader & dark theme src/environments/environment.ts, src/environments/environment.prod.ts, src/environments/.env.ts, src/index.html, src/theme/_dark_content.scss |
API URLs default to https://103.175.192.233; logo defaults changed to debug-bg.png. Page title fixed to CoIMS, favicon updated. Lottie loader replaced with inline CSS spinner and progress bar. Dark theme logo URLs switched to debug-bg.png. |
APP_NAME & WebApp label across all locales src/assets/translations/*.json (14 files) |
APP_NAME changed to COIMS and new labels.version.WebApp entry added across all locale files (cs, de, en, es-CL, es-MX, fr, it, ko, lt, lv, ne, pt, sw). |
Client → Members Terminology Refactor
| Layer / File(s) | Summary |
|---|---|
App routing & breadcrumb updates src/app/app-routing.module.ts, src/app/clients/clients-routing.module.ts, src/app/keyboards-shortcut-config.ts |
Top-level route path changed from /clients to /members. Breadcrumbs and route data updated from "Create Client" / "Clients View" to "Create Member" / "Members View". Keyboard shortcut changed from Create Client to Create Member. |
Account & loan view navigation paths src/app/deposits/*/..., src/app/savings/..., src/app/loans/..., src/app/products/...*, src/app/shared/account-number/... |
Fixed/recurring deposit, savings, loan, and loan product view components updated to navigate using /members/{id}/... instead of /clients/{id}/... in reload/back-navigation flows. |
Task approval & checker tabs src/app/tasks/checker-inbox-and-tasks-tabs/* |
Client approval, council approval, loan approval, loan disbursal, and reschedule loan templates updated to route to /members/... instead of /clients/... for all member navigation links. |
Account-related component navigation src/app/account-transfers/..., src/app/groups/..., src/app/system/manage-jobs/..., src/app/clients/clients-view/... |
Account-transfer, group, cob-loan-locked, and clients-view components updated to generate and navigate using /members/{id}/... routes. |
Client stepper form layout & styling src/app/clients/client-stepper/client-general-step/client-general-step.component.html, src/app/clients/client-stepper/client-general-step/client-general-step.component.scss |
Name fields refactored from shared wrapper to individual conditional blocks. Responsive layout classes updated across all form fields. New flex utility classes (.flex-10, .flex-15, .flex-22) added for fixed-width column behavior. |
Client → Members terminology in translations src/assets/translations/en-US.json, src/assets/translations/de-DE.json, src/assets/translations/fr-FR.json |
Permissions labels, button text (Add/Transfer), input labels, headings, menu items, and help text updated from "Clients" to "Members". |
Shell Navigation, Dialog Removal & Footer Updates
| Layer / File(s) | Summary |
|---|---|
Warning dialog removal & service cleanup src/app/core/authentication/authentication.service.ts, src/app/home/home.component.ts, src/app/home/home.module.ts, src/app/home/warning-dialog/* |
AuthenticationService removes dialog state field and public dialog API methods. HomeComponent and HomeModule remove WarningDialogComponent usage. Warning-dialog component files deleted entirely. |
Shell sidenav & toolbar restructuring src/app/core/shell/sidenav/sidenav.component.html, src/app/core/shell/toolbar/toolbar.component.html |
Sidenav logo replaced with translated APP_NAME text. Toolbar Institution tab removes READ_CLIENT permission; new standalone Clients tab with dedicated READ_CLIENT check. Toolbar menu simplified to only Groups and Centers; removed language/theme selectors and accounting menu options. |
Footer styling & dynamic year src/app/shared/footer/footer.component.html, src/app/shared/footer/footer.component.scss, src/app/shared/footer/footer.component.ts |
Footer template replaces backend info table with simplified shell showing copyright (using runtime currentYear) and translated APP_NAME; conditionally displays business date. Footer SCSS rewritten to flex-column centered layout. New currentYear property added to component. |
Page title hardcoded to CoIMS src/app/web-app.component.ts |
WebAppComponent title-setting logic simplified: removed route-data translation lookup, now directly sets title to fixed string COIMS. |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Suggested reviewers
- IOhacker
- adamsaghy
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check | The PR title 'fe: landing page UI' is too vague and doesn't accurately represent the scope of changes. The changeset includes extensive rebranding (Mifos to CoIMS), terminology changes (Clients to Members), component restructuring, routing updates, and environment configuration modifications across the entire application—far beyond just landing page UI updates. | Revise the title to accurately reflect the comprehensive nature of changes, such as 'fe: rebrand to CoIMS and rename clients to members' or 'fe: comprehensive UI and branding overhaul including clients-to-members terminology update'. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/login/login-form/login-form.component.scss (1)
26-73:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftReplace deprecated
::ng-deepwith alternative styling approaches.Multiple instances of
::ng-deepare used throughout the file. This pseudo-element is deprecated by Angular and will be removed. As per retrieved learnings, prefer styling MDC DOM classes directly in Angular Material v20+.♻️ Alternative approaches
Preferred: Use
:host ::ng-deepwrapper for better scoping::host ::ng-deep .tenant-section { mifosx-tenant-selector { // styles } }Alternative: Move Material overrides to a global theme configuration file and use Angular Material's theming API for customization instead of direct MDC class overrides.
Also applies to: 80-159, 167-198
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/login/login-form/login-form.component.scss` around lines 26 - 73, The deprecated ::ng-deep pseudo-element is being used for styling the tenant-section and other Material components throughout the file, which will be removed in future Angular versions. Replace all instances of ::ng-deep with :host ::ng-deep wrapper for better scoping. Specifically, change ::ng-deep .tenant-section at the beginning of the styles to :host ::ng-deep .tenant-section, and apply the same pattern to all other occurrences of ::ng-deep in the file (including those affecting lines 80-159 and 167-198 as mentioned in the review). This maintains the scoping while using the modern approach that Angular Material v20+ recommends.Source: Learnings
🧹 Nitpick comments (5)
src/index.html (1)
118-212: 🏗️ Heavy liftMove loader styles out of
index.htmlinto SCSS theme filesLine 118 introduces inline styles with hardcoded values. This bypasses your SCSS/theming conventions and the shared variable system expected for
src/**/*.{scss,html}.As per coding guidelines, use SCSS (not inline CSS) and leverage shared SCSS variables with 8px grid alignment for `src/**/*.{scss,html}`.Refactor direction
- <style> - .loader-wrapper { ... } - ... - </style> + <!-- keep only loader markup here -->Then define these classes in a shared SCSS file (for example under
src/theme/or global styles), replacing hardcoded color/size tokens with existing SCSS variables fromsrc/main.scss/src/theme/mifosx-theme.scssand snapping spacing/sizing to the 8px grid.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/index.html` around lines 118 - 212, The loader styles with hardcoded color and size values in the inline style block are bypassing your SCSS theming system. Move all the CSS rules for classes like loader-wrapper, loader-spinner, spinner-ring, spinner-ring-2, spinner-dot, loader-text, loader-subtext, loader-bar, loader-bar-fill and their keyframes to a shared SCSS file in src/theme/ (or appropriate global styles location). Replace all hardcoded color values like `#1074b9`, `#0a5a8f`, `#e2e8f0`, and `#94a3b8` with corresponding SCSS variables from src/main.scss or src/theme/mifosx-theme.scss. Also ensure all hardcoded dimensions (72px, 10px, 160px, gaps, margins) follow the 8px grid alignment convention. Remove the inline style block from index.html and reference the compiled styles from the new SCSS file.Sources: Coding guidelines, Linters/SAST tools
src/app/login/login.component.html (1)
20-46: ⚡ Quick winUse Angular Material icons instead of HTML entities.
The checkmark uses HTML entity
✓instead of Angular Material icons. As per coding guidelines, all visual components must strictly use Angular Material elements where possible.✨ Proposed fix to use Material icons
<div class="feature-item"> <div class="feature-check"> - <span class="check-mark">&`#10003`;</span> + <mat-icon class="check-mark">check</mat-icon> </div> <div class="feature-text"> <strong>{{ 'labels.login.features.savings.title' | translate }}</strong> <span>{{ 'labels.login.features.savings.description' | translate }}</span> </div> </div>Apply the same change to all three feature items. Update SCSS accordingly:
.check-mark { font-size: 0.8rem; font-weight: 700; color: `#4ade80`; line-height: 1; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/login/login.component.html` around lines 20 - 46, Replace the HTML entity checkmark (&`#10003`;) with Angular Material icon components in all three feature items. Locate the span elements with class "check-mark" that currently contain the HTML entity and replace them with the mat-icon component using the "check_circle" or "check" icon. Ensure the mat-icon is properly imported in the component and update the SCSS styling for the "check-mark" class as needed to accommodate the Material icon styling.Source: Coding guidelines
src/app/login/login-form/login-form.component.scss (2)
278-356: 🏗️ Heavy liftUse theme variables for dark mode colors instead of hardcoded values.
Dark theme colors (
#2a2a2a,#e2e2e5,#c4c6d0, etc.) are hardcoded. These should come from the centralized theme system to maintain consistency and enable theme customization.🎨 Recommended approach
Define dark theme colors in
src/theme/mifosx-theme.scss:// Dark theme palette $dark-bg-primary: `#2a2a2a`; $dark-text-primary: `#e2e2e5`; $dark-text-muted: `#c4c6d0`; $dark-border: `#444`;Then use them:
:host-context(.dark-theme) { .login-field { ::ng-deep { .mat-mdc-text-field-wrapper { background: $dark-bg-primary !important; border-color: $dark-border !important; } // ... } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/login/login-form/login-form.component.scss` around lines 278 - 356, The dark theme colors throughout the :host-context(.dark-theme) block (such as `#2a2a2a`, `#e2e2e5`, `#c4c6d0`, `#444`, `#888`, `#003a6b`, `#5ba2ec`, `#d1e4ff`, `#2c2f36`, `#44474e`) are hardcoded instead of using centralized theme variables. Create SCSS variables for all dark theme colors in the centralized theme file and then replace each hardcoded color value in the dark theme context with its corresponding variable. This includes colors applied to .mat-mdc-text-field-wrapper, .mat-mdc-select, .mat-mdc-floating-label, .mat-mdc-input-element, .welcome-message, .protected-data, and related Material selectors.Source: Coding guidelines
17-405: ⚖️ Poor tradeoffAlign spacing values to the 8px grid system.
As per coding guidelines, stick to the 8px grid system for visual design and spacing. Many values use arbitrary rem units that don't align to 8px multiples.
📐 Examples of misaligned values
Assuming 1rem = 16px:
0.75rem(12px) → use0.5rem(8px) or1rem(16px)0.875rem(14px) → use1rem(16px)0.35rem(5.6px) → use0.5rem(8px)0.65rem(10.4px) → use0.5rem(8px) or1rem(16px)Review all spacing/sizing values throughout the file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/login/login-form/login-form.component.scss` around lines 17 - 405, Review and replace all spacing values throughout the login-form.component.scss file to align with the 8px grid system. Specifically, replace 0.75rem values with 0.5rem or 1rem, replace 0.875rem values with 1rem, replace 0.35rem values with 0.5rem, and replace 0.65rem values with 0.5rem or 1rem depending on context. Check all gap, padding, margin, and similar sizing properties in classes like `#login-form`, .login-field, .login-submit-button, .oidc-login-content, and the responsive media queries to ensure all values conform to 8px multiples (0.5rem = 8px, 1rem = 16px, 1.5rem = 24px, 2rem = 32px, etc).Source: Coding guidelines
src/app/login/login.component.scss (1)
17-441: ⚖️ Poor tradeoffAlign spacing and sizing to the 8px grid system.
As per coding guidelines: "Stick to the 8px grid system for visual design and spacing." Many values use arbitrary rem units (e.g.,
0.875rem,1.25rem,2.5rem) that don't align to 8px multiples (0.5rem, 1rem, 1.5rem, 2rem).📐 8px grid conversion examples
Assuming 1rem = 16px, align to 8px grid:
0.875rem(14px) →1rem(16px)1.25rem(20px) →1.5rem(24px)2.5rem(40px) →2.5rem(40px) ✓ already aligned0.75rem(12px) →0.5rem(8px) or1rem(16px)3.5rem(56px) →3.5rem(56px) ✓ already alignedReview all spacing values and round to nearest 8px multiple.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/login/login.component.scss` around lines 17 - 441, Review all spacing and sizing values throughout the login component SCSS file and convert arbitrary rem units to align with the 8px grid system (multiples of 0.5rem). For example: change 0.875rem to 1rem, 1.25rem to 1.5rem, 0.75rem to either 0.5rem or 1rem depending on context, 0.825rem to 1rem, 0.95rem to 1rem, 0.8rem to 1rem, and 0.9rem to 1rem. Focus on spacing properties in classes like .hero-title, .hero-subtitle, .feature-check, .feature-text, .login-card, .card-subtitle, .version-label, and .version-value, as well as padding, margin, gap, and font-size values throughout the entire stylesheet to ensure all dimensions are consistent with the 8px grid standard.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/login/login-form/login-form.component.scss`:
- Around line 10-14: Remove the duplicate color variable definitions
($brand-blue, $text-dark, $text-muted, $input-bg, $input-border) from
login-form.component.scss and consolidate them into the centralized theme file.
Ensure these variables are defined once in the theme file (verifying they exist
or adding them if necessary), then import the theme file at the top of both
login-form.component.scss and login.component.scss using `@import` statement to
access these shared variables. This eliminates duplication and ensures
consistent styling across all components using these colors.
In `@src/app/login/login.component.html`:
- Line 68: The subtitle text in the p element with class card-subtitle contains
hardcoded English text instead of using an internationalization translation key.
Replace the hardcoded text "Enter your credentials to access your account" with
an i18n translation key by adding an i18n attribute to the p element (e.g.,
i18n="@@loginSubtitleKey") and move the text content to your translation files
for proper i18n support.
- Around line 13-16: The h1 element with the hero-title class contains the
hardcoded string "Welcome to CoIMS" and the p element with the hero-subtitle
class contains hardcoded text. Replace these hardcoded strings with translation
pipe syntax using `@ngx-translate/core` keys structured as labels.login.hero.title
and labels.login.hero.subtitle. Wrap each string with the translate pipe (use
the pipe syntax) in the template. Additionally, add the corresponding
translation key-value pairs to all locale files in src/assets/translations/
directory (e.g., en-US.json) with the proper nested structure under
labels.login.hero. After making these changes, run npm run translations:extract
to ensure the i18n variables are properly extracted per coding guidelines.
- Around line 93-96: In the login-footer paragraph element of the login
component template, replace all hardcoded text strings with i18n translation
keys. Specifically, replace the hardcoded copyright year and company name text
"2026 CoIMS. All rights reserved. Developed by" with an appropriate i18n key
using the i18n attribute or interpolation syntax, and replace the link text
"Debugsoft" with a separate i18n translation key. This ensures all user-facing
text in the footer is translatable and supports multiple languages through the
application's internationalization system.
- Around line 20-46: Replace the hardcoded feature text in all three
feature-item divs (Savings & Deposits, Loan Management, and Share Capital) with
i18n translation keys. Update each strong tag and span containing the feature
titles and descriptions to use the appropriate i18n syntax or translation pipe.
Add the corresponding translation key-value pairs to all locale translation
files and run npm run translations:extract to complete the i18n integration.
In `@src/app/login/login.component.scss`:
- Around line 10-14: Move the color variable definitions ($brand-blue,
$brand-blue-dark, $text-dark, $text-muted, $input-bg) from login.component.scss
to the centralized src/theme/mifosx-theme.scss file. Then import the theme file
at the top of login.component.scss using `@import` statement and remove the local
variable definitions from login.component.scss, allowing the component to
reference the centralized theme variables instead. This ensures consistency
across the application by leveraging the shared theme configuration.
- Around line 189-283: Replace the deprecated ::ng-deep selector in the
login.component.scss file with the preferred approach of using :host ::ng-deep
for proper view encapsulation. Wrap the entire .login-card block and all its
nested selectors (.mat-mdc-text-field-wrapper, .mat-focused, .mdc-line-ripple,
.mat-mdc-floating-label, .mat-mdc-input-element,
.mat-mdc-form-field-icon-prefix, .mat-mdc-form-field-icon-suffix,
.mat-mdc-form-field-error, .mat-mdc-select, .login-submit-button,
.login-progress) with :host ::ng-deep instead of just ::ng-deep to maintain
proper Angular encapsulation while achieving the same styling goals.
- Line 344: The `word-break: break-word` property value is deprecated in CSS.
Replace this deprecated value by removing the `word-break: break-word` line and
adding `overflow-wrap: break-word` in the login.component.scss file at line 344.
The overflow-wrap property with the break-word value provides the same behavior
while following modern CSS standards.
In `@src/assets/translations/es-CL.json`:
- Line 435: The "WebApp" entry in the labels.version section of the es-CL.json
translation file is untranslated (identical key and value), which breaks
consistency with surrounding entries like "Mifos WebApp" that are properly
translated to Spanish or branded equivalents. Replace the untranslated "WebApp":
"WebApp" with a Spanish translation such as "Aplicación Web" or ensure it
matches an intentional branding pattern used consistently. If the entry is meant
to remain untranslated as a brand identifier, verify this same pattern is
applied uniformly across all 14 locale translation files for consistency.
In `@src/assets/translations/lt-LT.json`:
- Around line 434-435: The translation key mismatch between the lt-LT.json file
and the system-information.component.html file needs to be resolved. The
component references labels.version.Mifos WebApp (at line 14 of
system-information.component.html) but the translation file is adding a new
WebApp key while leaving Mifos WebApp unchanged. Choose one consistent approach:
either update the component to reference the new WebApp key instead of Mifos
WebApp, or keep using Mifos WebApp as the key in the translation file and ensure
its value is set to the new rebranded value (removing the duplicate WebApp entry
if it was only added for this migration). Ensure the key name used in the
component matches exactly with the key name in the translation file.
In `@src/environments/environment.prod.ts`:
- Around line 28-32: The production environment configuration file contains
hardcoded IP addresses (https://103.175.192.233) as fallback values for the
baseApiUrls and baseApiUrl properties, which violates the principle of keeping
deployment targets environment-driven. Replace these hardcoded fallback values
in both locations with a deployment-local alternative such as
window.location.origin, or remove the fallback entirely to ensure that
environment variables must be properly configured at deploy time, preventing API
traffic from being inadvertently pinned to a fixed infrastructure endpoint.
In `@src/environments/environment.ts`:
- Around line 31-35: The baseApiUrls property on line 31 and the baseApiUrl
property on line 35 both contain hardcoded external backend URLs (specifically
'https://103.175.192.233') as fallback values when environment variables are not
provided. This violates coding guidelines and can misroute traffic to incorrect
backends. Replace these hardcoded deployment targets with environment-driven
configuration or use a more appropriate fallback such as the current
window.location.origin approach that is already being used as a secondary
fallback. Ensure that loadedEnv variables are properly injected and that no
concrete external hosts are hardcoded in the environment configuration file.
---
Outside diff comments:
In `@src/app/login/login-form/login-form.component.scss`:
- Around line 26-73: The deprecated ::ng-deep pseudo-element is being used for
styling the tenant-section and other Material components throughout the file,
which will be removed in future Angular versions. Replace all instances of
::ng-deep with :host ::ng-deep wrapper for better scoping. Specifically, change
::ng-deep .tenant-section at the beginning of the styles to :host ::ng-deep
.tenant-section, and apply the same pattern to all other occurrences of
::ng-deep in the file (including those affecting lines 80-159 and 167-198 as
mentioned in the review). This maintains the scoping while using the modern
approach that Angular Material v20+ recommends.
---
Nitpick comments:
In `@src/app/login/login-form/login-form.component.scss`:
- Around line 278-356: The dark theme colors throughout the
:host-context(.dark-theme) block (such as `#2a2a2a`, `#e2e2e5`, `#c4c6d0`, `#444`, `#888`,
`#003a6b`, `#5ba2ec`, `#d1e4ff`, `#2c2f36`, `#44474e`) are hardcoded instead of using
centralized theme variables. Create SCSS variables for all dark theme colors in
the centralized theme file and then replace each hardcoded color value in the
dark theme context with its corresponding variable. This includes colors applied
to .mat-mdc-text-field-wrapper, .mat-mdc-select, .mat-mdc-floating-label,
.mat-mdc-input-element, .welcome-message, .protected-data, and related Material
selectors.
- Around line 17-405: Review and replace all spacing values throughout the
login-form.component.scss file to align with the 8px grid system. Specifically,
replace 0.75rem values with 0.5rem or 1rem, replace 0.875rem values with 1rem,
replace 0.35rem values with 0.5rem, and replace 0.65rem values with 0.5rem or
1rem depending on context. Check all gap, padding, margin, and similar sizing
properties in classes like `#login-form`, .login-field, .login-submit-button,
.oidc-login-content, and the responsive media queries to ensure all values
conform to 8px multiples (0.5rem = 8px, 1rem = 16px, 1.5rem = 24px, 2rem = 32px,
etc).
In `@src/app/login/login.component.html`:
- Around line 20-46: Replace the HTML entity checkmark (&`#10003`;) with Angular
Material icon components in all three feature items. Locate the span elements
with class "check-mark" that currently contain the HTML entity and replace them
with the mat-icon component using the "check_circle" or "check" icon. Ensure the
mat-icon is properly imported in the component and update the SCSS styling for
the "check-mark" class as needed to accommodate the Material icon styling.
In `@src/app/login/login.component.scss`:
- Around line 17-441: Review all spacing and sizing values throughout the login
component SCSS file and convert arbitrary rem units to align with the 8px grid
system (multiples of 0.5rem). For example: change 0.875rem to 1rem, 1.25rem to
1.5rem, 0.75rem to either 0.5rem or 1rem depending on context, 0.825rem to 1rem,
0.95rem to 1rem, 0.8rem to 1rem, and 0.9rem to 1rem. Focus on spacing properties
in classes like .hero-title, .hero-subtitle, .feature-check, .feature-text,
.login-card, .card-subtitle, .version-label, and .version-value, as well as
padding, margin, gap, and font-size values throughout the entire stylesheet to
ensure all dimensions are consistent with the 8px grid standard.
In `@src/index.html`:
- Around line 118-212: The loader styles with hardcoded color and size values in
the inline style block are bypassing your SCSS theming system. Move all the CSS
rules for classes like loader-wrapper, loader-spinner, spinner-ring,
spinner-ring-2, spinner-dot, loader-text, loader-subtext, loader-bar,
loader-bar-fill and their keyframes to a shared SCSS file in src/theme/ (or
appropriate global styles location). Replace all hardcoded color values like
`#1074b9`, `#0a5a8f`, `#e2e8f0`, and `#94a3b8` with corresponding SCSS variables from
src/main.scss or src/theme/mifosx-theme.scss. Also ensure all hardcoded
dimensions (72px, 10px, 160px, gaps, margins) follow the 8px grid alignment
convention. Remove the inline style block from index.html and reference the
compiled styles from the new SCSS file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b8034b0b-7c3e-4c4f-b5e1-ea7414457974
⛔ Files ignored due to path filters (11)
src/apple-touch-icon.pngis excluded by!**/*.pngand included by**/*src/assets/images/MifosX_logo.pngis excluded by!**/*.pngand included by**/*src/assets/images/MifosX_logoSmall.pngis excluded by!**/*.pngand included by**/*src/assets/images/debug-bg.pngis excluded by!**/*.pngand included by**/*src/assets/images/debug.pngis excluded by!**/*.pngand included by**/*src/assets/images/default_home.pngis excluded by!**/*.pngand included by**/*src/assets/images/hero.jpgis excluded by!**/*.jpgand included by**/*src/assets/images/mifos-logo-flat.pngis excluded by!**/*.pngand included by**/*src/assets/images/mifos_lg-logo.jpgis excluded by!**/*.jpgand included by**/*src/assets/images/white-mifos.pngis excluded by!**/*.pngand included by**/*src/favicon.icois excluded by!**/*.icoand included by**/*
📒 Files selected for processing (25)
src/app/login/login-form/login-form.component.htmlsrc/app/login/login-form/login-form.component.scsssrc/app/login/login-form/login-form.component.tssrc/app/login/login.component.htmlsrc/app/login/login.component.scsssrc/app/login/login.component.tssrc/assets/images/cover_image_resized.webpsrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.jsonsrc/environments/.env.tssrc/environments/environment.prod.tssrc/environments/environment.tssrc/index.htmlsrc/theme/_dark_content.scss
Comment on lines
+10
to
+14
| $brand-blue: #1074b9; | ||
| $text-dark: #1e293b; | ||
| $text-muted: #64748b; | ||
| $input-bg: #f1f5f9; | ||
| $input-border: #e2e8f0; |
There was a problem hiding this comment.
Consolidate color variables with login.component.scss and use theme files.
These color variables are duplicated from login.component.scss. As per coding guidelines, leverage SCSS variables from src/main.scss and src/theme/mifosx-theme.scss rather than defining them in multiple component files.
📝 Recommended approach
- Define colors once in
src/theme/mifosx-theme.scss - Import the theme file in both component SCSS files:
`@import` 'src/theme/mifosx-theme.scss';
// Then use the imported variables
.login-field {
::ng-deep .mat-mdc-text-field-wrapper {
background: $input-bg !important;
// ...
}
}This eliminates duplication and ensures consistency across components.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/login/login-form/login-form.component.scss` around lines 10 - 14,
Remove the duplicate color variable definitions ($brand-blue, $text-dark,
$text-muted, $input-bg, $input-border) from login-form.component.scss and
consolidate them into the centralized theme file. Ensure these variables are
defined once in the theme file (verifying they exist or adding them if
necessary), then import the theme file at the top of both
login-form.component.scss and login.component.scss using `@import` statement to
access these shared variables. This eliminates duplication and ensures
consistent styling across all components using these colors.
Source: Coding guidelines
Comment on lines
+13
to
+16
| <h1 class="hero-title">Welcome to CoIMS</h1> | ||
| <p class="hero-subtitle"> | ||
| Your trusted cooperative banking platform for savings, loans, share management, and seamless financial services. | ||
| </p> |
There was a problem hiding this comment.
Replace hardcoded strings with i18n translation keys.
The hero title and subtitle are hardcoded in English. As per coding guidelines, all user-facing strings must use @ngx-translate/core translation variables instead of hardcoded text.
🌐 Proposed fix to add i18n support
- <h1 class="hero-title">Welcome to CoIMS</h1>
- <p class="hero-subtitle">
- Your trusted cooperative banking platform for savings, loans, share management, and seamless financial services.
- </p>
+ <h1 class="hero-title">{{ 'labels.login.hero.title' | translate }}</h1>
+ <p class="hero-subtitle">
+ {{ 'labels.login.hero.subtitle' | translate }}
+ </p>Then add the translation keys to all locale files (e.g., src/assets/translations/en-US.json):
"labels": {
"login": {
"hero": {
"title": "Welcome to CoIMS",
"subtitle": "Your trusted cooperative banking platform for savings, loans, share management, and seamless financial services."
}
}
}After adding the strings, run npm run translations:extract to extract i18n variables as per coding guidelines.
📝 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.
Suggested change
| <h1 class="hero-title">Welcome to CoIMS</h1> | |
| <p class="hero-subtitle"> | |
| Your trusted cooperative banking platform for savings, loans, share management, and seamless financial services. | |
| </p> | |
| <h1 class="hero-title">{{ 'labels.login.hero.title' | translate }}</h1> | |
| <p class="hero-subtitle"> | |
| {{ 'labels.login.hero.subtitle' | translate }} | |
| </p> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/login/login.component.html` around lines 13 - 16, The h1 element with
the hero-title class contains the hardcoded string "Welcome to CoIMS" and the p
element with the hero-subtitle class contains hardcoded text. Replace these
hardcoded strings with translation pipe syntax using `@ngx-translate/core` keys
structured as labels.login.hero.title and labels.login.hero.subtitle. Wrap each
string with the translate pipe (use the pipe syntax) in the template.
Additionally, add the corresponding translation key-value pairs to all locale
files in src/assets/translations/ directory (e.g., en-US.json) with the proper
nested structure under labels.login.hero. After making these changes, run npm
run translations:extract to ensure the i18n variables are properly extracted per
coding guidelines.
Source: Coding guidelines
Comment on lines
+20
to
46
| <div class="feature-item"> | ||
| <div class="feature-check"> | ||
| <span class="check-mark">✓</span> | ||
| </div> | ||
| <div class="feature-text"> | ||
| <strong>Savings & Deposits</strong> | ||
| <span>Manage member savings, fixed deposits, and recurring deposits</span> | ||
| </div> | ||
| </div> | ||
| <div class="feature-item"> | ||
| <div class="feature-check"> | ||
| <span class="check-mark">✓</span> | ||
| </div> | ||
| <div class="hero-links"> | ||
| <a [href]="'mifosHome' | documentationLink" target="_blank" rel="noopener noreferrer" class="hero-link"> | ||
| {{ 'labels.text.Mifos Initiative' | translate }} | ||
| <fa-icon icon="external-link-alt" size="sm"></fa-icon> | ||
| </a> | ||
| <a | ||
| [href]="'mifosVolunteer' | documentationLink" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| class="hero-link" | ||
| > | ||
| {{ 'labels.text.Get involved' | translate }} | ||
| <fa-icon icon="external-link-alt" size="sm"></fa-icon> | ||
| </a> | ||
| <div class="feature-text"> | ||
| <strong>Loan Management</strong> | ||
| <span>Process loan applications, disbursements, and repayments</span> | ||
| </div> | ||
| </div> | ||
| } | ||
| <!-- Production Mode: Minimal branding at bottom --> | ||
| @if (productionMode) { | ||
| <div class="hero-branding-bottom"> | ||
| <h1 class="hero-title-minimal">{{ 'APP_NAME' | translate }}</h1> | ||
| <div class="feature-item"> | ||
| <div class="feature-check"> | ||
| <span class="check-mark">✓</span> | ||
| </div> | ||
| <div class="feature-text"> | ||
| <strong>Share Capital</strong> | ||
| <span>Track member shares, dividends, and equity contributions</span> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Replace hardcoded feature text with i18n translation keys.
All three feature items contain hardcoded English text. As per coding guidelines, use proper i18n variables for all user-facing strings.
🌐 Proposed fix to add i18n support
<div class="feature-item">
<div class="feature-check">
<span class="check-mark">&`#10003`;</span>
</div>
<div class="feature-text">
- <strong>Savings & Deposits</strong>
- <span>Manage member savings, fixed deposits, and recurring deposits</span>
+ <strong>{{ 'labels.login.features.savings.title' | translate }}</strong>
+ <span>{{ 'labels.login.features.savings.description' | translate }}</span>
</div>
</div>
<div class="feature-item">
<div class="feature-check">
<span class="check-mark">&`#10003`;</span>
</div>
<div class="feature-text">
- <strong>Loan Management</strong>
- <span>Process loan applications, disbursements, and repayments</span>
+ <strong>{{ 'labels.login.features.loans.title' | translate }}</strong>
+ <span>{{ 'labels.login.features.loans.description' | translate }}</span>
</div>
</div>
<div class="feature-item">
<div class="feature-check">
<span class="check-mark">&`#10003`;</span>
</div>
<div class="feature-text">
- <strong>Share Capital</strong>
- <span>Track member shares, dividends, and equity contributions</span>
+ <strong>{{ 'labels.login.features.shares.title' | translate }}</strong>
+ <span>{{ 'labels.login.features.shares.description' | translate }}</span>
</div>
</div>Add the corresponding keys to all locale translation files and run npm run translations:extract.
📝 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.
Suggested change
| <div class="feature-item"> | |
| <div class="feature-check"> | |
| <span class="check-mark">✓</span> | |
| </div> | |
| <div class="feature-text"> | |
| <strong>Savings & Deposits</strong> | |
| <span>Manage member savings, fixed deposits, and recurring deposits</span> | |
| </div> | |
| </div> | |
| <div class="feature-item"> | |
| <div class="feature-check"> | |
| <span class="check-mark">✓</span> | |
| </div> | |
| <div class="hero-links"> | |
| <a [href]="'mifosHome' | documentationLink" target="_blank" rel="noopener noreferrer" class="hero-link"> | |
| {{ 'labels.text.Mifos Initiative' | translate }} | |
| <fa-icon icon="external-link-alt" size="sm"></fa-icon> | |
| </a> | |
| <a | |
| [href]="'mifosVolunteer' | documentationLink" | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| class="hero-link" | |
| > | |
| {{ 'labels.text.Get involved' | translate }} | |
| <fa-icon icon="external-link-alt" size="sm"></fa-icon> | |
| </a> | |
| <div class="feature-text"> | |
| <strong>Loan Management</strong> | |
| <span>Process loan applications, disbursements, and repayments</span> | |
| </div> | |
| </div> | |
| } | |
| <!-- Production Mode: Minimal branding at bottom --> | |
| @if (productionMode) { | |
| <div class="hero-branding-bottom"> | |
| <h1 class="hero-title-minimal">{{ 'APP_NAME' | translate }}</h1> | |
| <div class="feature-item"> | |
| <div class="feature-check"> | |
| <span class="check-mark">✓</span> | |
| </div> | |
| <div class="feature-text"> | |
| <strong>Share Capital</strong> | |
| <span>Track member shares, dividends, and equity contributions</span> | |
| </div> | |
| </div> | |
| <div class="feature-item"> | |
| <div class="feature-check"> | |
| <span class="check-mark">&`#10003`;</span> | |
| </div> | |
| <div class="feature-text"> | |
| <strong>{{ 'labels.login.features.savings.title' | translate }}</strong> | |
| <span>{{ 'labels.login.features.savings.description' | translate }}</span> | |
| </div> | |
| </div> | |
| <div class="feature-item"> | |
| <div class="feature-check"> | |
| <span class="check-mark">&`#10003`;</span> | |
| </div> | |
| <div class="feature-text"> | |
| <strong>{{ 'labels.login.features.loans.title' | translate }}</strong> | |
| <span>{{ 'labels.login.features.loans.description' | translate }}</span> | |
| </div> | |
| </div> | |
| <div class="feature-item"> | |
| <div class="feature-check"> | |
| <span class="check-mark">&`#10003`;</span> | |
| </div> | |
| <div class="feature-text"> | |
| <strong>{{ 'labels.login.features.shares.title' | translate }}</strong> | |
| <span>{{ 'labels.login.features.shares.description' | translate }}</span> | |
| </div> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/login/login.component.html` around lines 20 - 46, Replace the
hardcoded feature text in all three feature-item divs (Savings & Deposits, Loan
Management, and Share Capital) with i18n translation keys. Update each strong
tag and span containing the feature titles and descriptions to use the
appropriate i18n syntax or translation pipe. Add the corresponding translation
key-value pairs to all locale translation files and run npm run
translations:extract to complete the i18n integration.
Source: Coding guidelines
| /> | ||
| } | ||
| </div> | ||
| <p class="card-subtitle">Enter your credentials to access your account</p> |
There was a problem hiding this comment.
Replace hardcoded subtitle with i18n translation key.
🌐 Proposed fix
- <p class="card-subtitle">Enter your credentials to access your account</p>
+ <p class="card-subtitle">{{ 'labels.login.card.subtitle' | translate }}</p>📝 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.
Suggested change
| <p class="card-subtitle">Enter your credentials to access your account</p> | |
| <p class="card-subtitle">{{ 'labels.login.card.subtitle' | translate }}</p> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/login/login.component.html` at line 68, The subtitle text in the p
element with class card-subtitle contains hardcoded English text instead of
using an internationalization translation key. Replace the hardcoded text "Enter
your credentials to access your account" with an i18n translation key by adding
an i18n attribute to the p element (e.g., i18n="@@loginSubtitleKey") and move
the text content to your translation files for proper i18n support.
Source: Coding guidelines
Comment on lines
+93
to
+96
| <p class="login-footer"> | ||
| © 2026 CoIMS. All rights reserved. Developed by | ||
| <a href="https://dibugsoft.com/" target="_blank" rel="noopener noreferrer">Debugsoft</a> | ||
| </p> |
There was a problem hiding this comment.
Replace hardcoded footer text with i18n translation keys.
The footer contains hardcoded copyright text and developer attribution. Use translation variables for internationalization support.
🌐 Proposed fix
<p class="login-footer">
- © 2026 CoIMS. All rights reserved. Developed by
- <a href="https://dibugsoft.com/" target="_blank" rel="noopener noreferrer">Debugsoft</a>
+ {{ 'labels.login.footer.copyright' | translate: { year: 2026 } }}
+ {{ 'labels.login.footer.developedBy' | translate }}
+ <a href="https://dibugsoft.com/" target="_blank" rel="noopener noreferrer">{{ 'labels.login.footer.developer' | translate }}</a>
</p>📝 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.
Suggested change
| <p class="login-footer"> | |
| © 2026 CoIMS. All rights reserved. Developed by | |
| <a href="https://dibugsoft.com/" target="_blank" rel="noopener noreferrer">Debugsoft</a> | |
| </p> | |
| <p class="login-footer"> | |
| {{ 'labels.login.footer.copyright' | translate: { year: 2026 } }} | |
| {{ 'labels.login.footer.developedBy' | translate }} | |
| <a href="https://dibugsoft.com/" target="_blank" rel="noopener noreferrer">{{ 'labels.login.footer.developer' | translate }}</a> | |
| </p> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/login/login.component.html` around lines 93 - 96, In the login-footer
paragraph element of the login component template, replace all hardcoded text
strings with i18n translation keys. Specifically, replace the hardcoded
copyright year and company name text "2026 CoIMS. All rights reserved. Developed
by" with an appropriate i18n key using the i18n attribute or interpolation
syntax, and replace the link text "Debugsoft" with a separate i18n translation
key. This ensures all user-facing text in the footer is translatable and
supports multiple languages through the application's internationalization
system.
Source: Coding guidelines
| font-size: 0.75rem; | ||
| color: $text-dark; | ||
| text-align: right; | ||
| word-break: break-word; |
There was a problem hiding this comment.
Replace deprecated word-break: break-word value.
Stylelint correctly flags that break-word is deprecated for the word-break property. Use overflow-wrap: break-word instead.
🔧 Proposed fix
.version-value {
font-weight: 400;
font-size: 0.75rem;
color: $text-dark;
text-align: right;
- word-break: break-word;
+ overflow-wrap: break-word;
max-width: 60%;
}📝 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.
Suggested change
| word-break: break-word; | |
| .version-value { | |
| font-weight: 400; | |
| font-size: 0.75rem; | |
| color: $text-dark; | |
| text-align: right; | |
| overflow-wrap: break-word; | |
| max-width: 60%; | |
| } |
🧰 Tools
🪛 Stylelint (17.13.0)
[error] 344-344: Deprecated keyword "break-word" for property "word-break" (declaration-property-value-keyword-no-deprecated)
(declaration-property-value-keyword-no-deprecated)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/login/login.component.scss` at line 344, The `word-break: break-word`
property value is deprecated in CSS. Replace this deprecated value by removing
the `word-break: break-word` line and adding `overflow-wrap: break-word` in the
login.component.scss file at line 344. The overflow-wrap property with the
break-word value provides the same behavior while following modern CSS
standards.
Source: Linters/SAST tools
| "version": { | ||
| "Tenant": "Instancia", | ||
| "Mifos WebApp": "Mifos® WebApp", | ||
| "WebApp": "WebApp", |
There was a problem hiding this comment.
Untranslated "WebApp" label breaks translation consistency.
The new labels.version.WebApp entry at line 435 has an identical key and value ("WebApp": "WebApp"). All other labels in this section and throughout the translation file provide proper Spanish translations or branded equivalents:
- Line 434:
"Mifos WebApp": "Mifos® WebApp" - Line 436:
"Apache Fineract": "Apache Fineract®" - Line 437:
"Server": "Servidor"
The "WebApp" entry should either be translated to Spanish (e.g., "WebApp": "Aplicación Web") or clarified if it's intentionally untranslated as a brand identifier. If untranslated by design, ensure this pattern is consistent across all 14 locale translation files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/assets/translations/es-CL.json` at line 435, The "WebApp" entry in the
labels.version section of the es-CL.json translation file is untranslated
(identical key and value), which breaks consistency with surrounding entries
like "Mifos WebApp" that are properly translated to Spanish or branded
equivalents. Replace the untranslated "WebApp": "WebApp" with a Spanish
translation such as "Aplicación Web" or ensure it matches an intentional
branding pattern used consistently. If the entry is meant to remain untranslated
as a brand identifier, verify this same pattern is applied uniformly across all
14 locale translation files for consistency.
Comment on lines
434
to
+435
| "Mifos WebApp": "Mifos® WebApp", | ||
| "WebApp": "WebApp", |
There was a problem hiding this comment.
Rebrand key contract is still broken for System Information label.
Line 435 adds labels.version.WebApp, but src/app/system/system-information/system-information.component.html still reads labels.version.Mifos WebApp (Line 14 there). With Line 434 unchanged ("Mifos® WebApp"), the old brand will still render in that screen.
Use one key consistently across both sides (or map both keys to the same rebranded value during migration), e.g. keep labels.version.Mifos WebApp but update its value to the new label and remove/add aliases intentionally.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/assets/translations/lt-LT.json` around lines 434 - 435, The translation
key mismatch between the lt-LT.json file and the
system-information.component.html file needs to be resolved. The component
references labels.version.Mifos WebApp (at line 14 of
system-information.component.html) but the translation file is adding a new
WebApp key while leaving Mifos WebApp unchanged. Choose one consistent approach:
either update the component to reference the new WebApp key instead of Mifos
WebApp, or keep using Mifos WebApp as the key in the translation file and ensure
its value is set to the new rebranded value (removing the duplicate WebApp entry
if it was only added for this migration). Ensure the key name used in the
component matches exactly with the key name in the translation file.
Comment on lines
+28
to
+32
| baseApiUrls: loadedEnv['fineractApiUrls'] || 'https://103.175.192.233,' + window.location.origin, | ||
| // For connecting to server running elsewhere set the base API URL | ||
| baseApiUrl: | ||
| loadedEnv['fineractApiUrl'] || | ||
| (loadedEnv['fineractApiUrls']?.length > 0 ? loadedEnv['fineractApiUrls'].split(',')[0] : window.location.origin), | ||
| (loadedEnv['fineractApiUrls']?.length > 0 ? loadedEnv['fineractApiUrls'].split(',')[0] : 'https://103.175.192.233'), |
There was a problem hiding this comment.
Production fallback now pins API traffic to a fixed host
Line 28 and Line 32 introduce a hardcoded production default backend. If window.env is missing/incomplete at deploy time, all API/auth traffic defaults to that endpoint instead of staying deployment-local.
Proposed fix
- baseApiUrls: loadedEnv['fineractApiUrls'] || 'https://103.175.192.233,' + window.location.origin,
+ baseApiUrls: loadedEnv['fineractApiUrls'] || window.location.origin,
...
- (loadedEnv['fineractApiUrls']?.length > 0 ? loadedEnv['fineractApiUrls'].split(',')[0] : 'https://103.175.192.233'),
+ (loadedEnv['fineractApiUrls']?.length > 0 ? loadedEnv['fineractApiUrls'].split(',')[0] : window.location.origin),📝 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.
Suggested change
| baseApiUrls: loadedEnv['fineractApiUrls'] || 'https://103.175.192.233,' + window.location.origin, | |
| // For connecting to server running elsewhere set the base API URL | |
| baseApiUrl: | |
| loadedEnv['fineractApiUrl'] || | |
| (loadedEnv['fineractApiUrls']?.length > 0 ? loadedEnv['fineractApiUrls'].split(',')[0] : window.location.origin), | |
| (loadedEnv['fineractApiUrls']?.length > 0 ? loadedEnv['fineractApiUrls'].split(',')[0] : 'https://103.175.192.233'), | |
| baseApiUrls: loadedEnv['fineractApiUrls'] || window.location.origin, | |
| // For connecting to server running elsewhere set the base API URL | |
| baseApiUrl: | |
| loadedEnv['fineractApiUrl'] || | |
| (loadedEnv['fineractApiUrls']?.length > 0 ? loadedEnv['fineractApiUrls'].split(',')[0] : window.location.origin), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/environments/environment.prod.ts` around lines 28 - 32, The production
environment configuration file contains hardcoded IP addresses
(https://103.175.192.233) as fallback values for the baseApiUrls and baseApiUrl
properties, which violates the principle of keeping deployment targets
environment-driven. Replace these hardcoded fallback values in both locations
with a deployment-local alternative such as window.location.origin, or remove
the fallback entirely to ensure that environment variables must be properly
configured at deploy time, preventing API traffic from being inadvertently
pinned to a fixed infrastructure endpoint.
Source: Coding guidelines
Comment on lines
+31
to
+35
| baseApiUrls: loadedEnv.fineractApiUrls || 'https://103.175.192.233,' + window.location.origin, | ||
| // For connecting to server running elsewhere set the base API URL | ||
| baseApiUrl: | ||
| loadedEnv.fineractApiUrl || | ||
| (loadedEnv.fineractApiUrls?.length > 0 ? loadedEnv.fineractApiUrls.split(',')[0] : window.location.origin), | ||
| (loadedEnv.fineractApiUrls?.length > 0 ? loadedEnv.fineractApiUrls.split(',')[0] : 'https://103.175.192.233'), |
There was a problem hiding this comment.
Hardcoded external backend fallback can misroute all default traffic
Line 31 and Line 35 now force a concrete external host when env injection is absent. Given src/app/settings/settings.service.ts uses environment.baseApiUrl as the runtime server default, this can send login/API traffic to the wrong backend by default.
Proposed fix
- baseApiUrls: loadedEnv.fineractApiUrls || 'https://103.175.192.233,' + window.location.origin,
+ baseApiUrls: loadedEnv.fineractApiUrls || window.location.origin,
...
- (loadedEnv.fineractApiUrls?.length > 0 ? loadedEnv.fineractApiUrls.split(',')[0] : 'https://103.175.192.233'),
+ (loadedEnv.fineractApiUrls?.length > 0 ? loadedEnv.fineractApiUrls.split(',')[0] : window.location.origin),📝 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.
Suggested change
| baseApiUrls: loadedEnv.fineractApiUrls || 'https://103.175.192.233,' + window.location.origin, | |
| // For connecting to server running elsewhere set the base API URL | |
| baseApiUrl: | |
| loadedEnv.fineractApiUrl || | |
| (loadedEnv.fineractApiUrls?.length > 0 ? loadedEnv.fineractApiUrls.split(',')[0] : window.location.origin), | |
| (loadedEnv.fineractApiUrls?.length > 0 ? loadedEnv.fineractApiUrls.split(',')[0] : 'https://103.175.192.233'), | |
| baseApiUrls: loadedEnv.fineractApiUrls || window.location.origin, | |
| // For connecting to server running elsewhere set the base API URL | |
| baseApiUrl: | |
| loadedEnv.fineractApiUrl || | |
| (loadedEnv.fineractApiUrls?.length > 0 ? loadedEnv.fineractApiUrls.split(',')[0] : window.location.origin), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/environments/environment.ts` around lines 31 - 35, The baseApiUrls
property on line 31 and the baseApiUrl property on line 35 both contain
hardcoded external backend URLs (specifically 'https://103.175.192.233') as
fallback values when environment variables are not provided. This violates
coding guidelines and can misroute traffic to incorrect backends. Replace these
hardcoded deployment targets with environment-driven configuration or use a more
appropriate fallback such as the current window.location.origin approach that is
already being used as a secondary fallback. Ensure that loadedEnv variables are
properly injected and that no concrete external hosts are hardcoded in the
environment configuration file.
Source: Coding guidelines
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/assets/translations/en-US.json (1)
1-4484:⚠️ Potential issue | 🟡 MinorRemove UTF-8 BOM and apply Prettier formatting to translation file.
The file
src/assets/translations/en-US.jsoncontains a UTF-8 Byte Order Mark (BOM) at the start, which prevents JSON parsing and Prettier formatting. Remove the BOM and run Prettier formatting:# Remove BOM and format with Prettier npx prettier --write src/assets/translations/en-US.jsonThe repository's Prettier configuration (
.prettierrc) is properly set up withprettier-plugin-multiline-arrays, and the lint script includesprettier . --check. After applying the fix above, the file will be compliant with the coding guideline requiring Prettier formatting before merge.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/assets/translations/en-US.json` around lines 1 - 4484, The file src/assets/translations/en-US.json contains a UTF-8 Byte Order Mark (BOM) at the beginning (before the opening curly brace) which prevents proper JSON parsing and Prettier formatting. Remove this invisible BOM character from the start of the file and then apply Prettier formatting by running the command `npx prettier --write src/assets/translations/en-US.json` to ensure the file complies with the project's Prettier formatting requirements before merging.Source: Coding guidelines
src/app/web-app.component.ts (1)
177-193:⚠️ Potential issue | 🟠 MajorThe observable pipeline extracts route data but ignores the title property, breaking dynamic page title functionality that routes expect to work.
Routes throughout the application (products, reports, users, templates) define custom
titleproperties in their route data configuration. The comment on line 176 explicitly states the intent: "Change page title on navigation or language change, based on route data". However, the subscription callback doesn't use the extracted route data—it always sets a static 'COIMS' title regardless of the route's custom title.The
mergeMap((route) => route.data)correctly fetches route data, but the emitted data is never accessed. Fix by using the route data to extract and set the appropriate title, potentially with i18n translation support.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/web-app.component.ts` around lines 177 - 193, The subscription callback in the merge observable pipeline is ignoring the route data that was extracted via mergeMap((route) => route.data). Currently it always sets a static 'COIMS' title, but it should instead extract the title property from the emitted route data. Modify the subscribe callback to accept the route data as a parameter, extract the title property from that data object, and pass it to this.titleService.setTitle() instead of the hardcoded string. Additionally, consider applying i18n translation to the extracted title value to support dynamic page titles based on the current language and route configuration.src/app/deposits/fixed-deposits/fixed-deposit-account-view/fixed-deposit-account-view.component.ts (1)
112-118:⚠️ Potential issue | 🔴 CriticalUpdate entityType check for members route migration.
The constructor checks
this.router.url.includes('clients')to setentityType, but routes now use/members/:clientId/.... This condition will fail andentityTypewill remain undefined, breaking the template display that uses{{ 'labels.text.' + entityType | translate }}.Additionally, this issue exists in 7 other account view components:
src/app/shares/shares-account-view/shares-account-view.component.tssrc/app/savings/savings-account-view/savings-account-view.component.tsand general-tabsrc/app/loans/loans-view/loans-view.component.tssrc/app/deposits/recurring-deposits/recurring-deposits-account-view/recurring-deposits-account-view.component.tsand general-tabProposed fix
- if (this.router.url.includes('clients')) { + if (this.router.url.includes('members')) { this.entityType = 'Client'; } else if (this.router.url.includes('groups')) {Apply the same change to all 8 affected files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/deposits/fixed-deposits/fixed-deposit-account-view/fixed-deposit-account-view.component.ts` around lines 112 - 118, The entityType assignment logic in the constructor of fixed-deposit-account-view.component.ts (and 7 other account view components) checks for route segments like 'clients', 'groups', and 'centers' using this.router.url.includes(), but routes have been migrated to use '/members/:clientId/...' format, causing the entityType to remain undefined. Update the conditional checks in all 8 affected files (fixed-deposit-account-view.component.ts, shares-account-view.component.ts, savings-account-view.component.ts, savings-general-tab.component.ts, loans-view.component.ts, recurring-deposits-account-view.component.ts, and recurring-deposits-general-tab.component.ts) to also check for 'members' in the URL so that entityType is properly set regardless of whether the old route pattern or new member-based route pattern is used.src/app/deposits/recurring-deposits/recurring-deposits-account-view/recurring-deposits-account-view.component.ts (1)
112-118:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winUpdate the entityType logic to recognize 'members' route.
The entityType detection logic checks for
'clients'in the URL, but after this PR the route path uses'members'instead. This will cause entityType to remain undefined for member-based routes, breaking the intended behavior.🐛 Proposed fix
- if (this.router.url.includes('clients')) { + if (this.router.url.includes('members')) { this.entityType = 'Client';Alternatively, if the entityType should also be renamed to 'Member', update both the condition and the value:
- if (this.router.url.includes('clients')) { - this.entityType = 'Client'; + if (this.router.url.includes('members')) { + this.entityType = 'Member';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/deposits/recurring-deposits/recurring-deposits-account-view/recurring-deposits-account-view.component.ts` around lines 112 - 118, The entityType detection logic in the if-else chain needs to be updated to recognize the new route path. Add a condition to check if this.router.url.includes('members') before or after the existing conditions that check for 'clients', 'groups', and 'centers'. When 'members' is found in the URL, set this.entityType to the appropriate value ('Member' if being renamed to match the new route, or 'Client' if maintaining the previous entity classification). This ensures entityType is properly defined when the route uses 'members' instead of the previous 'clients' path.
🧹 Nitpick comments (2)
src/app/shared/footer/footer.component.html (1)
32-32: Use property binding forngClassinstead of interpolation.Line 32 uses
ngClass="{{ styleClass }}"but should use[ngClass]="styleClass". AlthoughstyleClassis typed asstring, property binding is the idiomatic Angular pattern and avoids potential brittleness if the type changes in the future.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/shared/footer/footer.component.html` at line 32, Replace the string interpolation syntax on the ngClass directive in the footer-shell div element with property binding syntax. Change `ngClass="{{ styleClass }}"` to `[ngClass]="styleClass"` to follow Angular's idiomatic pattern for directive bindings and ensure better type safety if the styleClass property type changes in the future.src/app/app-routing.module.ts (1)
36-37: ⚖️ Poor tradeoffConsider renaming the module and folder to align with the new 'members' path.
The route path is now 'members', but it still loads
ClientsModulefrom the./clients/folder. This semantic mismatch can confuse future maintainers who expect the path and module naming to align.For consistency with the terminology migration, consider renaming:
./clients/clients.module.ts→./members/members.module.tsClientsModuleclass →MembersModulesrc/app/clients/folder →src/app/members/🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/app-routing.module.ts` around lines 36 - 37, The route path 'members' is semantically mismatched with the ClientsModule folder and naming convention. Rename the entire clients folder to members, rename the clients.module.ts file to members.module.ts, and rename the ClientsModule class to MembersModule. Update the loadChildren import statement in the routing configuration to import from './members/members.module' and change the module reference from m.ClientsModule to m.MembersModule. Ensure all internal references and exports within the renamed module file are updated to use MembersModule consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/app/clients/client-stepper/client-general-step/client-general-step.component.html`:
- Around line 11-12: The responsive sizing classes (flex-20, flex-15, flex-40,
flex-23, flex-30, flex-98, flex-10) used in client-general-step.component.html
are currently defined in src/main.scss with hardcoded percentage values. Replace
these hardcoded values with SCSS variables by either creating new variables in
src/main.scss or src/theme/mifosx-theme.scss for each flex percentage value,
then update the class definitions to reference these variables instead of the
explicit percentage values. Ensure all responsive sizing classes used throughout
the component are updated consistently.
- Around line 79-132: The removal of the name-fields-row wrapper container
eliminated essential CSS rules that controlled spacing and responsive layout for
the firstname, middlename, and lastname form fields. To fix this, either restore
the wrapper div around the three mat-form-field elements (for firstname,
middlename, and lastname) with its original CSS rules including the gap property
for horizontal spacing and media queries for responsive behavior, or apply
equivalent CSS classes/styles directly to each mat-form-field element to achieve
the same layout effect with proper spacing between fields and responsive
adjustments for screens 768px and below.
In `@src/app/clients/clients-routing.module.ts`:
- Line 71: In the clients-routing.module.ts file, the route data configuration
contains incorrect i18n translation key string literals that don't match the
actual translation keys in the translation files. Update the title and
breadcrumb properties in the route metadata: replace 'Create Member' with
'Create Client' on line 71, and replace 'Members View' with 'Clients View' on
line 81. These corrections will ensure the breadcrumb component can properly
perform i18n lookup using the existing translation keys.
In `@src/app/shared/footer/footer.component.html`:
- Around line 34-35: The footer.component.html template contains hardcoded
English text "All rights reserved. Developed by" that bypasses localization.
Create a new translation key for this user-facing string and apply the translate
pipe to it, similar to how 'APP_NAME' is already translated in the same
template. After updating the template with the translation key, run npm run
translations:extract to add the new string to the translation files for all
supported locales.
In `@src/app/shared/footer/footer.component.scss`:
- Line 68: The gap property in the footer component SCSS file is set to 0.25rem
(4px), which violates the project's 8px grid spacing system. Update the gap
value to align with the 8px grid by changing it to 0.5rem (8px) or another
multiple of 8px such as 1rem (16px), depending on the intended visual spacing
for the footer layout.
In `@src/app/web-app.component.ts`:
- Around line 191-193: The titleService.setTitle() method in the subscribe block
is using a hardcoded string 'COIMS' instead of a translated value, which
violates i18n guidelines. Replace the hardcoded string with a call to the
translate service to fetch the 'APP_NAME' translation key. Inject the
TranslateService if not already available, and use its get method to retrieve
the translated APP_NAME value, then pass that translated value to
titleService.setTitle() instead of the hardcoded 'COIMS' string.
---
Outside diff comments:
In
`@src/app/deposits/fixed-deposits/fixed-deposit-account-view/fixed-deposit-account-view.component.ts`:
- Around line 112-118: The entityType assignment logic in the constructor of
fixed-deposit-account-view.component.ts (and 7 other account view components)
checks for route segments like 'clients', 'groups', and 'centers' using
this.router.url.includes(), but routes have been migrated to use
'/members/:clientId/...' format, causing the entityType to remain undefined.
Update the conditional checks in all 8 affected files
(fixed-deposit-account-view.component.ts, shares-account-view.component.ts,
savings-account-view.component.ts, savings-general-tab.component.ts,
loans-view.component.ts, recurring-deposits-account-view.component.ts, and
recurring-deposits-general-tab.component.ts) to also check for 'members' in the
URL so that entityType is properly set regardless of whether the old route
pattern or new member-based route pattern is used.
In
`@src/app/deposits/recurring-deposits/recurring-deposits-account-view/recurring-deposits-account-view.component.ts`:
- Around line 112-118: The entityType detection logic in the if-else chain needs
to be updated to recognize the new route path. Add a condition to check if
this.router.url.includes('members') before or after the existing conditions that
check for 'clients', 'groups', and 'centers'. When 'members' is found in the
URL, set this.entityType to the appropriate value ('Member' if being renamed to
match the new route, or 'Client' if maintaining the previous entity
classification). This ensures entityType is properly defined when the route uses
'members' instead of the previous 'clients' path.
In `@src/app/web-app.component.ts`:
- Around line 177-193: The subscription callback in the merge observable
pipeline is ignoring the route data that was extracted via mergeMap((route) =>
route.data). Currently it always sets a static 'COIMS' title, but it should
instead extract the title property from the emitted route data. Modify the
subscribe callback to accept the route data as a parameter, extract the title
property from that data object, and pass it to this.titleService.setTitle()
instead of the hardcoded string. Additionally, consider applying i18n
translation to the extracted title value to support dynamic page titles based on
the current language and route configuration.
In `@src/assets/translations/en-US.json`:
- Around line 1-4484: The file src/assets/translations/en-US.json contains a
UTF-8 Byte Order Mark (BOM) at the beginning (before the opening curly brace)
which prevents proper JSON parsing and Prettier formatting. Remove this
invisible BOM character from the start of the file and then apply Prettier
formatting by running the command `npx prettier --write
src/assets/translations/en-US.json` to ensure the file complies with the
project's Prettier formatting requirements before merging.
---
Nitpick comments:
In `@src/app/app-routing.module.ts`:
- Around line 36-37: The route path 'members' is semantically mismatched with
the ClientsModule folder and naming convention. Rename the entire clients folder
to members, rename the clients.module.ts file to members.module.ts, and rename
the ClientsModule class to MembersModule. Update the loadChildren import
statement in the routing configuration to import from './members/members.module'
and change the module reference from m.ClientsModule to m.MembersModule. Ensure
all internal references and exports within the renamed module file are updated
to use MembersModule consistently.
In `@src/app/shared/footer/footer.component.html`:
- Line 32: Replace the string interpolation syntax on the ngClass directive in
the footer-shell div element with property binding syntax. Change `ngClass="{{
styleClass }}"` to `[ngClass]="styleClass"` to follow Angular's idiomatic
pattern for directive bindings and ensure better type safety if the styleClass
property type changes in the future.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fa843e35-508c-41d2-ae98-8e46319647d2
📒 Files selected for processing (40)
src/app/account-transfers/view-account-transfer/view-account-transfer.component.tssrc/app/app-routing.module.tssrc/app/clients/client-stepper/client-general-step/client-general-step.component.htmlsrc/app/clients/client-stepper/client-general-step/client-general-step.component.scsssrc/app/clients/clients-routing.module.tssrc/app/clients/clients-view/clients-view.component.tssrc/app/collaterals/view-collateral/view-collateral.component.htmlsrc/app/core/authentication/authentication.service.tssrc/app/core/shell/sidenav/sidenav.component.htmlsrc/app/core/shell/toolbar/toolbar.component.htmlsrc/app/deposits/fixed-deposits/fixed-deposit-account-view/charges-tab/charges-tab.component.tssrc/app/deposits/fixed-deposits/fixed-deposit-account-view/fixed-deposit-account-view.component.tssrc/app/deposits/fixed-deposits/fixed-deposit-account-view/transactions-tab/transactions-tab.component.tssrc/app/deposits/recurring-deposits/recurring-deposits-account-view/charges-tab/charges-tab.component.tssrc/app/deposits/recurring-deposits/recurring-deposits-account-view/recurring-deposits-account-view.component.tssrc/app/groups/groups-view/general-tab/general-tab.component.htmlsrc/app/home/home.component.tssrc/app/home/home.module.tssrc/app/home/warning-dialog/warning-dialog.component.htmlsrc/app/home/warning-dialog/warning-dialog.component.scsssrc/app/home/warning-dialog/warning-dialog.component.tssrc/app/keyboards-shortcut-config.tssrc/app/loans/loans-view/loan-account-tab-base.component.tssrc/app/loans/loans-view/transactions/view-transaction/view-transaction.component.tssrc/app/products/loan-products/common/loan-product-base.component.tssrc/app/savings/savings-account-view/transactions-tab/transactions-tab.component.tssrc/app/shared/account-number/account-number.component.tssrc/app/shared/footer/footer.component.htmlsrc/app/shared/footer/footer.component.scsssrc/app/shared/footer/footer.component.tssrc/app/system/manage-jobs/cob-workflow/loan-locked/loan-locked.component.tssrc/app/tasks/checker-inbox-and-tasks-tabs/client-approval/client-approval.component.htmlsrc/app/tasks/checker-inbox-and-tasks-tabs/council-approval/council-approval.component.htmlsrc/app/tasks/checker-inbox-and-tasks-tabs/loan-approval/loan-approval.component.htmlsrc/app/tasks/checker-inbox-and-tasks-tabs/loan-disbursal/loan-disbursal.component.htmlsrc/app/tasks/checker-inbox-and-tasks-tabs/reschedule-loan/reschedule-loan.component.htmlsrc/app/web-app.component.tssrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/fr-FR.json
💤 Files with no reviewable changes (6)
- src/app/core/shell/sidenav/sidenav.component.html
- src/app/home/warning-dialog/warning-dialog.component.scss
- src/app/home/warning-dialog/warning-dialog.component.html
- src/app/home/warning-dialog/warning-dialog.component.ts
- src/app/core/authentication/authentication.service.ts
- src/app/home/home.component.ts
✅ Files skipped from review due to trivial changes (5)
- src/app/clients/client-stepper/client-general-step/client-general-step.component.scss
- src/app/loans/loans-view/loan-account-tab-base.component.ts
- src/app/tasks/checker-inbox-and-tasks-tabs/loan-disbursal/loan-disbursal.component.html
- src/app/shared/footer/footer.component.ts
- src/assets/translations/de-DE.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/assets/translations/fr-FR.json
Comment on lines
+11
to
+12
| <!-- Row 1: Office, Legal Form, External Id, First Name / Middle Name (or Full Name for Entity) --> | ||
| <mat-form-field class="flex-20"> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check that flex-* and margin-v classes use SCSS variables and follow 8px grid
echo "=== Checking flex-* utility class definitions ==="
rg -n "\.flex-(10|15|20|23|30|40|98)" --type=scss -C2
echo -e "\n=== Checking margin-v class definition ==="
rg -n "\.margin-v" --type=scss -C2
echo -e "\n=== Checking SCSS variable usage in src/main.scss ==="
cat src/main.scss | head -50
echo -e "\n=== Checking SCSS variable usage in src/theme/mifosx-theme.scss ==="
cat src/theme/mifosx-theme.scss | head -50Repository: openMF/web-app
Length of output: 3511
🏁 Script executed:
#!/bin/bash
# Search for where flex-* classes are defined
echo "=== Searching for flex-10, flex-15, flex-20 class definitions ==="
rg "flex-(10|15|20|23|30|40|98)" -n
echo -e "\n=== Searching for margin-v class definition ==="
rg "margin-v" -n
echo -e "\n=== Checking for utility/helper SCSS files ==="
find . -name "*helper*" -o -name "*util*" -o -name "*grid*" | head -20Repository: openMF/web-app
Length of output: 50371
🏁 Script executed:
#!/bin/bash
# Read the actual SCSS definitions for flex-* classes
echo "=== Checking flex-10 definition (around line 275) ==="
sed -n '270,285p' src/main.scss
echo -e "\n=== Checking flex-15 definition (around line 287) ==="
sed -n '285,295p' src/main.scss
echo -e "\n=== Checking flex-20 definition (around line 299) ==="
sed -n '295,310p' src/main.scss
echo -e "\n=== Checking flex-30 definition (around line 323) ==="
sed -n '318,335p' src/main.scss
echo -e "\n=== Checking flex-40 definition (around line 347) ==="
sed -n '342,355p' src/main.scss
echo -e "\n=== Checking flex-98 definition (around line 473) ==="
sed -n '468,480p' src/main.scss
echo -e "\n=== Searching for margin-v class ==="
rg "margin-v" -n src/main.scssRepository: openMF/web-app
Length of output: 1145
Verify responsive sizing classes use SCSS variables instead of hardcoded values.
Multiple form fields use responsive sizing classes (flex-20, flex-15, flex-40, flex-23, flex-30, flex-98, flex-10) that currently contain hardcoded percentage values (e.g., flex: 0 0 20%) in src/main.scss. Per coding guidelines, these should leverage SCSS variables from src/main.scss or src/theme/mifosx-theme.scss rather than explicit hardcoded values.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/app/clients/client-stepper/client-general-step/client-general-step.component.html`
around lines 11 - 12, The responsive sizing classes (flex-20, flex-15, flex-40,
flex-23, flex-30, flex-98, flex-10) used in client-general-step.component.html
are currently defined in src/main.scss with hardcoded percentage values. Replace
these hardcoded values with SCSS variables by either creating new variables in
src/main.scss or src/theme/mifosx-theme.scss for each flex percentage value,
then update the class definitions to reference these variables instead of the
explicit percentage values. Ensure all responsive sizing classes used throughout
the component are updated consistently.
Source: Coding guidelines
Comment on lines
+79
to
+132
| @if (createClientForm.get('firstname')) { | ||
| <mat-form-field class="flex-20"> | ||
| <mat-label>{{ 'labels.inputs.First Name' | translate }}</mat-label> | ||
| <input matInput required formControlName="firstname" /> | ||
| @if (createClientForm.controls.firstname.hasError('required')) { | ||
| <mat-error> | ||
| {{ 'labels.inputs.Client first name' | translate }} {{ 'labels.commons.is' | translate }} | ||
| <strong>{{ 'labels.commons.required' | translate }}</strong> | ||
| </mat-error> | ||
| } | ||
| @if (createClientForm.get('middlename')) { | ||
| <mat-form-field class="name-field middle-name"> | ||
| <mat-label>{{ 'labels.inputs.Middle Name' | translate }}</mat-label> | ||
| <input matInput formControlName="middlename" /> | ||
| @if (createClientForm.controls.middlename.hasError('pattern')) { | ||
| <mat-error> | ||
| {{ 'labels.inputs.Client middle name' | translate }} | ||
| <strong>{{ 'labels.inputs.cannot' | translate }}</strong> | ||
| {{ 'labels.commons.begin with a special character or number' | translate }} | ||
| </mat-error> | ||
| } | ||
| </mat-form-field> | ||
| @if (createClientForm.controls.firstname.hasError('pattern')) { | ||
| <mat-error> | ||
| {{ 'labels.inputs.Client first name' | translate }} | ||
| <strong>{{ 'labels.inputs.cannot' | translate }}</strong> | ||
| {{ 'labels.commons.begin with a special character or number' | translate }} | ||
| </mat-error> | ||
| } | ||
| @if (createClientForm.get('lastname')) { | ||
| <mat-form-field class="name-field last-name"> | ||
| <mat-label>{{ 'labels.inputs.Last Name' | translate }}</mat-label> | ||
| <input matInput required formControlName="lastname" /> | ||
| @if (createClientForm.controls.lastname.hasError('required')) { | ||
| <mat-error> | ||
| {{ 'labels.inputs.Client last name' | translate }} {{ 'labels.commons.is' | translate }} | ||
| <strong>{{ 'labels.commons.required' | translate }}</strong> | ||
| </mat-error> | ||
| } | ||
| @if (createClientForm.controls.lastname.hasError('pattern')) { | ||
| <mat-error> | ||
| {{ 'labels.inputs.Client last name' | translate }} | ||
| <strong>{{ 'labels.inputs.cannot' | translate }}</strong> | ||
| {{ 'labels.commons.begin with a special character or number' | translate }} | ||
| </mat-error> | ||
| } | ||
| </mat-form-field> | ||
| </mat-form-field> | ||
| } | ||
|
|
||
| @if (createClientForm.get('middlename')) { | ||
| <mat-form-field class="flex-20"> | ||
| <mat-label>{{ 'labels.inputs.Middle Name' | translate }}</mat-label> | ||
| <input matInput formControlName="middlename" /> | ||
| @if (createClientForm.controls.middlename.hasError('pattern')) { | ||
| <mat-error> | ||
| {{ 'labels.inputs.Client middle name' | translate }} | ||
| <strong>{{ 'labels.inputs.cannot' | translate }}</strong> | ||
| {{ 'labels.commons.begin with a special character or number' | translate }} | ||
| </mat-error> | ||
| } | ||
| </div> | ||
| </mat-form-field> | ||
| } | ||
|
|
||
| <mat-divider></mat-divider> | ||
| <!-- Row 2: Last Name, Date of Birth, Gender, Staff, Is Staff? --> | ||
| @if (createClientForm.get('lastname')) { | ||
| <mat-form-field class="flex-20"> | ||
| <mat-label>{{ 'labels.inputs.Last Name' | translate }}</mat-label> | ||
| <input matInput required formControlName="lastname" /> | ||
| @if (createClientForm.controls.lastname.hasError('required')) { | ||
| <mat-error> | ||
| {{ 'labels.inputs.Client last name' | translate }} {{ 'labels.commons.is' | translate }} | ||
| <strong>{{ 'labels.commons.required' | translate }}</strong> | ||
| </mat-error> | ||
| } | ||
| @if (createClientForm.controls.lastname.hasError('pattern')) { | ||
| <mat-error> | ||
| {{ 'labels.inputs.Client last name' | translate }} | ||
| <strong>{{ 'labels.inputs.cannot' | translate }}</strong> | ||
| {{ 'labels.commons.begin with a special character or number' | translate }} | ||
| </mat-error> | ||
| } | ||
| </mat-form-field> | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify TypeScript component manages firstname/middlename/lastname form controls based on legalFormId
echo "=== Checking component form control management ==="
ast-grep --pattern $'addControl($$$) {
$$$
}'
echo -e "\n=== Checking for legalFormId-based control logic ==="
rg -n "legalFormId|firstname|middlename|lastname" src/app/clients/client-stepper/client-general-step/client-general-step.component.ts -A3 -B3
echo -e "\n=== Checking if name-fields-row CSS was removed ==="
rg -n "name-fields-row" --type=scssRepository: openMF/web-app
Length of output: 2271
🏁 Script executed:
#!/bin/bash
# Find and check CSS/SCSS files for name-fields-row and flex-20 definitions
echo "=== Searching for name-fields-row in CSS/SCSS files ==="
find src -type f \( -name "*.scss" -o -name "*.css" \) -exec grep -l "name-fields-row\|flex-20" {} \;
echo -e "\n=== Checking component-specific SCSS file ==="
ls -la src/app/clients/client-stepper/client-general-step/ | grep -E "\.(scss|css)$"
echo -e "\n=== Checking for flex-20 definition in theme files ==="
grep -r "flex-20" src/theme/ src/main.scss 2>/dev/null || echo "Not found in theme files"
echo -e "\n=== Checking for grid/flex utilities in main styles ==="
rg "flex-20|name-fields-row" src/main.scss src/styles.scss 2>/dev/null || echo "Pattern not found"Repository: openMF/web-app
Length of output: 527
🏁 Script executed:
#!/bin/bash
# Check the actual CSS definitions for name-fields-row and related styling
echo "=== Checking name-fields-row definition in component SCSS ==="
cat -n src/app/clients/client-stepper/client-general-step/client-general-step.component.scss
echo -e "\n=== Checking name-fields-row definition in main.scss ==="
rg -A5 "\.name-fields-row" src/main.scss
echo -e "\n=== Checking flex-20 definition ==="
rg -A3 "\.flex-20" src/main.scssRepository: openMF/web-app
Length of output: 3441
Fix CSS layout issues caused by removing the name-fields-row wrapper container.
The refactoring removed the name-fields-row wrapper div, which had essential CSS rules that are now lost:
- The flex container's
gap: 8pxno longer applies horizontally between name fields - Media query rules that adjusted layout for screens ≤768px are no longer active
- The proportional flex sizing (flex: 1.2 for first/last name, flex: 1 for middle name) is replaced by uniform
flex-20(20% width), changing the field proportions - Sibling margin rules that added top spacing to elements following the name fields group are no longer applied
While the TypeScript component correctly manages the form control lifecycle (adding/removing firstname, middlename, lastname based on legalFormId), the HTML refactoring breaks the responsive design and flex layout. Either restore the wrapper container with its CSS or apply equivalent styling directly to the individual mat-form-field elements.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/app/clients/client-stepper/client-general-step/client-general-step.component.html`
around lines 79 - 132, The removal of the name-fields-row wrapper container
eliminated essential CSS rules that controlled spacing and responsive layout for
the firstname, middlename, and lastname form fields. To fix this, either restore
the wrapper div around the three mat-form-field elements (for firstname,
middlename, and lastname) with its original CSS rules including the gap property
for horizontal spacing and media queries for responsive behavior, or apply
equivalent CSS classes/styles directly to each mat-form-field element to achieve
the same layout effect with proper spacing between fields and responsive
adjustments for screens 768px and below.
| { | ||
| path: 'create', | ||
| data: { title: 'Create Client', breadcrumb: 'Create Client', routeParamBreadcrumb: false }, | ||
| data: { title: 'Create Member', breadcrumb: 'Create Member', routeParamBreadcrumb: false }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if "Create Member" translation key exists in translation files
rg -n "Create Member" src/assets/translations/ --type jsonRepository: openMF/web-app
Length of output: 40
🏁 Script executed:
# First, check the exact content of the routing file around line 71
sed -n '65,85p' src/app/clients/clients-routing.module.tsRepository: openMF/web-app
Length of output: 832
🏁 Script executed:
# List translation files to understand structure
find src/assets/translations/ -type f -name "*.json" | head -20Repository: openMF/web-app
Length of output: 513
🏁 Script executed:
# Search for both "Create Member" and "Create Client" to understand the pattern
rg "Create (Member|Client)" src/assets/translations/ -t json --max-count 20Repository: openMF/web-app
Length of output: 2038
🏁 Script executed:
# Check if "Members View" translation key exists
rg "Members View" src/assets/translations/ -t jsonRepository: openMF/web-app
Length of output: 131
🏁 Script executed:
# Check if there are any "Member" keys at all in translation files
rg "Member" src/assets/translations/ -t json | head -20Repository: openMF/web-app
Length of output: 1673
🏁 Script executed:
# Check one translation file to understand the structure
cat src/assets/translations/en-US.json | grep -A2 -B2 "Create Client"Repository: openMF/web-app
Length of output: 463
Use the correct i18n translation keys for route metadata.
The route data uses string literals that don't exist as translation keys. Replace 'Create Member' with 'Create Client' and 'Members View' (on line 81) with 'Clients View' to match the existing translation keys in the translation files. The breadcrumb component requires these to be actual translation keys for proper i18n lookup.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/clients/clients-routing.module.ts` at line 71, In the
clients-routing.module.ts file, the route data configuration contains incorrect
i18n translation key string literals that don't match the actual translation
keys in the translation files. Update the title and breadcrumb properties in the
route metadata: replace 'Create Member' with 'Create Client' on line 71, and
replace 'Members View' with 'Clients View' on line 81. These corrections will
ensure the breadcrumb component can properly perform i18n lookup using the
existing translation keys.
Source: Learnings
Comment on lines
+34
to
+35
| © {{ currentYear }} {{ 'APP_NAME' | translate }}. All rights reserved. Developed by | ||
| <a href="https://dibugsoft.com/" target="_blank" rel="noopener noreferrer">Debugsoft</a> |
There was a problem hiding this comment.
Localize the new footer sentence instead of hardcoding English text.
Line 34 introduces user-facing copy (All rights reserved. Developed by) outside the translation keys; this will bypass locale translations.
As per coding guidelines, src/app/**/*.{html,ts} requires proper i18n variables for all user-facing strings and running npm run translations:extract when adding new strings.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/shared/footer/footer.component.html` around lines 34 - 35, The
footer.component.html template contains hardcoded English text "All rights
reserved. Developed by" that bypasses localization. Create a new translation key
for this user-facing string and apply the translate pipe to it, similar to how
'APP_NAME' is already translated in the same template. After updating the
template with the translation key, run npm run translations:extract to add the
new string to the translation files for all supported locales.
Source: Coding guidelines
| min-width: 90%; | ||
| padding-left: 5%; | ||
| padding-right: 5%; | ||
| gap: 0.25rem; |
There was a problem hiding this comment.
Align footer spacing to the 8px grid.
Line 68 sets gap: 0.25rem (4px), which breaks the 8px spacing system used by the project.
As per coding guidelines, src/**/*.{scss,html} must stick to the 8px grid system for visual design and spacing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/shared/footer/footer.component.scss` at line 68, The gap property in
the footer component SCSS file is set to 0.25rem (4px), which violates the
project's 8px grid spacing system. Update the gap value to align with the 8px
grid by changing it to 0.5rem (8px) or another multiple of 8px such as 1rem
(16px), depending on the intended visual spacing for the footer layout.
Source: Coding guidelines
Comment on lines
+191
to
193
| .subscribe(() => { | ||
| this.titleService.setTitle('COIMS'); | ||
| }); |
There was a problem hiding this comment.
Hardcoded title string violates i18n guidelines.
The title is set to the hardcoded string 'COIMS' instead of using the translation service. As per coding guidelines, all user-facing strings must use i18n variables from @ngx-translate/core. The APP_NAME translation key is already defined in the locale files.
🌐 Proposed fix using translation service
- .subscribe(() => {
- this.titleService.setTitle('COIMS');
+ .subscribe(() => {
+ this.titleService.setTitle(this.translateService.instant('APP_NAME'));
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/web-app.component.ts` around lines 191 - 193, The
titleService.setTitle() method in the subscribe block is using a hardcoded
string 'COIMS' instead of a translated value, which violates i18n guidelines.
Replace the hardcoded string with a call to the translate service to fetch the
'APP_NAME' translation key. Inject the TranslateService if not already
available, and use its get method to retrieve the translated APP_NAME value,
then pass that translated value to titleService.setTitle() instead of the
hardcoded 'COIMS' string.
Source: Coding guidelines
Contributor
|
The code has been developed by Mifos Community, not ni DBUGSPT, please respect the License |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Bug Fixes
Style
Chores