-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Dashboard > Home: Card expenses section display exact amount #3367
Conversation
WalkthroughListen up, superstar! This pull request brings a stylish transformation to the card detail component. The changes focus on smart currency display, using conditional rendering to showcase exact currency values based on screen size. New properties and styling adjustments enhance the visual consistency and responsiveness, ensuring every pixel tells a vibrant story! Changes
Possibly related PRs
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 🪧 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 (
|
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.
Actionable comments posted: 7
🔭 Outside diff range comments (1)
src/app/shared/components/spent-cards/card-detail/card-detail.component.ts (1)
Line range hint
1-58
: Mind-blowing architectural suggestion coming your way!Like how I handle multiple roles in my movies, this component is handling multiple responsibilities. Consider creating a dedicated screen size service for better reusability across components!
Create a new service:
@Injectable({ providedIn: 'root' }) export class ScreenSizeService { private readonly SMALL_SCREEN_WIDTH = 320; private screenSizeSubject = new BehaviorSubject<boolean>(false); isSmallScreen$ = this.screenSizeSubject.asObservable(); constructor() { if (typeof window !== 'undefined') { this.checkScreenSize(); fromEvent(window, 'resize') .pipe(debounceTime(200)) .subscribe(() => this.checkScreenSize()); } } private checkScreenSize(): void { this.screenSizeSubject.next(window.innerWidth <= this.SMALL_SCREEN_WIDTH); } }This way, other components can simply inject and subscribe to the service! What do you say, partner?
🧰 Tools
🪛 eslint
[error] 26-26: Member isSmallScreen should be declared before all public constructor definitions.
(@typescript-eslint/member-ordering)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/app/shared/components/spent-cards/card-detail/card-detail.component.html
(2 hunks)src/app/shared/components/spent-cards/card-detail/card-detail.component.scss
(1 hunks)src/app/shared/components/spent-cards/card-detail/card-detail.component.spec.ts
(2 hunks)src/app/shared/components/spent-cards/card-detail/card-detail.component.ts
(4 hunks)
🧰 Additional context used
🪛 eslint
src/app/shared/components/spent-cards/card-detail/card-detail.component.ts
[error] 26-26: Member isSmallScreen should be declared before all public constructor definitions.
(@typescript-eslint/member-ordering)
🔇 Additional comments (2)
src/app/shared/components/spent-cards/card-detail/card-detail.component.scss (2)
44-44
: Mind it! Perfect vertical alignment, just like my signature walk!
The addition of align-items: center
brings perfect balance to the force... I mean, to the stats layout. When style meets precision, magic happens!
60-60
: Thalaiva approves these precise measurements!
Reducing font sizes from 18px and 16px to 14px is like my perfect timing - small changes, big impact! These adjustments will make the numbers as clear as my famous sunglasses, even on the smallest screens!
Also applies to: 66-66
src/app/shared/components/spent-cards/card-detail/card-detail.component.scss
Show resolved
Hide resolved
src/app/shared/components/spent-cards/card-detail/card-detail.component.ts
Outdated
Show resolved
Hide resolved
src/app/shared/components/spent-cards/card-detail/card-detail.component.ts
Outdated
Show resolved
Hide resolved
src/app/shared/components/spent-cards/card-detail/card-detail.component.html
Outdated
Show resolved
Hide resolved
src/app/shared/components/spent-cards/card-detail/card-detail.component.html
Outdated
Show resolved
Hide resolved
src/app/shared/components/spent-cards/card-detail/card-detail.component.spec.ts
Show resolved
Hide resolved
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.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/app/shared/components/spent-cards/card-detail/card-detail.component.ts (1)
25-26
:⚠️ Potential issueKabali says: Let's maintain our style, thalaiva!
These style issues were mentioned in previous reviews, but they're still hanging around like villains in my movies!
Let's fix them with style:
+ private readonly SMALL_SCREEN_WIDTH = 320; + isSmallScreen: boolean = false; constructor(🧰 Tools
🪛 eslint
[error] 26-26: Member isSmallScreen should be declared before all public constructor definitions.
(@typescript-eslint/member-ordering)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/app/shared/components/spent-cards/card-detail/card-detail.component.ts
(4 hunks)
🧰 Additional context used
🪛 eslint
src/app/shared/components/spent-cards/card-detail/card-detail.component.ts
[error] 26-26: Member isSmallScreen should be declared before all public constructor definitions.
(@typescript-eslint/member-ordering)
🔇 Additional comments (1)
src/app/shared/components/spent-cards/card-detail/card-detail.component.ts (1)
1-1
: Mind it! The OnInit implementation is perfect, just like my signature style!
The way you've imported and implemented OnInit interface is spot on! That's how we roll in the Angular world, partner!
Also applies to: 12-12
src/app/shared/components/spent-cards/card-detail/card-detail.component.ts
Outdated
Show resolved
Hide resolved
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.
same comments as #3365 (review)
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.
added comments
src/app/shared/components/spent-cards/card-detail/card-detail.component.ts
Outdated
Show resolved
Hide resolved
src/app/shared/components/spent-cards/card-detail/card-detail.component.ts
Outdated
Show resolved
Hide resolved
src/app/shared/components/spent-cards/card-detail/card-detail.component.html
Outdated
Show resolved
Hide resolved
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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/app/shared/components/spent-cards/card-detail/card-detail.component.html
(2 hunks)src/app/shared/components/spent-cards/card-detail/card-detail.component.ts
(1 hunks)
🔇 Additional comments (1)
src/app/shared/components/spent-cards/card-detail/card-detail.component.ts (1)
19-20
:
Listen up, partner! Your screen size check needs some style!
The screen size implementation needs more power, just like my signature moves! Here's what's cooking:
- Direct window access without safety check could crash in SSR
- No resize handling means the screen size won't update dynamically
- Magic number 320 should be a constant, like my punch dialogues - memorable and reusable!
Here's how to make it blockbuster worthy:
+ private readonly SMALL_SCREEN_WIDTH = 320;
+ isSmallScreen = false;
+
+ @HostListener('window:resize')
+ private checkScreenSize(): void {
+ if (typeof window !== 'undefined') {
+ this.isSmallScreen = window.innerWidth <= this.SMALL_SCREEN_WIDTH;
+ }
+ }
+
+ ngOnInit(): void {
+ this.checkScreenSize();
+ }
- // To track if the screen is small (320px or below)
- isSmallScreen = window.innerWidth <= 320;
Likely invalid or redundant comment.
src/app/shared/components/spent-cards/card-detail/card-detail.component.html
Show resolved
Hide resolved
src/app/shared/components/spent-cards/card-detail/card-detail.component.html
Show resolved
Hide resolved
|
Clickup
clickup link
UI Preview
Screen.Recording.2024-12-17.at.3.31.35.PM.mov
Summary by CodeRabbit
New Features
Style
Tests
ExactCurrencyPipe
to the test suite for enhanced testing capabilities of the Card Detail component.Chores