feat: Report card section display exact amount #3328
Conversation
WalkthroughThe pull request introduces modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range comments (1)
src/app/shared/components/reports-card/reports-card.component.spec.ts (1)
Line range hint
41-50: Kabali style testing needed for the new features!The test suite needs additional coverage for the new exactCurrency pipe and date display functionality.
Add these test cases:
it('should format currency correctly with exactCurrency pipe', () => { component.report.amount = 1234567.89; component.report.currency = 'USD'; fixture.detectChanges(); expect(getTextContent(getElementBySelector(fixture, '.reports-card--amount'))).toEqual('1,234,567.89'); }); it('should handle date display correctly when showDate is true/false', () => { component.showDate = true; fixture.detectChanges(); expect(getElementBySelector(fixture, '.reports-card--date')).toBeTruthy(); component.showDate = false; fixture.detectChanges(); expect(getElementBySelector(fixture, '.reports-card--date')).toBeFalsy(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (4)
src/app/fyle/my-reports/my-reports.page.html(1 hunks)src/app/shared/components/reports-card/reports-card.component.html(1 hunks)src/app/shared/components/reports-card/reports-card.component.scss(4 hunks)src/app/shared/components/reports-card/reports-card.component.spec.ts(2 hunks)
🔇 Additional comments (3)
src/app/fyle/my-reports/my-reports.page.html (3)
Line range hint 1-136: Superstar approves this template structure!
Like a well-choreographed fight sequence, this template has everything in perfect order - error handling, loading states, and infinite scroll. Keep rocking, partner!
75-75: Kabali style verification needed for this loader, macha!
The new loader is like my entry scene - it needs to make an impact! But before we roll it out:
Let me check if this property is properly defined in the component:
✅ Verification successful
Mind it! The loader is perfectly set up and ready to rock, macha!
Listen here, style-u! The verification shows everything is in perfect order:
- The
isNewLoaderEnabledproperty is properly defined as an Input() in the component - The HTML template is using this property like a well-choreographed dance sequence, controlling:
- Grid classes with
ngClass - Column sizes with dynamic calculations
- Icon container styling
- Grid classes with
The loader will make its grand entry with style, just like how I flip my cigarette! 🕶️
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the isNewLoaderEnabled property in the loading screen component
# Search for the property definition
ast-grep --pattern 'class $_ {
$$$
@Input() isNewLoaderEnabled: boolean;
$$$
}'
# Search for any related tests
rg 'isNewLoaderEnabled.*=.*true' -g '*.spec.ts'
Length of output: 137
Script:
#!/bin/bash
# Let's search for the loading screen component with style!
fd "loading-screen.component.ts" --type f
# Time to check where this property is being used, mind it!
rg "isNewLoaderEnabled" --type ts -C 3
# Let's also check the HTML template of the loading screen component
fd "loading-screen.component.html" --type f --exec cat {}
Length of output: 2581
75-75: 🛠️ Refactor suggestion
Mind it! The new loader needs consistency across all instances, partner!
I notice you've enabled the new loader in one place, but what about the other loading screen at the bottom during infinite scroll? When you make a style change, it should be like my punch - consistent everywhere!
Let me verify this inconsistency with my signature style:
Make it consistent like this, partner:
- <app-fy-loading-screen class="my-reports--shimmers"></app-fy-loading-screen>
+ <app-fy-loading-screen [isNewLoaderEnabled]="true" class="my-reports--shimmers"></app-fy-loading-screen>
|
Clickup
clickup link
UI Preview
Big screens
Small screens
Responsiveness
https://github.com/user-attachments/assets/cfe8a1a4-b90e-4d94-9b37-c98a6d00f87d
Summary by CodeRabbit
New Features
Bug Fixes
Style
Tests