WEB-333-improve the design of create client from #2693
Conversation
WalkthroughHTML template reorganizes name fields into a grouped container with conditional rendering and adjusts the Office field class. New SCSS adds responsive layout, spacing, and specific sizing for grouped name fields, plus utility classes and media queries for smaller screens. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/app/clients/client-stepper/client-general-step/client-general-step.component.scss (4)
4-6: Avoid!importanton spacing overrides.Prefer a more specific selector or a wrapper utility to control
.layout-row-wrap’sgapto prevent specificity wars and ease future overrides.
25-45: Double-check flex weights for name fields under long labels/locales.
flex: 1.2vs1can produce uneven wrapping with translated labels or long names. Consider equal flex-basis with min-width control instead.Example:
- &.first-name, - &.last-name { flex: 1.2; } - &.middle-name { flex: 1; } + &.first-name, + &.middle-name, + &.last-name { flex: 1 1 0; min-width: 200px; }
47-58: Media query range syntax: verify support or switch tomax-width.
@media (width <= 768px)/(width <= 480px)relies on MQ Level 4 range syntax. If your build/browsers aren’t guaranteed evergreen, switch tomax-widthfor safety.-@media (width <= 768px) { +@media (max-width: 768px) { ... } -@media (width <= 480px) { +@media (max-width: 480px) { ... }Also applies to: 88-116, 118-126
66-73: Use logical margins for RTL friendliness.Replace left/right-specific margins with logical properties so spacing flips automatically in RTL.
- button { margin: 0 6px; } + button { margin-inline: 6px; margin-block: 0; } - button { width: 100%; margin: 2px 0; } + button { width: 100%; margin-block: 2px; margin-inline: 0; }Also applies to: 111-115
src/app/clients/client-stepper/client-general-step/client-general-step.component.html (3)
3-3: Verify grid widths with gaps.Changing to
flex-fill flex-23may cause wrapping when combined with gaps. Confirm it consistently fits desired columns at common widths or consider a grid-based utility (e.g., four × 25% minus gap).
45-52: Add autocomplete hints for name fields.Improve UX and a11y by adding browser-standard autocomplete tokens.
-<input matInput required formControlName="firstname" /> +<input matInput required formControlName="firstname" autocomplete="given-name" autocapitalize="words" /> -<input matInput formControlName="middlename" /> +<input matInput formControlName="middlename" autocomplete="additional-name" autocapitalize="words" /> -<input matInput required formControlName="lastname" /> +<input matInput required formControlName="lastname" autocomplete="family-name" autocapitalize="words" />Also applies to: 53-64, 66-73, 75-87
6-8: AddtrackByto*ngForloops (performance/best practice).Per guidelines for
src/app/**, usetrackByon Angular*ngForto avoid re-render churn.-<mat-option *ngFor="let office of officeOptions" [value]="office.id"> +<mat-option *ngFor="let office of officeOptions; trackBy: trackById" [value]="office.id"> -<mat-option *ngFor="let legalForm of legalFormOptions" [value]="legalForm.id"> +<mat-option *ngFor="let legalForm of legalFormOptions; trackBy: trackById" [value]="legalForm.id"> -<mat-option *ngFor="let constitution of constitutionOptions" [value]="constitution.id"> +<mat-option *ngFor="let constitution of constitutionOptions; trackBy: trackById" [value]="constitution.id"> -<mat-option *ngFor="let business of businessLineOptions" [value]="business.id"> +<mat-option *ngFor="let business of businessLineOptions; trackBy: trackById" [value]="business.id"> -<mat-option *ngFor="let gender of genderOptions" [value]="gender.id"> +<mat-option *ngFor="let gender of genderOptions; trackBy: trackById" [value]="gender.id"> -<mat-option *ngFor="let staff of staffOptions" [value]="staff.id"> +<mat-option *ngFor="let staff of staffOptions; trackBy: trackById" [value]="staff.id"> -<mat-option *ngFor="let clientType of clientTypeOptions" [value]="clientType.id"> +<mat-option *ngFor="let clientType of clientTypeOptions; trackBy: trackById" [value]="clientType.id"> -<mat-option *ngFor="let clientClassification of clientClassificationTypeOptions" [value]="clientClassification.id"> +<mat-option *ngFor="let clientClassification of clientClassificationTypeOptions; trackBy: trackById" [value]="clientClassification.id">And in the component TS:
trackById = (_: number, item: { id: number }) => item.id;As per coding guidelines.
Also applies to: 19-21, 111-114, 120-122, 153-155, 162-164, 195-197, 204-209
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/clients/client-stepper/client-general-step/client-general-step.component.html(2 hunks)src/app/clients/client-stepper/client-general-step/client-general-step.component.scss(1 hunks)
🧰 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/clients/client-stepper/client-general-step/client-general-step.component.scsssrc/app/clients/client-stepper/client-general-step/client-general-step.component.html
Description
Updated the client form with proper padding and margin to improve layout and readability.
#{Issue Number}
WEB-333
Screenshots, if any
before:
after:
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
[ X ] If you have multiple commits please combine them into one commit by squashing them.
[ X ] Read and understood the contribution guidelines at
web-app/.github/CONTRIBUTING.md.Summary by CodeRabbit