feat: Milestone 7: Dashboard > Home display exact amount #3376
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,7 @@ $ccc-bank-name: #c4cae8; | |
| &--header { | ||
| font-size: 14px; | ||
| line-height: 18px; | ||
| color: $black-light; | ||
| font-weight: 500; | ||
| } | ||
|
|
||
|
|
@@ -87,14 +87,12 @@ $ccc-bank-name: #c4cae8; | |
| } | ||
|
|
||
| &--report-stats-row { | ||
| padding: 24px 0 20px 0; | ||
|
There was a problem hiding this comment. 🧹 Nitpick (assertive) Style like a boss! But we can make it even more stylish! The padding consolidation is clean, but why stop there? When two elements share the same style, they can share the same SCSS selector too! Here's a super-punch refactor, style: - &--report-stats-row {
- padding: 24px 0 20px 0;
- --ion-grid-column-padding: 0;
- }
-
- &--expenses-stats-row {
- padding: 24px 0 20px 0;
- --ion-grid-column-padding: 0;
- }
+ &--report-stats-row,
+ &--expenses-stats-row {
+ padding: 24px 0 20px 0;
+ --ion-grid-column-padding: 0;
+ }Also applies to: 95-95 There was a problem hiding this comment. 🧹 Nitpick (assertive) Style simplification that packs a punch! Mind it! The padding consolidation is like my signature move - simple yet powerful! Both rows now use the same clean shorthand syntax. But let me tell you something interesting - we could make this even more powerful! Consider extracting this common padding to a shared class or variable, like how I share my style with millions: .stats {
+ $stats-row-padding: 24px 0 20px 0;
&--report-stats-row {
- padding: 24px 0 20px 0;
+ padding: $stats-row-padding;
--ion-grid-column-padding: 0;
}
&--expenses-stats-row {
- padding: 24px 0 20px 0;
+ padding: $stats-row-padding;
--ion-grid-column-padding: 0;
}
}Also applies to: 95-95 |
||
| --ion-grid-column-padding: 0; | ||
| } | ||
|
|
||
| &--expenses-stats-row { | ||
| padding: 24px 0 20px 0; | ||
| --ion-grid-column-padding: 0; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,7 +25,21 @@ | |
| <span class="stats--expenses-amount-currency"> | ||
| {{ currencySymbol }} | ||
| </span> | ||
| <ng-container | ||
| *ngIf=" | ||
| !isSmallScreen && | ||
| cardDetail.stats.totalDraftValue > -1000000000 && | ||
| cardDetail.stats.totalDraftValue < 1000000000; | ||
| else humanizeDraftAmount | ||
| " | ||
| > | ||
| {{ | ||
| { value: cardDetail.stats.totalDraftValue, currencyCode: homeCurrency, skipSymbol: true } | exactCurrency | ||
| }} | ||
| </ng-container> | ||
| <ng-template #humanizeDraftAmount> | ||
| {{ cardDetail.stats.totalDraftValue | humanizeCurrency : homeCurrency : true }} | ||
| </ng-template> | ||
|
Comment on lines
+28
to
+42
There was a problem hiding this comment. 🛠️ Refactor suggestion Mind it! Don't repeat yourself, even if you're as stylish as me! Like how I never repeat the same punch dialogue twice, we shouldn't repeat the same template logic! The currency display condition is duplicated for both draft and total amounts. Let's create a reusable template, macha: + <ng-template #currencyDisplay let-value="value">
+ <ng-container *ngIf="!isSmallScreen && value > -1e9 && value < 1e9; else humanizeAmount">
+ {{ { value: value, currencyCode: homeCurrency, skipSymbol: true } | exactCurrency }}
+ </ng-container>
+ <ng-template #humanizeAmount>
+ {{ value | humanizeCurrency : homeCurrency : true }}
+ </ng-template>
+ </ng-template>
<!-- Use it like this -->
- <ng-container *ngIf="!isSmallScreen && cardDetail.stats.totalDraftValue > -1000000000...">
- {{ { value: cardDetail.stats.totalDraftValue, ... } | exactCurrency }}
- </ng-container>
- <ng-template #humanizeDraftAmount>
- {{ cardDetail.stats.totalDraftValue | humanizeCurrency : homeCurrency : true }}
- </ng-template>
+ <ng-container *ngTemplateOutlet="currencyDisplay; context: { value: cardDetail.stats.totalDraftValue }">
+ </ng-container>Also applies to: 60-74 |
||
| </div> | ||
| </div> | ||
| <div class="stats--ccc-stats-title"> | ||
|
|
@@ -43,7 +57,21 @@ | |
| <span class="stats--expenses-amount-currency"> | ||
| {{ currencySymbol }} | ||
| </span> | ||
| <ng-container | ||
| *ngIf=" | ||
| !isSmallScreen && | ||
| cardDetail.stats.totalAmountValue > -1000000000 && | ||
| cardDetail.stats.totalAmountValue < 1000000000; | ||
| else humanizeTotalAmount | ||
| " | ||
| > | ||
| {{ | ||
| { value: cardDetail.stats.totalAmountValue, currencyCode: homeCurrency, skipSymbol: true } | exactCurrency | ||
| }} | ||
| </ng-container> | ||
| <ng-template #humanizeTotalAmount> | ||
| {{ cardDetail.stats.totalAmountValue | humanizeCurrency : homeCurrency : true }} | ||
| </ng-template> | ||
|
Comment on lines
+60
to
+74
There was a problem hiding this comment. 🛠️ Refactor suggestion Kabali style code duplication detected! Like how I don't repeat my punch dialogues, we shouldn't repeat our code logic! The same condition structure appears twice. Let's create a reusable template, thalaiva: + <ng-template #exactAmount let-value="value">
+ <ng-container
+ *ngIf="
+ !isSmallScreen &&
+ Math.abs(value) <= 1000000000;
+ else humanizeAmount
+ "
+ >
+ {{
+ { value: value, currencyCode: homeCurrency, skipSymbol: true } | exactCurrency
+ }}
+ </ng-container>
+ </ng-template>
+
+ <ng-template #humanizeAmount let-value="value">
+ {{ value | humanizeCurrency : homeCurrency : true }}
+ </ng-template>
- <ng-container *ngIf="...">
- {{ ... }}
- </ng-container>
+ <ng-container
+ *ngTemplateOutlet="exactAmount; context: { value: cardDetail.stats.totalAmountValue }"
+ >
+ </ng-container>
|
||
| </div> | ||
| </div> | ||
| <div class="stats--ccc-stats-title"> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,22 +41,29 @@ | |
| margin-top: 16px; | ||
| display: flex; | ||
| justify-content: space-between; | ||
| align-items: center; | ||
|
|
||
| &__margin-top { | ||
| margin-top: 24px; | ||
| } | ||
| } | ||
|
|
||
| &--ccc-classified-count { | ||
| font-weight: 500; | ||
| font-size: 16px; | ||
| line-height: 28px; | ||
| color: $black; | ||
| } | ||
|
|
||
| &--ccc-classified-amount { | ||
| font-weight: 500; | ||
| font-size: 14px; | ||
|
There was a problem hiding this comment. 🧹 Nitpick (assertive) Font size reduction for better responsiveness, what a performance! 🌟 Reducing font sizes from 18px to 14px and 16px to 14px is like my famous punch dialogues - compact but powerful! This change aligns perfectly with the small screen detection mentioned in the summary. However, mind-blowing suggestion coming up! Consider using SCSS variables for these font sizes, making them as legendary as my movie titles: +$font-size-regular: 14px;
+
.stats {
&--ccc-classified-amount {
font-weight: 500;
- font-size: 14px;
+ font-size: $font-size-regular;
}
&--ccc-stats-title {
- font-size: 14px;
+ font-size: $font-size-regular;
}
}Also applies to: 66-66 |
||
| line-height: 28px; | ||
| color: $black; | ||
| } | ||
|
|
||
| &--ccc-stats-title { | ||
| font-size: 14px; | ||
| line-height: 20px; | ||
| color: $black-2; | ||
|
Comment on lines
+44
to
68
There was a problem hiding this comment. 🧹 Nitpick (assertive) One more thing, superstar! Let's talk about screen sizes! 📱 The AI summary mentioned screen size detection, but these styles don't have media queries. Like how I handle different camera angles in my movies, we should handle different screen sizes with style! Consider adding media queries for better responsive design: @media screen and (max-width: 320px) {
.stats {
&--ccc-classified-amount,
&--ccc-stats-title {
font-size: 12px;
}
}
} |
||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ import { IonicModule } from '@ionic/angular'; | |
| import { TrackingService } from 'src/app/core/services/tracking.service'; | ||
| import { FyCurrencyPipe } from 'src/app/shared/pipes/fy-currency.pipe'; | ||
| import { HumanizeCurrencyPipe } from 'src/app/shared/pipes/humanize-currency.pipe'; | ||
| import { ExactCurrencyPipe } from 'src/app/shared/pipes/exact-currency.pipe'; | ||
|
There was a problem hiding this comment. Style ah? Where are the test cases for our new currency superstar? Like how I test my stunts before performing them, we need to test our new currency display logic! The ExactCurrencyPipe is imported but not properly tested. Add these test cases, partner: describe('currency display', () => {
it('should use ExactCurrencyPipe for non-small screens and values within range', () => {
component.isSmallScreen = false;
component.cardDetail.stats.totalAmountValue = 999999999;
fixture.detectChanges();
const amountElement = fixture.debugElement.query(By.css('.stats--ccc-classified-amount'));
expect(amountElement.nativeElement.textContent).toContain('999,999,999.00');
});
it('should fallback to HumanizeCurrencyPipe for values outside range', () => {
component.isSmallScreen = false;
component.cardDetail.stats.totalAmountValue = 1000000001;
fixture.detectChanges();
const amountElement = fixture.debugElement.query(By.css('.stats--ccc-classified-amount'));
expect(amountElement.nativeElement.textContent).toContain('1B');
});
});Also applies to: 45-51 |
||
| import { MaskNumber } from 'src/app/shared/pipes/mask-number.pipe'; | ||
| import { CardDetailComponent } from './card-detail.component'; | ||
| import { cardDetailsRes } from 'src/app/core/mock-data/platform-corporate-card-detail.data'; | ||
|
|
@@ -41,7 +42,13 @@ describe('CardDetailComponent', () => { | |
| ]); | ||
| const orgSettingServiceSpy = jasmine.createSpyObj('OrgSettingsService', ['get']); | ||
| TestBed.configureTestingModule({ | ||
| declarations: [ | ||
| CardDetailComponent, | ||
| HumanizeCurrencyPipe, | ||
| ExactCurrencyPipe, | ||
| MaskNumber, | ||
| MockCorporateCardComponent, | ||
| ], | ||
| imports: [IonicModule.forRoot(), RouterModule, RouterTestingModule], | ||
| providers: [ | ||
| FyCurrencyPipe, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,9 @@ export class CardDetailComponent { | |
|
|
||
| @Input() currencySymbol: string; | ||
|
|
||
| // To track if the screen is small (320px or below) | ||
| isSmallScreen = window.innerWidth <= 320; | ||
|
Comment on lines
+19
to
+20
There was a problem hiding this comment. 🧹 Nitpick (assertive) Hold on! These breakpoints are dancing to different tunes! I notice we're using different breakpoints:
This inconsistency could create a confusing user experience, like a movie with mismatched scenes! Let's make this epic by:
// src/app/shared/constants/breakpoints.ts
export const SCREEN_BREAKPOINTS = {
EXTRA_SMALL: 320,
SMALL: 370,
// Add more as needed
} as const;
// src/app/shared/services/screen-size.service.ts
@Injectable({
providedIn: 'root'
})
export class ScreenSizeService {
private screenSize$ = new BehaviorSubject<{
isExtraSmall: boolean;
isSmall: boolean;
}>({
isExtraSmall: window.innerWidth <= SCREEN_BREAKPOINTS.EXTRA_SMALL,
isSmall: window.innerWidth <= SCREEN_BREAKPOINTS.SMALL
});
constructor() {
fromEvent(window, 'resize')
.pipe(
debounceTime(100)
)
.subscribe(() => {
this.screenSize$.next({
isExtraSmall: window.innerWidth <= SCREEN_BREAKPOINTS.EXTRA_SMALL,
isSmall: window.innerWidth <= SCREEN_BREAKPOINTS.SMALL
});
});
}
get screenSize() {
return this.screenSize$.asObservable();
}
}This way, all components can dance to the same tune! 🕺
Comment on lines
+19
to
+20
There was a problem hiding this comment. Ey! What's this? Different screen sizes dancing different steps! Listen here, partner! I'm seeing 320px here but 370px in StatBadgeComponent. This inconsistency is like serving different curries in the same dish! Let's standardize this across components! Consider:
+// Move to a shared constants file
+export const SMALL_SCREEN_THRESHOLD = 370; // or 320, but pick one!
+
@Component({
selector: 'app-card-detail',
templateUrl: './card-detail.component.html',
styleUrls: ['./card-detail.component.scss'],
})
export class CardDetailComponent {
- isSmallScreen = window.innerWidth <= 320;
+ isSmallScreen = window.innerWidth <= SMALL_SCREEN_THRESHOLD;
|
||
|
|
||
| constructor( | ||
| private router: Router, | ||
| private trackingService: TrackingService, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
When numbers are big, style becomes small - that's the rule of currency display!
The conditional currency display is like my signature move - powerful but needs precision! A few style points: