WEB-492: Material Design 3 implementation: Login #2921
Conversation
alberto-art3ch
changed the title
|
Note Reviews pausedUse the following commands to manage reviews:
Note
|
| Cohort / File(s) | Change Summary |
|---|---|
Config & Dependency \.stylelintrc, package.json |
Relaxed stylelint rules (nesting, specificity); added @material/web dependency. |
M3 UI library (new) src/app/shared/m3-ui/index.ts, src/app/shared/m3-ui/m3-button/*, src/app/shared/m3-ui/m3-icon/* |
Added standalone M3ButtonComponent (five variants) and M3IconComponent (multiple styles) with templates, styles, and public exports. |
Login page & form src/app/login/.../login.component.*, src/app/login/login-form/login-form.component.*, src/app/login/login.component.scss, src/app/login/login-form/login-form.component.scss |
Full MD3 visual redesign of login page and form: template refactors, new M3 components usage, password visibility toggle changes, loading UI, header controls and layout adjustments. |
Theming & integration src/app/login/login.component.ts, src/app/shared/theme-toggle/*, src/app/shared/theme-toggle/theming.service.ts, src/theme/_material-web.scss, src/main.scss, src/index.html |
Login subscribes to ThemingService and SettingsService; added theme toggle method and symmetric class toggling; new MD3 SCSS tokens, Material Web button/select styles, and Material Symbols font link. |
Shared selectors — appearance & labels src/app/shared/language-selector/*, src/app/shared/server-selector/*, src/app/shared/tenant-selector/* |
Added appearance and showLabel inputs to select components and updated templates to support 'fill'/'outline' variants; tenant selector icon/placement adjusted; tenant selector local styling removed. |
Shared components & icons src/app/shared/icons.module.ts, src/app/shared/footer/footer.component.* |
Expanded FontAwesome icon registrations; footer template restructured and MatDivider import removed. |
Standalone module export src/app/standalone-shared.module.ts |
Re-exported M3ButtonComponent. |
SCSS & global styles src/main.scss, src/theme/_material-web.scss |
Added MD3 button/select styling, theming tokens and dark-theme overrides; select panel and option styles updated. |
Translations src/assets/translations/* (multiple locales) |
Added three feature labels ("Secure & Reliable", "Community Driven", "Global Impact") and username/password placeholders across locales; removed some duplicated/unused keys in several files. |
Small template tweaks src/app/login/reset-password/*, src/app/system/external-services/.../edit-email.component.html |
Consolidated password-eye conditional rendering to if/else for simpler templates. |
Sequence Diagram(s)
sequenceDiagram
participant User
participant LoginComponent
participant SettingsService
participant ThemingService
User->>LoginComponent: load page
LoginComponent->>SettingsService: getThemePreference()
SettingsService-->>LoginComponent: currentDarkMode
LoginComponent->>ThemingService: setDarkMode(currentDarkMode)
ThemingService->>ThemingService: add/remove theme CSS classes
ThemingService-->>LoginComponent: emit theme change
LoginComponent->>LoginComponent: update themeDarkEnabled
Note over User,LoginComponent: User toggles theme
User->>LoginComponent: toggleTheme()
LoginComponent->>SettingsService: setThemeDarkEnabled(newState)
LoginComponent->>ThemingService: setDarkMode(newState)
ThemingService-->>LoginComponent: emit updated theme
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60–90 minutes
- Areas needing extra attention:
- New M3 components: verify CUSTOM_ELEMENTS usage, inputs/outputs, and fallback behavior.
- LoginComponent lifecycle: subscription management and side-effects from ThemingService.
- SCSS tokens and dark-theme overrides for visual regressions across breakpoints.
- Selector
appearance/showLabelinputs for backward compatibility. - Expanded icon registrations and translation removals (spot-check locales).
Possibly related PRs
- WEB-433-fix:the login form icon overlap #2814 — overlaps with login password icon rendering and visibility-toggle/template changes.
- WEB-487: Implements OAuth 2.1 PKCE flow for authentication #2864 — related to login UI/auth flows and MD3 login redesign elements.
Suggested reviewers
- IOhacker
- adamsaghy
- gkbishnoi07
Pre-merge checks and finishing touches
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly and specifically describes the main change: implementing Material Design 3 for the Login page, with the issue identifier WEB-492 providing clear context. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/shared/theme-toggle/theming.service.ts (1)
26-26: Replace deprecatedaddListenerwithaddEventListener.The
addListenermethod on MediaQueryList is deprecated. UseaddEventListenerinstead for modern browser compatibility.🔎 Proposed fix
- window.matchMedia('(prefers-color-scheme: dark)').addListener((e) => { + window.matchMedia('(prefers-color-scheme: dark)').addEventListener('change', (e) => { const turnOn = e.matches; this.theme.next(turnOn ? 'dark-theme' : 'light-theme'); // Trigger refresh of UI this.ref.tick(); });src/app/standalone-shared.module.ts (1)
1-1: Incorrect file path in comment.The comment states
src/app/shared/standalone-shared.module.tsbut the actual file path issrc/app/standalone-shared.module.ts(without the nestedshareddirectory).🔎 Suggested fix
-// src/app/shared/standalone-shared.module.ts +// src/app/standalone-shared.module.ts
🧹 Nitpick comments (18)
.stylelintrc (1)
18-21: Consider more targeted specificity limits instead of nullifying rules.While increasing
max-nesting-depth(5 → 7) andselector-max-compound-selectors(5 → 8) are reasonable incremental adjustments for MD3 components, completely disabling specificity checks by settingselector-max-specificityandno-descending-specificitytonullmay hide CSS maintainability issues:
selector-max-specificity: nullallows arbitrarily high specificity selectors without warningno-descending-specificity: nulldisables cascade-order validation, which helps prevent unexpected CSS overridesInstead, consider setting these to specific numeric limits (e.g.,
"0,4,0"for specificity) that accommodate MD3 styling without disabling validation entirely. This preserves linting feedback while accommodating the new design system.Did the MD3 component integration require complete specificity validation to be disabled, or would numeric thresholds work?
src/assets/translations/en-US.json (1)
1199-1202: Reorganize login feature translations into a dedicated section.The new translation entries "Secure & Reliable", "Community Driven", and "Global Impact" appear to be login page feature descriptions, but they're placed within the "heading" section alongside unrelated financial/accounting terms like "Not Yet Amortized Amount" and "Charged Off Amount".
This semantic mismatch reduces maintainability and makes the translation file harder to navigate. Consider creating a dedicated section for these entries:
🔎 Proposed reorganization
Move these entries to a more appropriate location, such as:
"placeholder": { "Enter your username": "Enter your username", "Enter your password": "Enter your password" - } + }, + "login": { + "features": { + "Secure & Reliable": "Secure & Reliable", + "Community Driven": "Community Driven", + "Global Impact": "Global Impact" + } + }Or alternatively, group them under an existing appropriate parent key based on your translation structure conventions.
src/assets/translations/lv-LV.json (1)
1195-1198: Verify Latvian translation accuracy for new UI strings.The new translations for the login page feature highlights look structurally correct, but should be verified by a native Latvian speaker to ensure:
- "Drošs un uzticams" accurately conveys "Secure & Reliable"
- "Kopienas vadīts" properly translates "Community Driven"
- "Globālā ietekme" correctly represents "Global Impact"
Note: The "Date" label on line 1195 is a duplicate of the existing translation at line 1500, which is acceptable as they're in different sections (labels vs inputs).
src/assets/translations/cs-CS.json (1)
1195-1197: Consider semantic grouping for feature highlight translations.The new translation keys "Secure & Reliable", "Community Driven", and "Global Impact" appear to be feature highlights for the login interface. While the translations themselves look correct, their placement within
labels.headingimmediately after financial-related labels (like "Amortized Amount", "Buy Down Fees") may not be semantically ideal.Consider:
- Are these meant to be used as section headings on the login page?
- Would a dedicated grouping like
labels.featuresorlabels.loginFeaturesprovide better semantic organization?- Does this placement match the structure in other locale files (en-US, de-DE, etc.)?
💡 Alternative structure suggestion
If these are feature highlights rather than general headings, consider a structure like:
"labels": { "heading": { ... - "Secure & Reliable": "Bezpečné a spolehlivé", - "Community Driven": "Komunitně řízené", - "Global Impact": "Globální dopad" + }, + "features": { + "Secure & Reliable": "Bezpečné a spolehlivé", + "Community Driven": "Komunitně řízené", + "Global Impact": "Globální dopad" } }This would make the purpose and usage of these keys clearer.
src/app/shared/m3-ui/m3-button/m3-button.component.html (1)
1-31: Consider using@switchfor better readability.The multiple
@ifstatements checking the samevariantvalue could be refactored to use Angular's@switchcontrol flow, which is more idiomatic for mutually exclusive conditions.🔎 Proposed refactor
<div [class]="containerClasses"> - @if (variant === 'filled') { + @switch (variant) { + @case ('filled') { <md-filled-button [type]="type" [disabled]="disabled" (click)="onClick($event)"> {{ label }}<ng-content></ng-content> </md-filled-button> } - - @if (variant === 'outlined') { + @case ('outlined') { <md-outlined-button [type]="type" [disabled]="disabled" (click)="onClick($event)"> {{ label }}<ng-content></ng-content> </md-outlined-button> } - - @if (variant === 'elevated') { + @case ('elevated') { <md-elevated-button [type]="type" [disabled]="disabled" (click)="onClick($event)"> {{ label }}<ng-content></ng-content> </md-elevated-button> } - - @if (variant === 'text') { + @case ('text') { <md-text-button [type]="type" [disabled]="disabled" (click)="onClick($event)"> {{ label }}<ng-content></ng-content> </md-text-button> } - - @if (variant === 'tonal') { + @case ('tonal') { <md-filled-tonal-button [type]="type" [disabled]="disabled" (click)="onClick($event)"> {{ label }}<ng-content></ng-content> </md-filled-tonal-button> } + } </div>src/main.scss (1)
28-86: Consider usingcolor-mix()for alpha transparency.The hardcoded color values with alpha transparency (e.g.,
#1074b914,#5ba2ec14) could be replaced with CSScolor-mix()for better maintainability and consistency with MD3 design tokens. This would also make it easier to adjust opacity values globally.Example refactor for hover state:
&:hover { background-color: color-mix(in srgb, var(--md-sys-color-primary) 8%, transparent); }This approach would:
- Use theme tokens consistently
- Make opacity adjustments more explicit
- Align with modern CSS practices
src/app/login/login-form/login-form.component.scss (1)
18-77: Consider scoping::ng-deepwith:hostfor better encapsulation.The standalone
::ng-deepusage is deprecated and pierces through all component boundaries. Prefix it with:hostto limit the scope to this component's template.🔎 Proposed fix
-::ng-deep .tenant-section { +:host ::ng-deep .tenant-section { width: 100%;Apply this pattern to all
::ng-deepselectors throughout the file (lines 18, 84, 194, 203, 316, 335, etc.).src/app/shared/tenant-selector/tenant-selector.component.scss (1)
1-2: Consider removing the empty SCSS file entirely.Since all styling has been moved to the parent component, this file now serves no functional purpose. Empty files with only comments add maintenance overhead.
Alternative approach
Remove the file entirely and update the component's
styleUrlsarray in the TypeScript file to reflect the removal, or simply remove thestyleUrlsproperty if no component-specific styles are needed.src/app/standalone-shared.module.ts (1)
20-20: Duplicate import paths for M3ButtonComponent.The same path is used for both the import declaration (line 20) and the re-export (line 56). Consider using
export { M3ButtonComponent }without repeating the path, since the symbol is already imported.🔎 Suggested refactor
-export { M3ButtonComponent } from './shared/m3-ui/m3-button/m3-button.component'; +export { M3ButtonComponent };Also applies to: 56-56
src/app/shared/server-selector/server-selector.component.ts (1)
50-55: Redundant assignment on line 54.The assignment
this.existMoreThanOneServer = trueis redundant. The else block is only entered when!this.existMoreThanOneServeris false (line 51), meaningexistMoreThanOneServeris already truthy from line 50.🔎 Suggested fix
if (!this.existMoreThanOneServer) { this.settingsService.setServer(this.servers[0]); } else { - this.existMoreThanOneServer = true; this.serverSelector.patchValue(this.settingsService.server); this.form = this.formBuilder.group({src/theme/_material-web.scss (2)
217-221: Verify disabled state contrast in dark theme.The disabled container and label colors are both
#e3e2e6, which is a light gray. On a dark theme background, this may provide adequate visibility, but the container and text being the same color could make disabled buttons appear as plain rectangles with no visible text. Consider using distinct colors for container vs. label.🔎 Suggested adjustment for better distinction
// Disabled state - --md-filled-button-disabled-container-color: #e3e2e6; + --md-filled-button-disabled-container-color: #3c3c3c; --md-filled-button-disabled-container-opacity: 1; - --md-filled-button-disabled-label-text-color: #e3e2e6; + --md-filled-button-disabled-label-text-color: #8c8c8c; --md-filled-button-disabled-label-text-opacity: 1;
21-54: Consider using SCSS variables or mixins to reduce token repetition.Typography, shape, and padding tokens are duplicated for each button variant with identical values. Extracting these to SCSS variables would improve maintainability.
🔎 Example refactor using SCSS variables
// Define shared tokens once $button-label-text-size: 14px; $button-label-text-weight: 500; $button-label-text-font: 'Roboto', sans-serif; $button-container-shape: 20px; $button-leading-space: 24px; $button-trailing-space: 24px; md-filled-button, md-outlined-button, md-elevated-button, md-text-button, md-filled-tonal-button { // Typography - apply to all variants --md-filled-button-label-text-size: #{$button-label-text-size}; --md-filled-button-label-text-weight: #{$button-label-text-weight}; // ... etc }src/app/shared/server-selector/server-selector.component.scss (1)
27-61: Notched outline selectors are repeated multiple times.The
.mdc-notched-outline__leading,.mdc-notched-outline__notch,.mdc-notched-outline__trailingselector chain appears three times in the outlined variant (normal, hover, focus states) and again in dark theme. Consider a mixin or shared selector group.🔎 Suggested refactor using SCSS mixin
@mixin notched-outline-border($color, $width: 1px) { .mdc-notched-outline { .mdc-notched-outline__leading, .mdc-notched-outline__notch, .mdc-notched-outline__trailing { border-color: $color; border-width: $width; } } } // Usage: &.outlined-variant { @include notched-outline-border(var(--md-sys-color-outline, #79747e)); &:hover:not(.mat-form-field-disabled) { @include notched-outline-border(var(--md-sys-color-on-surface, #1a1c1e)); } &.mat-focused { @include notched-outline-border(var(--md-sys-color-primary, #1074b9), 2px); } }src/app/login/login.component.html (1)
60-65: Simplify conditional logo rendering with@else.Two separate
@ifblocks with opposite conditions can be consolidated using@elsefor cleaner syntax.🔎 Proposed fix
@if (!themeDarkEnabled) { <img src="assets/images/mifos_lg-logo.jpg" alt="{{ 'APP_NAME' | translate }} Logo" class="logo-image" /> - } - @if (themeDarkEnabled) { + } @else { <img src="assets/images/white-mifos.png" alt="{{ 'APP_NAME' | translate }} Logo" class="logo-image" /> }src/app/shared/m3-ui/m3-icon/m3-icon.component.ts (1)
54-54: Consider using Angular'srequiredinput or adding runtime validation.The
nameinput uses definite assignment assertion (!) but if the component is used without providing aname, it will render an empty/invalid icon. In Angular 16+, you can use@Input({ required: true })to get compile-time checking in templates.🔎 Proposed fix
- @Input() name!: string; + @Input({ required: true }) name!: string;src/app/login/login.component.scss (1)
25-27: Consider using CSS custom properties for gradient colors.The hero overlay uses hardcoded hex colors with alpha. For consistency with the MD3 theming approach used elsewhere in this file, consider defining these as CSS custom properties.
src/app/login/login.component.ts (1)
80-82: Thevalueparameter is unused in the theme subscription callback.The subscription receives
value: stringbut only reads fromsettingsService.themeDarkEnabled. Either use thevalueparameter or remove it to avoid confusion.🔎 Proposed fix
- this.theme$ = this.themingService.theme.subscribe((value: string) => { + this.theme$ = this.themingService.theme.subscribe(() => { this.themeDarkEnabled = this.settingsService.themeDarkEnabled; });src/app/shared/m3-ui/m3-button/m3-button.component.ts (1)
150-152: Container classes logic replaces base class instead of adding a modifier.Following BEM conventions,
--full-widthshould be a modifier added alongside the base class, not a replacement. This ensures base styles always apply.🔎 Proposed fix
get containerClasses(): string { - return this.fullWidth ? 'm3-button-container--full-width' : 'm3-button-container'; + return this.fullWidth ? 'm3-button-container m3-button-container--full-width' : 'm3-button-container'; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.jsonand included by**/*
📒 Files selected for processing (46)
.stylelintrc(2 hunks)package.json(1 hunks)src/app/login/login-form/login-form.component.html(2 hunks)src/app/login/login-form/login-form.component.scss(1 hunks)src/app/login/login-form/login-form.component.ts(2 hunks)src/app/login/login.component.html(1 hunks)src/app/login/login.component.scss(1 hunks)src/app/login/login.component.ts(5 hunks)src/app/shared/footer/footer.component.html(1 hunks)src/app/shared/footer/footer.component.ts(0 hunks)src/app/shared/icons.module.ts(6 hunks)src/app/shared/language-selector/language-selector.component.html(1 hunks)src/app/shared/language-selector/language-selector.component.scss(1 hunks)src/app/shared/language-selector/language-selector.component.ts(2 hunks)src/app/shared/m3-ui/index.ts(1 hunks)src/app/shared/m3-ui/m3-button/m3-button.component.html(1 hunks)src/app/shared/m3-ui/m3-button/m3-button.component.scss(1 hunks)src/app/shared/m3-ui/m3-button/m3-button.component.ts(1 hunks)src/app/shared/m3-ui/m3-icon/m3-icon.component.html(1 hunks)src/app/shared/m3-ui/m3-icon/m3-icon.component.scss(1 hunks)src/app/shared/m3-ui/m3-icon/m3-icon.component.ts(1 hunks)src/app/shared/server-selector/server-selector.component.html(1 hunks)src/app/shared/server-selector/server-selector.component.scss(2 hunks)src/app/shared/server-selector/server-selector.component.ts(2 hunks)src/app/shared/tenant-selector/tenant-selector.component.html(2 hunks)src/app/shared/tenant-selector/tenant-selector.component.scss(1 hunks)src/app/shared/theme-toggle/theme-toggle.component.html(1 hunks)src/app/shared/theme-toggle/theme-toggle.component.ts(3 hunks)src/app/shared/theme-toggle/theming.service.ts(1 hunks)src/app/standalone-shared.module.ts(2 hunks)src/assets/translations/cs-CS.json(2 hunks)src/assets/translations/de-DE.json(2 hunks)src/assets/translations/en-US.json(3 hunks)src/assets/translations/es-CL.json(2 hunks)src/assets/translations/es-MX.json(2 hunks)src/assets/translations/fr-FR.json(2 hunks)src/assets/translations/it-IT.json(2 hunks)src/assets/translations/ko-KO.json(2 hunks)src/assets/translations/lt-LT.json(2 hunks)src/assets/translations/lv-LV.json(2 hunks)src/assets/translations/ne-NE.json(2 hunks)src/assets/translations/pt-PT.json(2 hunks)src/assets/translations/sw-SW.json(2 hunks)src/index.html(1 hunks)src/main.scss(2 hunks)src/theme/_material-web.scss(1 hunks)
💤 Files with no reviewable changes (1)
- src/app/shared/footer/footer.component.ts
🧰 Additional context used
📓 Path-based instructions (1)
src/app/**
⚙️ CodeRabbit configuration file
src/app/**: For Angular code: verify component separation, trackBy on *ngFor,
strict type safety, and clean observable patterns.
Files:
src/app/login/login-form/login-form.component.scsssrc/app/shared/language-selector/language-selector.component.scsssrc/app/shared/m3-ui/m3-icon/m3-icon.component.scsssrc/app/login/login-form/login-form.component.tssrc/app/shared/server-selector/server-selector.component.scsssrc/app/shared/theme-toggle/theming.service.tssrc/app/shared/tenant-selector/tenant-selector.component.scsssrc/app/shared/language-selector/language-selector.component.htmlsrc/app/login/login.component.scsssrc/app/shared/server-selector/server-selector.component.htmlsrc/app/shared/theme-toggle/theme-toggle.component.tssrc/app/shared/language-selector/language-selector.component.tssrc/app/login/login.component.htmlsrc/app/standalone-shared.module.tssrc/app/shared/m3-ui/index.tssrc/app/shared/footer/footer.component.htmlsrc/app/shared/tenant-selector/tenant-selector.component.htmlsrc/app/shared/m3-ui/m3-icon/m3-icon.component.tssrc/app/shared/m3-ui/m3-button/m3-button.component.scsssrc/app/shared/m3-ui/m3-button/m3-button.component.tssrc/app/shared/theme-toggle/theme-toggle.component.htmlsrc/app/login/login-form/login-form.component.htmlsrc/app/shared/icons.module.tssrc/app/login/login.component.tssrc/app/shared/m3-ui/m3-button/m3-button.component.htmlsrc/app/shared/m3-ui/m3-icon/m3-icon.component.htmlsrc/app/shared/server-selector/server-selector.component.ts
🧬 Code graph analysis (5)
src/app/login/login-form/login-form.component.ts (1)
src/app/shared/m3-ui/index.ts (1)
M3IconComponent(8-8)
src/app/shared/theme-toggle/theme-toggle.component.ts (1)
src/app/shared/m3-ui/index.ts (1)
M3IconComponent(8-8)
src/app/shared/m3-ui/m3-icon/m3-icon.component.ts (1)
src/app/shared/m3-ui/index.ts (2)
M3IconStyle(8-8)M3IconComponent(8-8)
src/app/shared/m3-ui/m3-button/m3-button.component.ts (2)
src/app/shared/m3-ui/index.ts (2)
M3ButtonVariant(7-7)M3ButtonComponent(7-7)src/app/standalone-shared.module.ts (2)
STANDALONE_SHARED_IMPORTS(22-55)M3ButtonComponent(56-56)
src/app/login/login.component.ts (1)
src/app/shared/m3-ui/index.ts (1)
M3IconComponent(8-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Lint, Build and Deploy
🔇 Additional comments (57)
.stylelintrc (1)
50-51: ✓ Add/^md-/pattern to support Material Design 3 web components.This addition appropriately allows stylelint to recognize Material Design web component selectors using the
md-prefix from@material/web.src/assets/translations/lt-LT.json (3)
1195-1198: New M3 login messaging translations added correctly.Lines 1195–1198 introduce three new heading entries for Material Design 3 login page features ("Secure & Reliable", "Community Driven", "Global Impact"). Translations appear accurate and consistent with existing Lithuanian terminology.
3747-3750: Newplaceholdertop-level section introduced for form field hints.Lines 3747–3750 add a new top-level object
"placeholder"containing Lithuanian translations for username and password input placeholders. This extends the i18n API surface and aligns with form field improvements in the M3 redesign.Action item: Verify that consuming components (login form, etc.) reference
placeholder.Enter your usernameandplaceholder.Enter your passwordpaths, or this new structure may remain unused.
904-904: Potential inconsistency in AI summary: "Create Savings Product" still present.The AI-generated summary claims that "Create Savings Product" was removed, but line 904 shows the key and its Lithuanian translation remain in the
headingsection:"Create Savings Product": "Sukurkite taupymo produktą",If this key was intentionally removed from another location or deprecated, please clarify the scope of that change. If the summary is inaccurate, no action is needed—the current presence is correct.
src/assets/translations/ne-NE.json (2)
1195-1198: Verify translation accuracy for Material Design 3 login taglinesThe additions under the
headingsection look properly formatted, but the translations would benefit from review by a native Nepali speaker to ensure they accurately convey the intended meaning of "Secure & Reliable," "Community Driven," and "Global Impact" in the context of a financial services platform.
3747-3750: Placeholder strings added for login formThe new
placeholdersection at root level is correctly positioned and follows JSON conventions. The login form placeholder translations ("Enter your username" / "Enter your password") appear to be accurate and appropriately placed for the Material Design 3 implementation.package.json (1)
50-50: No changes required — @material/web 2.4.1 is valid and current.Version 2.4.1 is the latest release, published 23 days ago. The dependency is correctly placed in "dependencies" as a runtime library supporting MD3 components. No active CVE issues are associated with this package. The caret version specifier allows for safe minor and patch updates.
src/assets/translations/es-MX.json (2)
1194-1197: LGTM! Login page translations are accurate.The new heading translations for the Material Design 3 login interface are grammatically correct and follow proper Spanish conventions. The marketing messages ("Secure & Reliable", "Community Driven", "Global Impact") are well-translated and appropriate for the context.
3748-3752: LGTM! Placeholder translations are well-structured.The new placeholder object provides proper Spanish translations for the login form inputs. The translations are grammatically correct, use appropriate imperative mood ("Ingrese"), and follow consistent patterns with existing translations in the file.
src/assets/translations/es-CL.json (2)
1194-1197: LGTM! Translation quality is excellent.The four new label entries are accurately translated to Spanish (Chile):
- "Date" → "Fecha"
- "Secure & Reliable" → "Seguro y Confiable"
- "Community Driven" → "Impulsado por la Comunidad"
- "Global Impact" → "Impacto Global"
All translations use proper capitalization and are grammatically correct. These labels align well with the Material Design 3 login interface implementation mentioned in the PR.
3747-3750: LGTM! Login form placeholders are correctly translated.The new placeholder section provides proper Spanish (Chile) translations for login form fields:
- "Enter your username" → "Ingrese su nombre de usuario"
- "Enter your password" → "Ingrese su contraseña"
The translations use the formal "usted" register, which is consistent with the professional tone throughout the application and appropriate for a login interface.
src/assets/translations/it-IT.json (2)
1195-1198: New marketing headings read well and fit existing sectionThe added labels
"Date","Secure & Reliable","Community Driven", and"Global Impact"are correctly translated and sit logically with the surrounding amortization-related headings. No JSON or i18n issues spotted here.
3746-3749: Top-levelplaceholderblock and JSON structure look correctThe new
"placeholder"section at the root is well-formed JSON, uses natural Italian placeholder text, and the added comma after the"countries"block keeps the overall structure valid. This aligns with how placeholders are modeled elsewhere in the file.src/assets/translations/en-US.json (1)
3856-3859: LGTM! Placeholder section is well-structured.The new "placeholder" section with username and password entries is properly organized and follows common internationalization patterns. The placement and structure align well with the Material Design 3 login implementation.
src/assets/translations/lv-LV.json (1)
3747-3750: LGTM! Well-structured placeholder translations.The new placeholder section is properly structured and follows i18n best practices. The Latvian translations use the consistent "Ievadiet" (Enter) pattern and are appropriately placed at the root level of the translation object.
src/assets/translations/ko-KO.json (2)
1197-1199: LGTM! Accurate translations for feature highlights.The Korean translations for these three new entries are accurate and appropriate:
- "Secure & Reliable" → "안전하고 신뢰할 수 있음" uses natural Korean phrasing
- "Community Driven" → "커뮤니티 주도형" correctly conveys the concept
- "Global Impact" → "글로벌 영향" follows standard terminology
These appear to be feature highlights for the login/welcome interface and are well-translated.
3747-3750: LGTM! Well-structured placeholder translations.The new
placeholderblock is properly structured and contains accurate Korean translations for the login form:
- "Enter your username" → "사용자 이름을 입력하세요"
- "Enter your password" → "비밀번호를 입력하세요"
Both translations use the appropriate polite form (존댓말) which is standard for user-facing UI elements in Korean applications.
src/assets/translations/de-DE.json (2)
1196-1198: New feature translations look good.The German translations for the three new UI strings are grammatically correct and appropriate:
- "Sicher und zuverlässig" (Secure & Reliable)
- "Gemeinschaftsorientiert" (Community Driven)
- "Globale Auswirkungen" (Global Impact)
3744-3747: Placeholder translations are correctly implemented.The new
placeholdersection with German translations for username and password prompts is properly formatted and the translations are accurate:
- "Geben Sie Ihren Benutzernamen ein" = "Enter your username"
- "Geben Sie Ihr Passwort ein" = "Enter your password"
Note: The AI summary incorrectly indicated these were untranslated, but the actual code contains proper German translations.
src/assets/translations/sw-SW.json (1)
1193-1196: LGTM - Feature descriptions added appropriately.The new translation entries for "Secure & Reliable", "Community Driven", and "Global Impact" are well-translated and appear to be feature/benefit descriptions for a login or landing page. The "Date" entry, while duplicated elsewhere in the file (line 1497), is acceptable as translation files often need the same term in different contexts.
src/assets/translations/cs-CS.json (1)
3745-3748: LGTM! Well-structured placeholder section for login form.The new top-level
placeholdersection is appropriately placed and provides proper Czech translations for the login form input placeholders. The translations are grammatically correct:
- "Zadejte své uživatelské jméno" (Enter your username)
- "Zadejte své heslo" (Enter your password)
The JSON structure is valid with proper comma placement and nesting. This aligns well with the PR's Material Design 3 login interface implementation.
src/assets/translations/pt-PT.json (1)
3747-3750: LGTM: Placeholder translations are accurate.The new placeholder section with login form field translations is well-structured and the Portuguese translations are clear and appropriate:
- "Digite seu nome de usuário" correctly translates "Enter your username"
- "Digite sua senha" correctly translates "Enter your password"
These additions align well with the Material Design 3 login interface implementation described in the PR objectives.
src/app/shared/theme-toggle/theming.service.ts (1)
42-50: LGTM! Symmetric class toggling improves theme consistency.The added removal of
light-themewhen enabling dark mode and the addition oflight-themewhen enabling light mode ensures both themes are mutually exclusive on the body element. This prevents potential styling conflicts.src/app/shared/language-selector/language-selector.component.scss (1)
1-131: LGTM! Well-structured MD3 styling with proper theme support.The variant-based styling approach is clean and maintainable:
- Clear separation between fill and outlined variants
- Consistent use of MD3 design tokens (CSS custom properties)
- Proper dark theme overrides with appropriate color adjustments
- Focus and hover states well-defined for accessibility
src/app/login/login-form/login-form.component.scss (1)
1-439: LGTM! Comprehensive MD3 styling with excellent theme and responsive support.The login form styling is well-implemented:
- Clear sectional organization with helpful comments
- Consistent use of MD3 design tokens
- Proper light/dark theme support with appropriate color adjustments
- Thoughtful responsive design for mobile devices
- Smooth animations enhance user experience
- Focus states properly defined for accessibility
src/index.html (1)
17-20: LGTM! Material Symbols font properly loaded for MD3 icons.The font link is correctly configured with appropriate variable font axes for the M3 icon system.
src/app/shared/m3-ui/m3-icon/m3-icon.component.html (1)
1-1: LGTM! Simple and effective icon template.The template correctly leverages Material Symbols by rendering the icon name as text content, with proper class and style bindings from the component.
src/app/shared/theme-toggle/theme-toggle.component.html (1)
1-3: LGTM! Successfully migrated to M3 icon component.The theme toggle correctly uses the new M3 icon component while maintaining the same conditional icon logic. The margin adjustment improves visual spacing.
src/app/login/login-form/login-form.component.ts (1)
11-11: LGTM! Clean migration to M3IconComponent.The replacement of
FaIconComponentwithM3IconComponentaligns well with the Material Design 3 migration objectives. The import and usage are correct.Also applies to: 28-28
src/app/shared/m3-ui/m3-button/m3-button.component.scss (1)
1-19: LGTM! Well-structured M3 button styling.The BEM naming convention and full-width modifier logic are clean and maintainable. All Material Web button variants (
md-filled-button,md-outlined-button,md-elevated-button,md-text-button,md-filled-tonal-button) are properly covered.src/app/shared/theme-toggle/theme-toggle.component.ts (2)
5-5: LGTM! Consistent icon migration to M3IconComponent.The replacement of
MatIconComponentwithM3IconComponentis consistent with the Material Design 3 migration strategy.Also applies to: 15-15
32-44: LGTM! Clear theme toggle implementation.The
toggleTheme()method correctly handles the complete theme switching process: toggling state, persisting preferences, and updating the theming service. The documentation is helpful and the logic is sound.src/app/shared/m3-ui/index.ts (1)
1-8: LGTM! Well-organized barrel export.This public facade follows Angular best practices by centralizing exports for the Material Design 3 UI components, making imports cleaner and more maintainable throughout the application.
src/app/shared/server-selector/server-selector.component.html (1)
2-11: LGTM! Clean MD3 appearance variant implementation.The dynamic appearance binding and conditional label/placeholder logic provide good flexibility for different UI contexts. The pattern aligns well with Material Design 3 principles.
src/app/shared/icons.module.ts (1)
46-46: Question: Is expanding FontAwesome necessary during M3 migration?This change adds numerous FontAwesome icons (
faGlobe,faExternalLinkAlt,faHeart,faBookOpen,faCode,faCodeBranch,faComments,faLightbulb,faHandHoldingHand,faHandHoldingHeart) to the library. However, the PR is actively migrating icon usage toM3IconComponentacross multiple components.Consider whether these new FontAwesome icons are needed for components that haven't been migrated yet, or if they could be replaced with Material Symbols icons from the start to avoid expanding a library that's being phased out.
Are these icons required for components not yet migrated to M3, or can they be replaced with Material Symbols equivalents?
Also applies to: 118-127, 151-267
src/app/shared/tenant-selector/tenant-selector.component.html (1)
1-1:appearance="fill"is correct and requires no changes.Angular Material form fields support two appearance variants:
fillandoutline. The default appearance isfillwhen not specified. The value in the code is already correct for Material Design 3.Likely an incorrect or invalid review comment.
src/app/shared/language-selector/language-selector.component.html (2)
1-10: LGTM! Clean conditional rendering for appearance variants.The implementation correctly binds the
appearanceinput and applies theoutlined-variantclass conditionally. The conditional label and placeholder logic properly handles both display modes - showing either a label or a placeholder based onshowLabel.
11-15: Track expression is acceptable for primitive string values.Using
track languageworks correctly for string primitives. Per Angular guidelines, for more complex objects, consider using a unique identifier property, but for simple string arrays like language codes, this is appropriate.src/app/standalone-shared.module.ts (1)
53-54: Good organization with MD3 component grouping.The comment clearly delineates Material Design 3 components, which will help maintainability as more MD3 components are added.
src/app/shared/language-selector/language-selector.component.ts (1)
28-32: LGTM! Well-typed inputs with sensible defaults.The new
appearanceandshowLabelinputs are properly typed with union type constraints and appropriate defaults. The JSDoc comments provide clear documentation. This pattern is consistent with the server-selector component changes.src/app/shared/server-selector/server-selector.component.ts (1)
30-34: LGTM! Consistent with language-selector component pattern.The
appearanceandshowLabelinputs mirror the implementation inLanguageSelectorComponent, maintaining consistency across selector components. Well-typed with appropriate defaults.src/app/shared/footer/footer.component.html (1)
1-16: LGTM! Clean template structure with modern control flow.The template correctly uses Angular's new
@ifcontrol flow syntax. The conditional rendering for business date and the footer structure are well-organized. Good separation between the backend info section and the always-visible copy label.Also applies to: 25-39
src/app/login/login-form/login-form.component.html (1)
64-75: LGTM: M3 button implementation with loading state.The submit button properly handles disabled state based on form validity and loading status. The conditional spinner inside the button provides good visual feedback during submission.
src/theme/_material-web.scss (1)
1-180: LGTM: Comprehensive MD3 button token definitions.The light theme baseline provides thorough coverage of all button variants with proper state-layer tokens for hover, focus, pressed, and disabled states. The structure follows Material Design 3 specifications well.
src/app/shared/server-selector/server-selector.component.scss (1)
1-133: LGTM: Well-structured variant and theme support.The SCSS properly separates base styles, outlined variant, and dark theme overrides. CSS variables with sensible fallbacks ensure graceful degradation.
src/app/shared/m3-ui/m3-icon/m3-icon.component.scss (1)
1-69: LGTM: Material Symbols font configuration is correct.The icon styles correctly use
font-variation-settingsto differentiate between filled (FILL: 1) and outlined (FILL: 0) variants while sharing the same font-family. Rounded and sharp variants properly specify their distinct font-families. The base.m3-iconclass provides consistent alignment and smooth rendering.src/app/login/login.component.html (2)
14-36: Inconsistent icon usage: fa-icon vs mifosx-m3-icon.The hero section and resource menus use
fa-icon(FontAwesome) while other UI elements use the newmifosx-m3-icon. If this is intentional (e.g., FontAwesome for decorative/branding icons, M3 for functional UI), consider adding a comment to document this decision. Otherwise, consider migrating to consistent M3 icon usage throughout.
1-123: LGTM: Well-structured MD3 login layout.The login page is well-organized with clear separation between hero panel and login panel. Conditional rendering for tenant selector, reset password, and two-factor authentication flows is properly implemented. Header controls use the new MD3-styled components with appropriate appearance variants.
src/app/shared/m3-ui/m3-icon/m3-icon.component.ts (2)
80-83: LGTM!The
iconClassgetter correctly handles thefilledflag as an override for backward compatibility while still supporting the full range of style variants.
88-94: LGTM!The inline styles approach is appropriate for dynamic icon sizing, and the object structure is correctly typed.
src/app/login/login.component.scss (2)
549-560: LGTM!The responsive design properly hides the hero panel on mobile and adjusts the login panel to take full viewport height, ensuring a good mobile experience.
158-174: Note:::ng-deepis deprecated but acceptable here.While
::ng-deepis deprecated, it remains the practical solution for styling encapsulated child components in Angular. The usage here is appropriate. When migrating to Angular's future styling solutions, these blocks will need updates.src/app/login/login.component.ts (1)
77-101: LGTM!The theme initialization and alert subscription setup is well-structured. The component correctly initializes the dark mode based on stored settings and subscribes to ongoing theme changes.
src/app/shared/m3-ui/m3-button/m3-button.component.ts (2)
130-145: LGTM!The variant-to-tag mapping is complete and correctly maps all button variants to their corresponding Material Web Components.
121-125: LGTM!The click handler correctly guards against disabled state before emitting events.
src/assets/translations/fr-FR.json (2)
1195-1198: New login hero/heading translations are accurate and consistentThe added French strings for “Secure & Reliable”, “Community Driven”, and “Global Impact” are idiomatic and match the style of surrounding heading entries; no changes needed here.
3745-3749: Structure is intentionally correct—no consolidation neededThe two namespaces serve distinct purposes:
- Top-level
placeholder: Login form–specific full instructions ("Enter your username", "Enter your password")labels.placeholders: Generic UI placeholders for other components ("Add new server", "Search Activity")Consistency across all 14 locale files is confirmed with matching structure. No refactoring to consolidate under
labels.placeholdersis needed—the semantic separation is appropriate. The developer properly maintained parallel translation updates across languages, which indicates the UI code integration is likely correct.
| (click)="togglePasswordVisibility()" | ||
| [attr.aria-label]="passwordInputType === 'password' ? 'Show password' : 'Hide password'" | ||
| > | ||
| @if (passwordInputType === 'password') { | ||
| <fa-icon icon="eye"></fa-icon> | ||
| <mifosx-m3-icon name="visibility"></mifosx-m3-icon> | ||
| } | ||
| @if (passwordInputType === 'text') { | ||
| <fa-icon icon="eye-slash"></fa-icon> | ||
| <mifosx-m3-icon name="visibility_off"></mifosx-m3-icon> | ||
| } |
There was a problem hiding this comment.
Hardcoded aria-label strings should be translated for i18n compliance.
The aria-label attribute on line 40 uses hardcoded English strings. For accessibility in a multi-language app, these should be translated.
Additionally, the two @if blocks for visibility icons can be simplified with @else.
🔎 Proposed fix
<button
type="button"
matSuffix
mat-icon-button
(click)="togglePasswordVisibility()"
- [attr.aria-label]="passwordInputType === 'password' ? 'Show password' : 'Hide password'"
+ [attr.aria-label]="(passwordInputType === 'password' ? 'labels.buttons.Show password' : 'labels.buttons.Hide password') | translate"
>
- @if (passwordInputType === 'password') {
+ @if (passwordInputType === 'password') {
<mifosx-m3-icon name="visibility"></mifosx-m3-icon>
- }
- @if (passwordInputType === 'text') {
+ } @else {
<mifosx-m3-icon name="visibility_off"></mifosx-m3-icon>
}
</button>🤖 Prompt for AI Agents
In src/app/login/login-form/login-form.component.html around lines 39 to 47, the
aria-label currently uses hardcoded English strings and the two visibility icon
@if blocks are redundant; replace the literal aria-label values with the app's
translation mechanism (e.g., use the translate pipe or i18n key such as
[attr.aria-label]="passwordInputType === 'password' ? ('login.showPassword' |
translate) : ('login.hidePassword' | translate)" or add an i18n attribute), and
collapse the conditional icon markup into an @if/@else so only one conditional
branch is used (show visibility when password, else show visibility_off). Ensure
keys exist in the translation files.
| <div class="login-actions"> | ||
| <a href="#" class="forgot-password-link"> | ||
| {{ 'labels.links.Forgot Password?' | translate }} | ||
| </a> | ||
| </div> |
There was a problem hiding this comment.
Empty href="#" causes page scroll and is not accessible.
Using href="#" scrolls to the top of the page when clicked and doesn't represent a proper navigation target. If "Forgot Password" functionality is not implemented yet, consider using a <button> styled as a link or routerLink for proper Angular navigation.
🔎 Proposed fix using button
<!-- Forgot Password Link -->
<div class="login-actions">
- <a href="#" class="forgot-password-link">
+ <button type="button" class="forgot-password-link" (click)="onForgotPassword()">
{{ 'labels.links.Forgot Password?' | translate }}
- </a>
+ </button>
</div>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/app/login/login-form/login-form.component.html around lines 78-82, the
anchor uses href="#" which causes page scroll and is not accessible; replace it
with either an Angular routerLink to the actual forgot-password route (e.g. <a
[routerLink]="/forgot-password" class="forgot-password-link">...) if a route
exists, or convert it to a semantic button styled as a link: <button
type="button" class="forgot-password-link" (click)="onForgotPassword()">... to
avoid navigation side-effects; ensure the component implements the
onForgotPassword handler (or navigates programmatically via Router) and maintain
the same CSS class for styling and proper accessibility attributes (aria-label
if needed).
| .logo-image { | ||
| max-width: 20rem; | ||
| padding: 0; | ||
| margin: 0; | ||
| content: url('../../assets/images/white-mifos.png'); | ||
| } |
There was a problem hiding this comment.
Logo swap via content property may not work consistently across browsers.
Using content on an <img> element to swap the source is non-standard behavior. While it works in some browsers, it's not reliable. Consider using a conditional [src] binding in the template or a CSS background-image approach.
🤖 Prompt for AI Agents
In src/app/login/login.component.scss around lines 476 to 481, the .logo-image
rule uses the non-standard content: url(...) to swap an image which is
unreliable across browsers; instead update the component to set the image src
via the template (e.g., use Angular [src] binding to choose the correct asset)
or change the CSS to use background-image on a non-<img> element and remove the
content property; ensure the template and styles consistently reference the
chosen asset path and remove the content declaration so browsers use a standard
image source mechanism.
| imports: [ | ||
| STANDALONE_SHARED_IMPORTS, | ||
| CommonModule | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the m3-button component and standalone-shared files
find src/app -name "*standalone*" -type f
find src/app -path "*/m3-button*" -type fRepository: openMF/web-app
Length of output: 259
🏁 Script executed:
#!/bin/bash
# Read the m3-button component to see the imports section
echo "=== M3 Button Component ==="
cat -n src/app/shared/m3-ui/m3-button/m3-button.component.ts | head -80
echo -e "\n=== Standalone Shared Module ==="
cat -n src/app/standalone-shared.module.tsRepository: openMF/web-app
Length of output: 5422
Remove M3ButtonComponent from STANDALONE_SHARED_IMPORTS to fix circular dependency and eliminate redundant CommonModule import.
The circular dependency is confirmed: M3ButtonComponent imports STANDALONE_SHARED_IMPORTS (line 66), which re-exports M3ButtonComponent itself (line 54 of standalone-shared.module.ts). This can cause runtime errors. Additionally, CommonModule is already included in STANDALONE_SHARED_IMPORTS (line 23), making the explicit import on line 67 redundant.
Remove M3ButtonComponent from STANDALONE_SHARED_IMPORTS array, then remove the explicit CommonModule import from this component since it's already available via STANDALONE_SHARED_IMPORTS.
🤖 Prompt for AI Agents
In src/app/shared/m3-ui/m3-button/m3-button.component.ts around lines 65 to 68,
the component imports STANDALONE_SHARED_IMPORTS which re-exports
M3ButtonComponent (creating a circular dependency) and also redundantly imports
CommonModule; to fix this remove M3ButtonComponent from the
STANDALONE_SHARED_IMPORTS array in standalone-shared.module.ts (so the shared
barrel no longer re-exports the button) and then delete the explicit
CommonModule import from this component's imports array so it only consumes
STANDALONE_SHARED_IMPORTS without creating a cycle or duplicate module import.
| </mat-option> | ||
| } | ||
| </mat-select> | ||
| <fa-icon matIconPrefix icon="building"></fa-icon> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Inconsistent icon component usage—consider using M3IconComponent.
While other components in this PR are migrating from FontAwesome to M3IconComponent (e.g., login-form.component.ts, theme-toggle.component.ts), this template still uses <fa-icon>. For consistency with the Material Design 3 migration and to reduce bundle size, consider migrating this icon to M3IconComponent as well.
Suggested refactor
Replace the FontAwesome icon with the M3 icon component:
- <fa-icon matIconPrefix icon="building"></fa-icon>
+ <mifosx-m3-icon matIconPrefix name="apartment" />Note: You'll also need to import M3IconComponent in the corresponding TypeScript file.
📝 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.
| <fa-icon matIconPrefix icon="building"></fa-icon> | |
| <mifosx-m3-icon matIconPrefix name="apartment" /> |
🤖 Prompt for AI Agents
In src/app/shared/tenant-selector/tenant-selector.component.html around line 10,
the template uses the FontAwesome <fa-icon> component which is inconsistent with
the PR's migration to Material3 icons; replace the <fa-icon matIconPrefix
icon="building"> with the equivalent M3IconComponent usage (e.g., use the
project's M3 icon selector/component and the matching "building" or appropriate
material icon name) and update any attributes to match M3IconComponent's API;
then import and declare M3IconComponent in tenant-selector.component.ts (add the
import and include it in the component's standalone/imports or module
declarations as used elsewhere in the PR) so the template compiles and bundle
usage is consistent.
| "Adjusted Amount": "Angepasster Betrag", | ||
| "Charged Off Amount": "Abgeschriebener Betrag", | ||
| "Date": "Datum" | ||
| "Date": "Datum", |
There was a problem hiding this comment.
Duplicate translation key "Date" detected.
The key "Date" is added here but already exists at line 1499 with the same translation value "Datum". In JSON objects, duplicate keys can cause undefined behavior—typically the last occurrence wins, but some parsers may error. This duplication should be removed.
🔎 Proposed fix
Remove the duplicate entry at line 1195:
"Not Yet Amortized Amount": "Noch nicht amortisierter Betrag",
"Adjusted Amount": "Angepasster Betrag",
"Charged Off Amount": "Abgeschriebener Betrag",
- "Date": "Datum",
"Secure & Reliable": "Sicher und zuverlässig",
"Community Driven": "Gemeinschaftsorientiert",
"Global Impact": "Globale Auswirkungen"The existing "Date": "Datum" entry at line 1499 should be retained.
📝 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.
| "Date": "Datum", |
🤖 Prompt for AI Agents
In src/assets/translations/de-DE.json around line 1195, there is a duplicate
"Date": "Datum" key that also exists at line 1499; remove the duplicate entry at
line 1195 and adjust surrounding commas so the JSON stays valid, keeping the
original entry at line 1499.
| "Secure & Reliable": "Seguro e confiável", | ||
| "Community Driven": "Impulsionado pela comunidade", | ||
| "Global Impact": "Impacto global" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the pt-PT.json file
fd -e json pt-PT . && head -20 src/assets/translations/pt-PT.jsonRepository: openMF/web-app
Length of output: 1513
🏁 Script executed:
# Check if pt-PT.json exists and examine lines around 1196
if [ -f "src/assets/translations/pt-PT.json" ]; then
echo "=== File exists ==="
wc -l src/assets/translations/pt-PT.json
echo ""
echo "=== Lines 1190-1200 ==="
sed -n '1190,1200p' src/assets/translations/pt-PT.json
echo ""
echo "=== Checking for 'confiável' in file ==="
grep -n "confiável\|fiável" src/assets/translations/pt-PT.json | head -20
else
echo "File not found"
fiRepository: openMF/web-app
Length of output: 665
Use European Portuguese spelling to match pt-PT.json locale.
While the written language is very similar, there are some minor spelling differences between the two Portuguese variants. If you are trying to create a "universal" Portuguese translation for both markets, this can be a problem. For example, the words "receção" (reception) and "facto" (fact or reality) would be completely normal for a reader in Portugal. But for a Brazilian they would seem jarring where the words are written "recepção" and "fato."
At line 1196, "confiável" (Brazilian Portuguese) should be "fiável" (European Portuguese) to match the pt-PT.json file designation. Targeting Brazil: Use Brazilian Portuguese translation for web, ads, UI, and legal content. Targeting Portugal: Use European Portuguese translation with regional spelling and vocabulary. This ensures proper user experience for European Portuguese speakers.
🤖 Prompt for AI Agents
In src/assets/translations/pt-PT.json around lines 1196 to 1198, the Portuguese
variant used is Brazilian spelling ("confiável"); replace that value with the
European Portuguese spelling "fiável" so the entry reads "Secure & Reliable":
"Seguro e fiável" (keeping the other entries unchanged) to match the pt-PT
locale.
src/assets/translations/sw-SW.json
Outdated
| "placeholder": { | ||
| "Enter your username": "Ingiza jina lako la mtumiaji", | ||
| "Enter your password": "Ingiza nenosiri lako" | ||
| } |
There was a problem hiding this comment.
Consolidate into existing "placeholders" section to avoid duplication.
A new root-level placeholder (singular) object has been created, but a placeholders (plural) section already exists at lines 2670-2673. This creates structural inconsistency in the translation file:
// Existing section (lines 2670-2673)
"placeholders": {
"Add new server": "Ongeza seva mpya",
"Search Activity": "Shughuli ya Utafutaji"
}
// New section (lines 3745-3748) - DUPLICATE STRUCTURE
"placeholder": {
"Enter your username": "Ingiza jina lako la mtumiaji",
"Enter your password": "Ingiza nenosiri lako"
}Issues:
- Inconsistent naming:
placeholdervsplaceholders - Application code likely references only one section
- Increases maintenance burden and potential for confusion
Recommendation:
Merge the new entries into the existing placeholders section and remove the duplicate structure.
🔎 Proposed fix
Remove lines 3744-3748 (including the comma on line 3744) and add the translations to the existing placeholders section:
"placeholders": {
"Add new server": "Ongeza seva mpya",
- "Search Activity": "Shughuli ya Utafutaji"
+ "Search Activity": "Shughuli ya Utafutaji",
+ "Enter your username": "Ingiza jina lako la mtumiaji",
+ "Enter your password": "Ingiza nenosiri lako"
},And remove the duplicate section:
"ZW": "Zimbabwe"
- },
- "placeholder": {
- "Enter your username": "Ingiza jina lako la mtumiaji",
- "Enter your password": "Ingiza nenosiri lako"
}
}🤖 Prompt for AI Agents
In src/assets/translations/sw-SW.json around lines 3745-3748, there is a new
root-level "placeholder" object that duplicates and conflicts with the existing
"placeholders" section (lines ~2670-2673); remove the entire "placeholder"
object (including any leading comma on the previous line) and merge its
key/value pairs ("Enter your username": "Ingiza jina lako la mtumiaji", "Enter
your password": "Ingiza nenosiri lako") into the existing "placeholders" object,
ensuring keys are added without introducing trailing comma issues and preserving
JSON validity.
ca1e024 to
c3e765f
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
src/assets/translations/pt-PT.json (1)
1196-1196: Use European Portuguese spelling "fiável" instead of Brazilian "confiável".This issue was previously flagged but remains unresolved. The translation uses "confiável" which is Brazilian Portuguese, but this file targets European Portuguese (pt-PT). The correct spelling for European Portuguese is "fiável".
🔎 Proposed fix
- "Secure & Reliable": "Seguro e confiável", + "Secure & Reliable": "Seguro e fiável",Based on past review comments, this spelling inconsistency affects the user experience for European Portuguese speakers.
src/app/shared/tenant-selector/tenant-selector.component.html (1)
10-10: Inconsistent icon component usage—consider migrating to M3IconComponent.This component continues to use FontAwesome's
<fa-icon>while other components in the PR (e.g.,login-form.component.ts) have migrated toM3IconComponent. For consistency with the Material Design 3 implementation and to reduce bundle size, consider updating this icon as well.src/app/login/login-form/login-form.component.html (2)
39-47: Hardcoded aria-label should use translated strings.The password visibility toggle button uses hardcoded English strings in the aria-label attribute, which breaks internationalization. This was previously identified and should be addressed.
As noted in the previous review, use the translation pipe:
[attr.aria-label]="(passwordInputType === 'password' ? 'labels.buttons.Show password' : 'labels.buttons.Hide password') | translate"
77-81: Replace href="#" with proper navigation or button element.The "Forgot Password" link uses
href="#", which causes unwanted page scrolling and is not accessible. This issue was previously identified and remains unresolved.Consider implementing one of the solutions from the previous review:
- Use a
<button>element styled as a link with a click handler- Use Angular
routerLinkif a route exists- Use
href="javascript:void(0)"as a last resortsrc/app/login/login.component.scss (1)
476-481: Remove unnecessary content property for logo swap.The
.logo-imagerule uses the non-standardcontent: url()to swap the logo in dark theme. However, the template (login.component.htmllines 60-66) already conditionally renders the correct logo using@ifdirectives based onthemeDarkEnabled. This CSS rule is redundant and could cause conflicts.🔎 Proposed fix
Remove the content property and just keep the sizing/spacing adjustments:
.logo-image { max-width: 20rem; padding: 0; margin: 0; - content: url('../../assets/images/white-mifos.png'); }The template already handles logo selection correctly.
🧹 Nitpick comments (7)
src/assets/translations/ko-KO.json (1)
1197-1199: Consider having the Korean translations reviewed by a native speaker.While the JSON structure is correct and the keys are appropriately placed, the accuracy of the Korean translations should ideally be verified by a Korean language expert to ensure:
- Correct grammar and natural phrasing
- Appropriate formality level for a banking/financial application
- Consistency with existing translation style in the file
Also applies to: 2675-2676
src/app/shared/language-selector/language-selector.component.scss (1)
13-81: Excellent Material Design 3 implementation with proper accessibility.The outlined variant styling is comprehensive and correct:
- Proper touch target size (48px min-height on line 25)
- Focus and hover states are well-defined with appropriate visual feedback
- Border width transitions (1px → 2px on focus) follow Material Design 3 patterns
- CSS custom properties with fallback values ensure compatibility
Optional: Extract magic numbers to SCSS variables
For improved maintainability, consider extracting repeated values to SCSS variables:
$language-selector-fill-margin: 10px; $language-selector-outlined-padding: 12px; $language-selector-min-height: 48px; $language-selector-border-width: 1px; $language-selector-border-width-focused: 2px; $language-selector-font-size: 0.875rem;This would make future adjustments easier and ensure consistency.
src/app/shared/icons.module.ts (1)
117-127: Consider maintaining alphabetical import order.The new icon imports at the end break the alphabetical ordering established throughout the rest of the file (lines 6-116). This makes the import list harder to scan and maintain.
🔎 Suggested ordering for consistency
Move the imports to their alphabetically correct positions:
faFileExcel, faFileExport, faFillDrip, faGlobe, faHandHoldingUsd, faHome, faKey, faLink, faList, faListUl, faLocationArrow, faLock, faLockOpen, faMinusCircle, faMinus, faMoneyBillAlt, faMoneyCheck, faPencilAlt, faPlay, faPlus, faPlusCircle, faQuestionCircle, faRandom, faRoad, faSearch, faShieldAlt, faSignOutAlt, faSitemap, faSync, faTable, faTachometerAlt, faTag, faTags, faTimesCircle, faTrash, faUndo, faUniversity, faUserCircle, faUser, faUsers, faUserShield, faUserTie, faDownload, faUpload, faCloudDownloadAlt, faCloudUploadAlt, faTimes, faStickyNote, faStop, faBan, faBolt, faLaptop, faDonate, faMoneyBill, faChartLine, faPen, faFlag, faArrowUp, faArrowDown, faFolderOpen, faIdBadge, faAddressCard, faKeyboard, faInfo, faTasks, faCoins, faPiggyBank, faFileExcel, faFileExport, faCalculator, faCalendarCheck, faPause, faReceipt, faTableCells, - faSave, - faExternalLinkAlt, - faHeart, - faBookOpen, - faCode, - faLaptopCode, - faComments, - faLightbulb, - faCodeBranch, - faHandHoldingHand, - faHandHoldingHeart + faSaveThen insert each icon in its correct alphabetical position earlier in the import list.
src/main.scss (1)
28-85: Consider using CSS custom properties for semi-transparent color values.The MD3 select panel styling uses hardcoded semi-transparent colors (e.g.,
#1074b914,#1074b91f,#5ba2ec14,#5ba2ec1f) for hover and active states. For better maintainability and consistency with the MD3 design system, consider defining these as CSS custom properties or using modern CSS color functions likergb()with alpha channels.🔎 Example approach using CSS variables
Define reusable alpha variants in your theme files:
// In theme files :root { --md-sys-color-primary-hover: rgba(16, 116, 185, 0.08); --md-sys-color-primary-active: rgba(16, 116, 185, 0.12); } .dark-theme { --md-sys-color-primary-hover: rgba(91, 162, 236, 0.08); --md-sys-color-primary-active: rgba(91, 162, 236, 0.12); }Then use them consistently:
&:hover { - background-color: #1074b914; + background-color: var(--md-sys-color-primary-hover); }src/app/login/login-form/login-form.component.scss (1)
18-77: Consider reducing reliance on ::ng-deep for better encapsulation.The tenant selector styling extensively uses
::ng-deepto pierce component encapsulation. While this works for styling Material components,::ng-deepis deprecated and can make styles brittle when components change. Consider these alternatives:
- Use component
appearanceinputs where available- Move shared field styles to a global theme file
- Use CSS custom properties that child components can reference
src/app/login/login.component.html (1)
60-66: Use @else for mutually exclusive logo display.The logo section uses two separate
@ifblocks for theme-based logo selection. Since these conditions are mutually exclusive, using@elsewould be clearer and more idiomatic.🔎 Proposed refactor
- @if (!themeDarkEnabled) { <img src="assets/images/mifos_lg-logo.jpg" alt="{{ 'APP_NAME' | translate }} Logo" class="logo-image" /> - } - @if (themeDarkEnabled) { + } @else { <img src="assets/images/white-mifos.png" alt="{{ 'APP_NAME' | translate }} Logo" class="logo-image" /> }src/app/login/login.component.ts (1)
107-114: Consider using takeUntil pattern for cleaner subscription management.The manual subscription cleanup is now properly implemented with null checks. However, for cleaner observable patterns (as per coding guidelines), consider using the
takeUntilpattern with a destroy subject, which automatically unsubscribes when the component is destroyed.🔎 Proposed refactor using takeUntil
import { Subject, takeUntil } from 'rxjs'; export class LoginComponent implements OnInit, OnDestroy { private destroy$ = new Subject<void>(); ngOnInit() { this.themeDarkEnabled = this.settingsService.themeDarkEnabled; this.themingService.theme .pipe(takeUntil(this.destroy$)) .subscribe((value: string) => { this.themeDarkEnabled = this.settingsService.themeDarkEnabled; }); this.themingService.setDarkMode(!!this.settingsService.themeDarkEnabled); this.alertService.alertEvent .pipe(takeUntil(this.destroy$)) .subscribe((alertEvent: Alert) => { // ... existing logic }); } ngOnDestroy() { this.destroy$.next(); this.destroy$.complete(); } }This eliminates the need for individual subscription properties and null checks.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.jsonand included by**/*
📒 Files selected for processing (48)
.stylelintrcpackage.jsonsrc/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/app/login/reset-password/reset-password.component.htmlsrc/app/shared/footer/footer.component.htmlsrc/app/shared/footer/footer.component.tssrc/app/shared/icons.module.tssrc/app/shared/language-selector/language-selector.component.htmlsrc/app/shared/language-selector/language-selector.component.scsssrc/app/shared/language-selector/language-selector.component.tssrc/app/shared/m3-ui/index.tssrc/app/shared/m3-ui/m3-button/m3-button.component.htmlsrc/app/shared/m3-ui/m3-button/m3-button.component.scsssrc/app/shared/m3-ui/m3-button/m3-button.component.tssrc/app/shared/m3-ui/m3-icon/m3-icon.component.htmlsrc/app/shared/m3-ui/m3-icon/m3-icon.component.scsssrc/app/shared/m3-ui/m3-icon/m3-icon.component.tssrc/app/shared/server-selector/server-selector.component.htmlsrc/app/shared/server-selector/server-selector.component.scsssrc/app/shared/server-selector/server-selector.component.tssrc/app/shared/tenant-selector/tenant-selector.component.htmlsrc/app/shared/tenant-selector/tenant-selector.component.scsssrc/app/shared/theme-toggle/theme-toggle.component.htmlsrc/app/shared/theme-toggle/theme-toggle.component.tssrc/app/shared/theme-toggle/theming.service.tssrc/app/standalone-shared.module.tssrc/app/system/external-services/email/edit-email/edit-email.component.htmlsrc/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/index.htmlsrc/main.scsssrc/theme/_material-web.scss
💤 Files with no reviewable changes (1)
- src/app/shared/footer/footer.component.ts
✅ Files skipped from review due to trivial changes (2)
- src/app/shared/tenant-selector/tenant-selector.component.scss
- src/app/shared/m3-ui/m3-icon/m3-icon.component.ts
🚧 Files skipped from review as they are similar to previous changes (23)
- src/app/shared/m3-ui/m3-button/m3-button.component.scss
- src/app/shared/theme-toggle/theme-toggle.component.ts
- src/app/shared/m3-ui/index.ts
- src/assets/translations/ne-NE.json
- src/assets/translations/de-DE.json
- src/app/standalone-shared.module.ts
- src/app/shared/server-selector/server-selector.component.html
- src/app/shared/language-selector/language-selector.component.ts
- src/assets/translations/lt-LT.json
- src/app/shared/m3-ui/m3-button/m3-button.component.html
- src/assets/translations/en-US.json
- .stylelintrc
- src/assets/translations/lv-LV.json
- src/app/shared/theme-toggle/theming.service.ts
- src/app/shared/server-selector/server-selector.component.ts
- src/app/shared/theme-toggle/theme-toggle.component.html
- src/index.html
- src/app/shared/m3-ui/m3-button/m3-button.component.ts
- src/app/shared/m3-ui/m3-icon/m3-icon.component.html
- src/assets/translations/fr-FR.json
- src/assets/translations/cs-CS.json
- src/assets/translations/it-IT.json
- src/app/shared/language-selector/language-selector.component.html
🧰 Additional context used
📓 Path-based instructions (1)
src/app/**
⚙️ CodeRabbit configuration file
src/app/**: For Angular code: verify component separation, trackBy on *ngFor,
strict type safety, and clean observable patterns.
Files:
src/app/login/login-form/login-form.component.scsssrc/app/login/login-form/login-form.component.tssrc/app/system/external-services/email/edit-email/edit-email.component.htmlsrc/app/login/login.component.htmlsrc/app/shared/footer/footer.component.htmlsrc/app/shared/language-selector/language-selector.component.scsssrc/app/shared/icons.module.tssrc/app/login/login-form/login-form.component.htmlsrc/app/login/login.component.scsssrc/app/shared/m3-ui/m3-icon/m3-icon.component.scsssrc/app/shared/server-selector/server-selector.component.scsssrc/app/login/login.component.tssrc/app/shared/tenant-selector/tenant-selector.component.htmlsrc/app/login/reset-password/reset-password.component.html
🧬 Code graph analysis (1)
src/app/login/login-form/login-form.component.ts (1)
src/app/shared/m3-ui/index.ts (1)
M3IconComponent(8-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Lint, Build and Deploy
🔇 Additional comments (26)
src/assets/translations/es-CL.json (2)
1194-1197: LGTM! Translations are accurate and consistent.The Spanish translations for these new label entries are appropriate for Chilean Spanish and follow the established conventions in this translation file. The vocabulary choices are suitable for a financial application login interface:
- "Seguro y Confiable" (Secure & Reliable)
- "Impulsado por la Comunidad" (Community Driven)
- "Impacto Global" (Global Impact)
The capitalization pattern (title case) is consistent with existing translations throughout the file.
2675-2676: LGTM! Placeholder translations use appropriate formal register.The translations for these input placeholders are correct and use the formal "usted" form, which is appropriate for a business/financial application login interface:
- "Ingrese su nombre de usuario" (Enter your username)
- "Ingrese su contraseña" (Enter your password)
The consistent use of the formal imperative "Ingrese" maintains a professional tone suitable for enterprise applications.
src/assets/translations/pt-PT.json (1)
2674-2674: Verify placeholder key naming convention.The key "Search Activity" doesn't follow the naming pattern of other keys in the
placeholderssection. Other placeholder keys use the format"placeholder.{action}"(e.g., lines 2675-2676: "Enter your username", "Enter your password").Please verify whether this key should be:
- Renamed to match the pattern, or
- Moved to a different section if it's not actually a placeholder
src/assets/translations/sw-SW.json (2)
1194-1196: Feature descriptions added appropriately for login page.The three new translations for "Secure & Reliable", "Community Driven", and "Global Impact" are well-placed in the heading section and align with the Material Design 3 login implementation objective. The Swahili translations appear grammatically correct and semantically appropriate for feature callouts.
2673-2674: Login form placeholders correctly added.The username and password placeholder translations are properly placed in the "placeholders" section. The Swahili translations "Ingiza jina lako la mtumiaji" and "Ingiza nenosiri lako" are idiomatic and appropriate for login form field placeholders.
Note: The duplicate "placeholder" (singular) section issue mentioned in previous reviews is not present in the current code - only the correct "placeholders" (plural) section exists.
src/assets/translations/ko-KO.json (2)
2675-2676: Verify the new placeholder keys are used in login form inputs.Two new placeholder entries have been added for username and password inputs. Please confirm these translation keys are referenced in the login form template.
1197-1199: Verify these new translation keys are used in the login page implementation.Three new heading entries have been added: "Secure & Reliable", "Community Driven", and "Global Impact". Since this PR is implementing Material Design 3 for the login page, please verify that these translation keys are actually referenced in the login component code.
#!/bin/bash # Description: Verify that the new translation keys are used in the codebase # Search for usage of the new translation keys in TypeScript/Angular files rg -n --type=ts -C3 'Secure & Reliable|Community Driven|Global Impact' # Also search in HTML templates rg -n --type=html -C3 'Secure & Reliable|Community Driven|Global Impact'src/app/shared/language-selector/language-selector.component.scss (2)
1-11: LGTM! Clean variant separation.The use of
:not(.outlined-variant)cleanly separates the default fill variant styling from the outlined variant. The minimal styling is appropriate for the fill variant.
84-131: LGTM! Dark theme implementation is correct.The dark theme styling properly mirrors the light theme structure with appropriate color values for dark mode. The scoping to
.dark-theme #language-selector.outlined-variantis correct.Please verify that the default (fill) variant also supports dark theme properly. If it relies on Angular Material's default theming, this is fine; otherwise, explicit dark theme styles may be needed similar to the outlined variant.
Optional: Consider SCSS mixins to reduce duplication
The dark theme block duplicates the structure from the light theme. While the current approach is clear and maintainable, you could optionally refactor using SCSS mixins:
@mixin outlined-variant-theme($border-color, $hover-color, $focus-color, $text-color, $arrow-color, $label-color) { .mdc-notched-outline { .mdc-notched-outline__leading, .mdc-notched-outline__notch, .mdc-notched-outline__trailing { border-color: $border-color; } } // ... rest of styles using parameters } // Then use: #language-selector.outlined-variant { @include outlined-variant-theme( var(--md-sys-color-outline, #79747e), var(--md-sys-color-on-surface, #1a1c1e), // ... etc ); }This is purely optional and the current approach works well.
src/app/shared/icons.module.ts (1)
142-267: LGTM! All icons properly registered.All newly imported icons are correctly registered with the Font Awesome library, maintaining alphabetical order in the registration list. The dependency injection pattern and module structure follow Angular best practices.
src/assets/translations/es-MX.json (2)
1195-1197: LGTM! Translation additions are accurate and well-formatted.The three new heading translations for Material Design 3 UI elements are grammatically correct and contextually appropriate:
- "Secure & Reliable" → "Seguro y Confiable" ✓
- "Community Driven" → "Impulsado por la Comunidad" ✓
- "Global Impact" → "Impacto Global" ✓
The title case capitalization is consistent with existing heading translations in the file.
2676-2678: LGTM! Placeholder translations are appropriate and user-friendly.The new placeholder translations for login form elements are grammatically correct and use the appropriate formal register:
- "Search Activity" → "Actividad de búsqueda" ✓
- "Enter your username" → "Ingrese su nombre de usuario" ✓
- "Enter your password" → "Ingrese su contraseña" ✓
The use of formal "usted" form ("Ingrese su") is appropriate for a professional business application.
src/app/login/reset-password/reset-password.component.html (1)
20-24: LGTM! Clean template consolidation.The simplified else branch for the password visibility toggle is functionally equivalent and improves template readability.
src/app/system/external-services/email/edit-email/edit-email.component.html (1)
28-32: LGTM! Consistent password toggle pattern.The else branch consolidation aligns with the pattern used in other password fields and improves maintainability.
src/app/shared/footer/footer.component.html (2)
1-34: Footer restructuring looks good.The removal of the redundant nested
@if (displayBackEndInfo)check around the Server row (previously flagged in past reviews) has been addressed. The new structure properly places all version/server information directly inside the outer conditional block with appropriate nested conditions only where needed (business date).
35-37: Copy label rendering is now unconditional.Moving the copy label outside the
displayBackEndInfoconditional ensures it always renders. Verify this aligns with the intended UX—if backend info is hidden, should the "by Mifos" label still appear?src/app/shared/server-selector/server-selector.component.scss (1)
1-133: LGTM! Well-structured MD3 styling with proper theming support.The outlined variant styling properly implements Material Design 3 tokens with CSS custom properties and fallbacks. Dark theme support is correctly scoped and comprehensive across all states (normal, hover, focus, pressed, disabled).
src/app/login/login-form/login-form.component.ts (1)
11-11: LGTM! Clean migration to M3IconComponent.The replacement of FontAwesome's
FaIconComponentwithM3IconComponentaligns with the PR's Material Design 3 implementation objectives.Also applies to: 28-28
src/theme/_material-web.scss (1)
182-315: LGTM! Dark theme support is comprehensive and properly scoped.The dark theme overrides cover all button variants and states with appropriate color adjustments for dark surfaces. The
.dark-themeclass scoping ensures proper isolation.package.json (1)
50-50: Version 2.4.1 is current and no known security vulnerabilities are documented.@material/web version 2.4.1 is the latest stable release. No security advisories or vulnerabilities were found for this package in current vulnerability databases.
src/app/login/login-form/login-form.component.scss (1)
428-439: LGTM! Well-optimized animation.The slide-in animation uses GPU-accelerated properties (transform, opacity) which ensures smooth performance. The subtle 10px translation provides good UX without being distracting.
src/app/login/login-form/login-form.component.html (1)
5-21: LGTM! Well-implemented username field.The username field properly implements:
- Autocomplete attribute for password managers
- Translated placeholder text
- M3 icon component integration
- Error message handling
src/app/login/login.component.scss (2)
423-456: LGTM! Performance-optimized animations.The keyframe animations use only
opacityandtransformproperties, which are GPU-accelerated and won't cause layout reflows. The animation durations and easings provide smooth, professional transitions.
527-668: LGTM! Comprehensive responsive design implementation.The responsive breakpoints provide excellent mobile experience:
- Progressive enhancement from desktop to mobile
- Appropriate font size and spacing adjustments
- Hero panel properly hidden on smaller screens
- Touch-friendly button sizes maintained
src/app/shared/m3-ui/m3-icon/m3-icon.component.scss (1)
1-69: Verify Material Symbols fonts are properly loaded.The SCSS file references
'Material Symbols Outlined','Material Symbols Rounded', and'Material Symbols Sharp'font families without including@font-facedeclarations or import statements in this file. The comment mentions fonts "will be loaded from Google Fonts or local files," but the loading mechanism isn't visible here.Ensure the fonts are loaded via:
@font-facedeclarations in a global stylesheet- Google Fonts import in
index.html- Or a separate font configuration file
src/app/login/login.component.ts (1)
78-85: Verify theme subscription logic for unused parameter and redundant reads.The subscription receives an emitted
valueparameter but doesn't use it, instead re-readingthemeDarkEnabledfromsettingsService. Confirm whether:
- The
valueparameter should be used instead of re-reading from the service- The
themeobservable emits theme state that should drive the dark mode logic- If the subscription is only for notification, the unused parameter should be removed
This affects code clarity and correctness of theme state synchronization.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/system/external-services/email/edit-email/edit-email.component.html (1)
21-27: Fix button configuration issues.The password visibility toggle button has several issues:
- Conflicting Material directives: Both
mat-buttonandmat-icon-buttonare applied. These are mutually exclusive—use onlymat-icon-buttonfor icon-only buttons.- Missing button type: Add
type="button"to prevent unintended form submission.- Accessibility gap: Add
aria-labelto announce the button's purpose to screen readers (e.g., "Toggle password visibility").🔎 Proposed fix
<button - mat-button matSuffix mat-icon-button + type="button" + [attr.aria-label]="passwordInputType === 'password' ? ('labels.buttons.Show Password' | translate) : ('labels.buttons.Hide Password' | translate)" (mousedown)="passwordInputType = 'text'" (mouseup)="passwordInputType = 'password'" >As per coding guidelines, ensuring accessibility and proper component usage in Angular templates.
♻️ Duplicate comments (10)
src/assets/translations/de-DE.json (1)
1195-1195: Duplicate translation key "Date" detected (still unresolved from previous review).The key
"Date"is added here but already exists at line 1499 with the same translation value"Datum". In JSON objects, duplicate keys cause undefined behavior—typically the last occurrence wins, but some parsers may error. This duplication must be removed.🔎 Proposed fix
Remove the duplicate entry at line 1195:
"Not Yet Amortized Amount": "Noch nicht amortisierter Betrag", "Adjusted Amount": "Angepasster Betrag", "Charged Off Amount": "Abgeschriebener Betrag", - "Date": "Datum", "Secure & Reliable": "Sicher und zuverlässig", "Community Driven": "Gemeinschaftsorientiert", "Global Impact": "Globale Auswirkungen"The existing
"Date": "Datum"entry at line 1499 should be retained.src/app/shared/tenant-selector/tenant-selector.component.html (1)
1-10: Consider migrating tenant icon to the MD3 icon component for consistency.Template structure and
@for (tenant of tenants; track tenant)look fine, but this still uses<fa-icon>while the rest of the PR is moving tomifosx-m3-icon. For bundle and visual consistency, consider swapping this toM3IconComponent(e.g., a suitable Material “apartment/building” icon) when convenient.src/app/login/login-form/login-form.component.html (2)
76-81: Replacehref="#"on “Forgot Password” with a semantic action.Using
href="#"causes an unnecessary scroll and isn’t ideal for accessibility. Prefer a button (styled as a link) or arouterLinkto the actual forgot-password route and handle the behavior in the component.Example using a button click handler
- <div class="login-actions"> - <a href="#" class="forgot-password-link"> - {{ 'labels.links.Forgot Password?' | translate }} - </a> - </div> + <div class="login-actions"> + <button + type="button" + class="forgot-password-link" + (click)="onForgotPassword()" + > + {{ 'labels.links.Forgot Password?' | translate }} + </button> + </div>
23-46: Localize the password visibility toggle aria-label.
[attr.aria-label]uses hardcoded English strings, which breaks i18n for screen readers. Bind to translation keys instead so the accessible name is localized.Suggested aria-label binding
- <button - type="button" - matSuffix - mat-icon-button - (click)="togglePasswordVisibility()" - [attr.aria-label]="passwordInputType === 'password' ? 'Show password' : 'Hide password'" - > + <button + type="button" + matSuffix + mat-icon-button + (click)="togglePasswordVisibility()" + [attr.aria-label]=" + (passwordInputType === 'password' + ? 'labels.buttons.Show password' + : 'labels.buttons.Hide password') | translate + " + >src/app/login/login.component.scss (1)
476-481: Removecontent: url(...)logo swap now that template handles dark logo.Using
contenton an<img>is non-standard and unnecessary since the template already conditionally renders the light/dark logos. Dropping thecontentrule avoids cross-browser inconsistencies and keeps logo selection in one place.Scoped SCSS cleanup
.logo-image { - max-width: 20rem; - padding: 0; - margin: 0; - content: url('../../assets/images/white-mifos.png'); + max-width: 20rem; + padding: 0; + margin: 0; }src/theme/_material-web.scss (1)
21-36: Remove unsupported button typography custom properties.The
--md-*-button-label-text-size/--md-*-button-label-text-weightproperties (for filled/outlined/elevated/text/tonal) are not recognized Material Web button tokens, so they will be ignored and could confuse future maintainers. Keep only the documented--md-*-button-label-text-font(if needed) and drive size/weight via the system typescale tokens at the theme root instead. Also consider moving the--md-sys-color-*assignments out of the button selectors and into your global theme so other components consistently pick them up.Example cleanup (keep only supported tokens)
// Typography - --md-filled-button-label-text-size: 14px; - --md-filled-button-label-text-weight: 500; --md-filled-button-label-text-font: 'Roboto', sans-serif; - --md-outlined-button-label-text-size: 14px; - --md-outlined-button-label-text-weight: 500; --md-outlined-button-label-text-font: 'Roboto', sans-serif; - --md-elevated-button-label-text-size: 14px; - --md-elevated-button-label-text-weight: 500; --md-elevated-button-label-text-font: 'Roboto', sans-serif; - --md-text-button-label-text-size: 14px; - --md-text-button-label-text-weight: 500; --md-text-button-label-text-font: 'Roboto', sans-serif; - --md-filled-tonal-button-label-text-size: 14px; - --md-filled-tonal-button-label-text-weight: 500; --md-filled-tonal-button-label-text-font: 'Roboto', sans-serif;src/assets/translations/pt-PT.json (1)
1195-1198: Align new pt-PT strings with European Portuguese spelling.For the pt-PT locale these new entries should use European Portuguese forms; right now they’re Brazilian:
"Secure & Reliable": "Seguro e confiável"→"Seguro e fiável""Enter your username": "Digite seu nome de usuário"→ e.g."Introduza o seu nome de utilizador""Enter your password": "Digite sua senha"→ e.g."Introduza a sua palavra-passe"This keeps the file consistent with its
pt-PTdesignation.Targeted JSON edits
- "Secure & Reliable": "Seguro e confiável", + "Secure & Reliable": "Seguro e fiável", @@ - "Enter your username": "Digite seu nome de usuário", - "Enter your password": "Digite sua senha" + "Enter your username": "Introduza o seu nome de utilizador", + "Enter your password": "Introduza a sua palavra-passe"Also applies to: 2673-2676
src/app/login/login.component.html (2)
115-119: Add aria-label to the icon-only reload button for accessibility.The reload button currently relies on
matTooltiptext, which is not exposed to screen readers. Add an aria-label tied to a translated string so assistive tech can announce its purpose.Suggested template change
- <button mat-icon-button (click)="reloadSettings()" class="reload-button" matTooltip="Reload Settings"> + <button + mat-icon-button + (click)="reloadSettings()" + class="reload-button" + matTooltip="Reload Settings" + [attr.aria-label]="'labels.buttons.Reload Settings' | translate" + > <mifosx-m3-icon name="refresh"></mifosx-m3-icon> </button>
29-37: Harden external links opened withtarget="_blank"by addingrelattributes.All external anchors using
target="_blank"(hero links and the three resource menus) should includerel="noopener noreferrer"to prevent tab-napping viawindow.opener.Apply to all external anchors in this template
- <a href="https://mifos.org/" target="_blank" class="hero-link"> + <a href="https://mifos.org/" target="_blank" rel="noopener noreferrer" class="hero-link"> @@ - <a href="https://mifosforge.jira.com/wiki/spaces/docs/pages/52035622/User+Manual" target="_blank" mat-menu-item> + <a href="https://mifosforge.jira.com/wiki/spaces/docs/pages/52035622/User+Manual" + target="_blank" rel="noopener noreferrer" mat-menu-item> @@ - <a - href="https://cwiki.apache.org/confluence/display/FINERACT/Apache+Fineract+1.0+Functional+Specifications" - target="_blank" - mat-menu-item - > + <a + href="https://cwiki.apache.org/confluence/display/FINERACT/Apache+Fineract+1.0+Functional+Specifications" + target="_blank" rel="noopener noreferrer" + mat-menu-item + >…and similarly for the remaining external links in the community and contribute menus.
Also applies to: 127-177
src/app/shared/m3-ui/m3-button/m3-button.component.ts (1)
65-68: Circular dependency and redundant CommonModule import remain unresolved.The past review comment identified that:
- M3ButtonComponent imports
STANDALONE_SHARED_IMPORTS, which re-exportsM3ButtonComponentitself (as shown in the relevant code snippet fromstandalone-shared.module.tsline 51), creating a circular dependency.CommonModuleis explicitly imported here but is already included inSTANDALONE_SHARED_IMPORTS(as shown in the snippet lines 20-50), making it redundant.Remove
M3ButtonComponentfrom theSTANDALONE_SHARED_IMPORTSarray instandalone-shared.module.ts, then remove the explicitCommonModuleimport from this component's imports array.🔎 Proposed fix
In this file, remove the redundant
CommonModule:imports: [ - STANDALONE_SHARED_IMPORTS, - CommonModule + STANDALONE_SHARED_IMPORTS ],Additionally, in
src/app/standalone-shared.module.ts, removeM3ButtonComponentfrom theSTANDALONE_SHARED_IMPORTSarray to break the circular dependency.
🧹 Nitpick comments (11)
.stylelintrc (1)
20-21: Consider setting realistic specificity limits instead of removing constraints entirely.Disabling both
selector-max-specificityandno-descending-specificityremoves important CSS architecture checks. While MD3 theming may increase complexity, setting these tonullis overly permissive and could mask poor CSS practices.Suggest setting
selector-max-specificityto a higher but still reasonable limit (e.g.,"1,4,3"or"2,3,3") and keepingno-descending-specificityenabled to maintain architectural guardrails even within MD3's broader design token patterns.To verify whether unlimited specificity is necessary, check if Material Web's MD3 components or the new theming patterns truly require selectors with higher specificity than a reasonable limit would allow. If they do, document the specific selectors requiring higher specificity.
src/app/shared/icons.module.ts (1)
142-267: All new icons are correctly registered.Each of the 11 newly imported icons has been properly added to
library.addIcons, ensuring they will be available for use throughout the application.Optional: Consider removing pre-existing duplicate registrations.
The following icons are registered more than once (pre-existing issue, not introduced by this PR):
faBanappears at lines 147 and 244faBarsappears at lines 148 and 262faUndoappears at lines 230 and 263While this doesn't break functionality, removing the duplicates would slightly reduce bundle size and improve maintainability.
src/app/shared/server-selector/server-selector.component.scss (1)
1-80: MD3 outlined variant styling looks solid; consider pruning the redundant fill-variant width rule.The outlined/dark-theme styles correctly target MDC classes, use MD3 CSS variables with sensible fallbacks, and the
.stickyupdates are consistent with the new token-based theming.One minor cleanup opportunity: inside
#server-selector.mat-mdc-form-fieldthe block&:not(.outlined-variant) { mat-form-field { width: 96%; } }likely never matches (the host with
id="server-selector"is itself themat-form-field, so there’s no nestedmat-form-field), andmat-form-field { width: 96%; }below already sets the width globally. You could drop this inner block to avoid dead CSS.Also applies to: 82-133, 146-152
src/app/standalone-shared.module.ts (1)
52-53: Clarify how M3ButtonComponent should be consumed from the shared moduleRe‑exporting
M3ButtonComponenthere is useful, but it’s not part ofSTANDALONE_SHARED_IMPORTS, so standalone components that spread that array won’t automatically get the MD3 button. If you intend the MD3 button to be a common primitive like the Material controls above, consider also adding it toSTANDALONE_SHARED_IMPORTS; otherwise this re‑export is fine but callers must still import the component explicitly in theirimportsarrays.src/app/shared/theme-toggle/theming.service.ts (1)
40-50: Unify body class handling for OS theme changesThe symmetric add/remove of
dark-themeandlight-themeinsetDarkModeis good. Right now, though, thematchMedia('(prefers-color-scheme: dark)')listener only updates thethemesubject and doesn’t touchdocument.body.classList, so changing the OS theme while the app is open may leave body classes out of sync.Consider delegating to
setDarkMode(turnOn)from the listener to keep both the BehaviorSubject and body classes consistent:Possible refactor
- window.matchMedia('(prefers-color-scheme: dark)').addListener((e) => { - const turnOn = e.matches; - this.theme.next(turnOn ? 'dark-theme' : 'light-theme'); - - // Trigger refresh of UI - this.ref.tick(); - }); + window.matchMedia('(prefers-color-scheme: dark)').addListener((e) => { + const turnOn = e.matches; + this.setDarkMode(turnOn); + // Trigger refresh of UI + this.ref.tick(); + });src/app/shared/theme-toggle/theme-toggle.component.ts (1)
5-44: Theme toggle wiring to settings and theming services looks goodUsing
M3IconComponentand the newtoggleTheme()method cleanly encapsulates theme switching (local state → persisted setting → theming service). One small cleanup opportunity: since this component has no@Input()s,ngOnChangeswill never fire and can be dropped along with theOnChangesinterface to simplify the class.src/main.scss (1)
2-85: MD3 select panel styling looks coherent; consider centralizing state colors.The new
.mat-mdc-select-paneland option styles align well with MD3 tokens and dark-theme overrides. To keep theming easier to maintain, you might eventually replace the hard-coded state colors (#1074b9*) with shared CSS variables or SCSS variables so light/dark variants stay in sync with a single source of truth.src/app/login/login-form/login-form.component.html (1)
4-5: Hook up.login-formSCSS by adding the matching class to the form.The SCSS defines styles for
.login-form(gap and slide-in animation), but the form only hasclass="layout-column". Adding thelogin-formclass will ensure the intended MD3 form spacing and animation actually apply.Minimal template tweak
- <form class="layout-column" [formGroup]="loginForm" (ngSubmit)="login()" id="login-form"> + <form class="login-form layout-column" [formGroup]="loginForm" (ngSubmit)="login()" id="login-form">src/app/login/login-form/login-form.component.scss (1)
5-439: MD3 login form styling is consistent; consider future reduction of::ng-deep.The new
.login-field, progress, submit button, and OIDC blocks provide a coherent MD3 look in both light and dark themes, and the responsive tweaks are sensible. Long term, you may want to migrate some of the::ng-deepoverrides for Angular Material internals into a shared theme or global styles to reduce reliance on deprecated selectors, but it’s fine for now given the current Material APIs.src/app/login/login.component.scss (1)
14-140: Overall MD3 login layout and responsiveness look solid.The hero/login two-panel layout, header controls (server/language/theme), card elevation, and responsive breakpoints are all coherent and match the MD3 direction. Only minor note is that there’s quite a bit of
::ng-deepand selector coupling to specific IDs/classes (e.g.#language-selector.outlined-variant), so if those component internals evolve you may want to centralize these overrides in a shared theming layer to keep maintenance localized.Also applies to: 241-370, 528-655
src/app/login/login.component.ts (1)
78-85: Subscription callback ignores the emitted theme value.The subscription to
themingService.themereceives avalue: stringparameter but doesn't use it. Instead, it readssettingsService.themeDarkEnabledagain, which duplicates the initial read on Line 78. This pattern defeats the purpose of reactive subscriptions.Consider refactoring to use the emitted value or clarify why the service needs to be re-read on every theme change. If the emitted value represents the theme state, use it directly to update
themeDarkEnabled.🔎 Proposed refactor to use the emitted theme value
If
themingService.themeemits a boolean or a value that can determine dark mode state:- this.themeDarkEnabled = this.settingsService.themeDarkEnabled; - // Subscribe to theme changes - this.theme$ = this.themingService.theme.subscribe((value: string) => { - this.themeDarkEnabled = this.settingsService.themeDarkEnabled; - }); - - // Initialize theme based on settings - this.themingService.setDarkMode(!!this.settingsService.themeDarkEnabled); + // Initialize theme from settings + const initialThemeDark = this.settingsService.themeDarkEnabled; + this.themeDarkEnabled = initialThemeDark; + this.themingService.setDarkMode(!!initialThemeDark); + + // Subscribe to theme changes + this.theme$ = this.themingService.theme.subscribe((theme: string) => { + // Update state based on emitted theme value + this.themeDarkEnabled = theme === 'dark' || this.settingsService.themeDarkEnabled; + });Note: Adjust the logic based on what
themingService.themeactually emits.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.jsonand included by**/*
📒 Files selected for processing (48)
.stylelintrcpackage.jsonsrc/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/app/login/reset-password/reset-password.component.htmlsrc/app/shared/footer/footer.component.htmlsrc/app/shared/footer/footer.component.tssrc/app/shared/icons.module.tssrc/app/shared/language-selector/language-selector.component.htmlsrc/app/shared/language-selector/language-selector.component.scsssrc/app/shared/language-selector/language-selector.component.tssrc/app/shared/m3-ui/index.tssrc/app/shared/m3-ui/m3-button/m3-button.component.htmlsrc/app/shared/m3-ui/m3-button/m3-button.component.scsssrc/app/shared/m3-ui/m3-button/m3-button.component.tssrc/app/shared/m3-ui/m3-icon/m3-icon.component.htmlsrc/app/shared/m3-ui/m3-icon/m3-icon.component.scsssrc/app/shared/m3-ui/m3-icon/m3-icon.component.tssrc/app/shared/server-selector/server-selector.component.htmlsrc/app/shared/server-selector/server-selector.component.scsssrc/app/shared/server-selector/server-selector.component.tssrc/app/shared/tenant-selector/tenant-selector.component.htmlsrc/app/shared/tenant-selector/tenant-selector.component.scsssrc/app/shared/theme-toggle/theme-toggle.component.htmlsrc/app/shared/theme-toggle/theme-toggle.component.tssrc/app/shared/theme-toggle/theming.service.tssrc/app/standalone-shared.module.tssrc/app/system/external-services/email/edit-email/edit-email.component.htmlsrc/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/index.htmlsrc/main.scsssrc/theme/_material-web.scss
💤 Files with no reviewable changes (1)
- src/app/shared/footer/footer.component.ts
🚧 Files skipped from review as they are similar to previous changes (18)
- src/assets/translations/fr-FR.json
- src/app/shared/server-selector/server-selector.component.ts
- src/assets/translations/cs-CS.json
- src/assets/translations/ne-NE.json
- src/app/shared/server-selector/server-selector.component.html
- src/assets/translations/lt-LT.json
- src/index.html
- src/app/shared/m3-ui/index.ts
- src/app/shared/m3-ui/m3-icon/m3-icon.component.ts
- src/app/login/reset-password/reset-password.component.html
- src/app/shared/m3-ui/m3-icon/m3-icon.component.scss
- src/assets/translations/en-US.json
- src/assets/translations/es-MX.json
- src/assets/translations/ko-KO.json
- src/app/shared/m3-ui/m3-button/m3-button.component.html
- src/assets/translations/lv-LV.json
- src/app/shared/m3-ui/m3-button/m3-button.component.scss
- src/app/shared/m3-ui/m3-icon/m3-icon.component.html
🧰 Additional context used
📓 Path-based instructions (1)
src/app/**
⚙️ CodeRabbit configuration file
src/app/**: For Angular code: verify component separation, trackBy on *ngFor,
strict type safety, and clean observable patterns.
Files:
src/app/login/login-form/login-form.component.tssrc/app/shared/theme-toggle/theme-toggle.component.htmlsrc/app/shared/language-selector/language-selector.component.htmlsrc/app/login/login-form/login-form.component.scsssrc/app/shared/theme-toggle/theme-toggle.component.tssrc/app/shared/language-selector/language-selector.component.tssrc/app/login/login.component.htmlsrc/app/shared/icons.module.tssrc/app/shared/server-selector/server-selector.component.scsssrc/app/shared/tenant-selector/tenant-selector.component.scsssrc/app/login/login-form/login-form.component.htmlsrc/app/shared/language-selector/language-selector.component.scsssrc/app/shared/tenant-selector/tenant-selector.component.htmlsrc/app/login/login.component.scsssrc/app/standalone-shared.module.tssrc/app/login/login.component.tssrc/app/shared/footer/footer.component.htmlsrc/app/system/external-services/email/edit-email/edit-email.component.htmlsrc/app/shared/m3-ui/m3-button/m3-button.component.tssrc/app/shared/theme-toggle/theming.service.ts
🧬 Code graph analysis (4)
src/app/login/login-form/login-form.component.ts (1)
src/app/shared/m3-ui/index.ts (1)
M3IconComponent(8-8)
src/app/shared/theme-toggle/theme-toggle.component.ts (1)
src/app/shared/m3-ui/index.ts (1)
M3IconComponent(8-8)
src/app/login/login.component.ts (1)
src/app/shared/m3-ui/index.ts (1)
M3IconComponent(8-8)
src/app/shared/m3-ui/m3-button/m3-button.component.ts (2)
src/app/shared/m3-ui/index.ts (2)
M3ButtonVariant(7-7)M3ButtonComponent(7-7)src/app/standalone-shared.module.ts (2)
STANDALONE_SHARED_IMPORTS(21-51)M3ButtonComponent(52-52)
🔇 Additional comments (19)
.stylelintrc (1)
18-19: LGTM: Increases to nesting/compound selectors and MD3/template ignores.The increases to
max-nesting-depth(5→7) andselector-max-compound-selectors(5→8) are reasonable accommodations for complex theme-driven MD3 styling. Adding/^md-/andng-templatetoignoreTypescorrectly supports Material Web components and Angular templates.Also applies to: 50-51
src/app/system/external-services/email/edit-email/edit-email.component.html (1)
28-32: LGTM: Simplified password visibility toggle logic.The consolidation from two separate
@ifconditions to an@if/@elsestructure makes the mutually exclusive logic more explicit and improves readability.src/assets/translations/de-DE.json (1)
2674-2676: LGTM! New placeholder translations are accurate.The new placeholder entries are well-translated using appropriate German imperative forms and maintain consistency with the existing translation style in the file.
src/assets/translations/sw-SW.json (2)
1193-1196: LGTM! New heading translations added correctly.The new translation entries for "Date", "Secure & Reliable", "Community Driven", and "Global Impact" are properly formatted and appropriately placed in the "heading" section. The Swahili translations are contextually accurate for what appear to be feature highlights or marketing labels.
2670-2675: LGTM! Duplicate section issue resolved and new placeholders added correctly.The new placeholder translations for "Search Activity", "Enter your username", and "Enter your password" have been properly added to the existing "placeholders" section. The Swahili translations are accurate and appropriate for form input placeholders.
✅ Good work: The duplicate "placeholder" (singular) section mentioned in the previous review has been successfully resolved. The file now correctly maintains a single "placeholders" (plural) section, eliminating the structural inconsistency and potential confusion.
src/app/shared/footer/footer.component.html (2)
1-34: Well-structured conditional rendering with proper gating.The refactored template correctly addresses the previously flagged redundant nested
@ifissue. The Server row (lines 17-22) is now directly inside the parent block without its own conditional wrapper, and the business date section appropriately uses a separate@if (isBusinessDateDefined)condition.Minor style note: Line 2 uses
ngClass="{{ styleClass }}"interpolation syntax. The property binding[ngClass]="styleClass"is slightly more idiomatic, though both work correctly.
35-37: Good placement of the branding footer.Moving the copy-label outside the
displayBackEndInfoconditional ensures consistent footer branding visibility regardless of backend info display settings. The translate pipe usage forAPP_NAMEand labels is correct.src/app/shared/icons.module.ts (1)
6-128: LGTM! Icon imports are correctly added.All 11 new Font Awesome icons are properly imported from
@fortawesome/free-solid-svg-iconsand will expand the icon set available to the application. The implementation follows the existing pattern.src/app/shared/theme-toggle/theme-toggle.component.html (1)
1-2: MD3 theme-toggle icon usage looks good.Icon swapping via
darkModeOn ? 'wb_sunny' : 'nights_stay'and the move tomifosx-m3-iconare straightforward and consistent with the MD3 migration.package.json (1)
50-50: Confirm @material/web version and integration with Angular 20.Adding
@material/webfor MD3 components makes sense; just ensure this version is compatible with your Angular 20 toolchain and verified in your supported browsers (including SSR/build if any), and that your lockfile is updated.src/app/shared/language-selector/language-selector.component.ts (1)
2-2: Typedappearance/showLabelinputs align well with selector variants.Input additions are strictly typed, defaulted, and match how mat-form-field’s
appearanceis typically bound; this keeps the language selector consistent with the server selector API.Also applies to: 28-32
src/assets/translations/es-CL.json (1)
1194-1197: New ES-CL translations for login-related copy look correct and consistent.The added heading and placeholder strings are linguistically sound and follow existing key naming patterns; they should plug into the new login UI without surprises. Consider mirroring any future wording tweaks across all locales.
Also applies to: 2674-2677
src/assets/translations/it-IT.json (2)
1195-1198: New heading labels are consistent with MD3 login copyThe added heading entries (“Date”, “Secure & Reliable”, “Community Driven”, “Global Impact”) are correctly localized and aligned with the key structure used in other locales, so they should drop into the new login UI without issues.
2674-2676: Login placeholders wired correctly to translation keysThe new placeholders for search, username, and password reuse the existing
labels.inputs.Languagenamespace style and look idiomatic. They should work cleanly with the updated login form controls that use these placeholders.src/app/shared/tenant-selector/tenant-selector.component.scss (1)
1-2: Tenant selector styling delegation is fineLeaving only documentation comments here and moving sizing/spacing to the parent login form keeps the selector lean and consistent with other MD3 fields.
src/app/login/login-form/login-form.component.ts (1)
11-31: M3IconComponent integration in login form looks correctSwitching to
M3IconComponentand adding it to the standaloneimportsarray is consistent with the MD3 migration and will let the template use the new icon component without impacting existing logic.src/app/shared/language-selector/language-selector.component.html (1)
1-10: Language selector template matches new appearance/showLabel APIBinding
appearance, conditionally rendering the<mat-label>via@if (showLabel), and using a placeholder only when the label is hidden all line up with the new MD3 styling and the SCSS for#language-selector.outlined-variant. The@for ... track languageusage also provides proper tracking for the options.src/app/shared/language-selector/language-selector.component.scss (1)
1-130: MD3 outlined variant styling for language selector is well-scopedThe SCSS cleanly separates default (filled) vs outlined variants via the
.outlined-variantclass on#language-selector, and the notched-outline, hover/focus, and dark-theme overrides all align with the MD3 token names and the updated template. This should give consistent visuals with the other MD3 selectors on both light and dark themes.src/app/shared/m3-ui/m3-button/m3-button.component.ts (1)
70-70: Verify thatViewEncapsulation.Noneis necessary for Material Web Components.
ViewEncapsulation.Nonedisables view encapsulation, allowing styles to leak globally. While this is often required for styling native web components (like Material Web Components), it can lead to unintended style conflicts across the application.Confirm that this encapsulation mode is necessary for proper Material Web Component styling, and consider scoping the component's CSS to minimize potential side effects.
✅ Actions performedReviews paused. |
2 similar comments
✅ Actions performedReviews paused. |
✅ Actions performedReviews paused. |
alberto-art3ch
force-pushed
the
WEB-492/material_design_3_initial_implementation
branch
11 times, most recently
from
39626e8 to
1e292cb
Compare
alberto-art3ch
force-pushed
the
WEB-492/material_design_3_initial_implementation
branch
from
1e292cb to
a6bd9a2
Compare
alberto-art3ch
force-pushed
the
WEB-492/material_design_3_initial_implementation
branch
from
a6bd9a2 to
45e9b69
Compare
1 participant
Description
Material Design 3 implementation, we are starting the implementation for these components, now we will have custom components, in this case the button and icon as the first ones
WEB-492
Screenshots
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
If you have multiple commits please combine them into one commit by squashing them.
Read and understood the contribution guidelines at
web-app/.github/CONTRIBUTING.md.Summary by CodeRabbit
New Features
Improvements
Translations
✏️ Tip: You can customize this high-level summary in your review settings.