fix: update the ui for the new split_expense_page #3460
Conversation
Sishhhh
requested review from
Dimple16
and removed request for
arjunaj5 and
Chethan-Fyle
WalkthroughListen up, superstar! This pull request is a dazzling makeover of the split expense page! We've revamped the HTML and SCSS, creating a smoother expense-splitting experience. The changes include a clear new UI with improved error handling, a fresh date selection, and a more intuitive layout that'll make users shout "Wow!" Changes
Suggested Labels
Suggested Reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (23)
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 (
|
|
@Dimple16 @hlkavya0213 , will add the split form project, cost center , category, purpose alignment, and UI in the next PR, with the component logic |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
src/app/fyle/split-expense/split-expense.page.html(5 hunks)src/app/fyle/split-expense/split-expense.page.scss(4 hunks)src/theme/colors.scss(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (12.x)
🔇 Additional comments (7)
src/theme/colors.scss (1)
48-48: Mind it! These color variables are perfectly balanced, like a well-choreographed dance sequence!The new shadow and gradient variables enhance the visual hierarchy. Keep rocking!
Also applies to: 91-91
src/app/fyle/split-expense/split-expense.page.scss (3)
78-81: Style makeover! This card design is a blockbuster hit!The combination of border, shadow, and spacing creates a clean, elevated look.
92-105: Superstar move with the icon container styling!The new left content and icon container classes create a professional, aligned layout.
249-251: The amount block's gradient background is pure style, like my signature walk!Using $pink-gradient-light-2 adds a subtle yet stylish touch to highlight important information.
src/app/fyle/split-expense/split-expense.page.html (3)
29-32: Warning message that packs a punch! Kabali style!Clear warning about the permanent nature of split actions helps users make informed decisions.
48-55: Error messages that hit harder than my punch dialogues!Good implementation of error messages with icons for better visibility.
205-219: Footer buttons that pack style and functionality! Thalaiva approved!The split actions in the footer are well-organized and clearly labeled.
| <div class="split-expense--main-block"> | ||
| <div | ||
| class="split-expense--item split-expense--date" | ||
| [ngClass]="{'split-expense--date-invalid': splitExpenseForm.controls.txn_dt.touched && !splitExpenseForm.controls.txn_dt.valid}" | ||
| > | ||
| <div class="split-expense--label split-expense--date-label">Date of spend</div> | ||
| <span class="split-expense--mandatory">*</span> | ||
| <input | ||
| appFormatDate | ||
| class="split-expense--date-input date-input__format smartlook-show" | ||
| type="date" | ||
| formControlName="txn_dt" | ||
| [min]="minDate" | ||
| [max]="maxDate" | ||
| /> | ||
| </div> | ||
| <div | ||
| class="split-expense--error" | ||
| *ngIf="splitExpenseForm.controls.txn_dt.touched && !splitExpenseForm.controls.txn_dt.valid" | ||
| > | ||
| Please select a valid date. | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Date input needs some validation power-up!
Add client-side date validation to prevent future dates:
<input
appFormatDate
class="split-expense--date-input date-input__format smartlook-show"
type="date"
formControlName="txn_dt"
[min]="minDate"
[max]="maxDate"
+ (change)="validateDate($event)"
/>Also, consider adding a date picker icon for better user experience:
<div class="split-expense--label split-expense--date-label">Date of spend</div>
<span class="split-expense--mandatory">*</span>
+<div class="date-input-container">
<input
appFormatDate
class="split-expense--date-input date-input__format smartlook-show"
type="date"
formControlName="txn_dt"
[min]="minDate"
[max]="maxDate"
/>
+<mat-icon class="date-picker-icon" svgIcon="calendar"></mat-icon>
+</div>Committable suggestion skipped: line range outside the PR's diff.
| <div class="split-expense--split-amount">{{ currency }} {{remainingAmount }}</div> | ||
| </div> | ||
| </div> | ||
| <div *ngIf="splitExpensesFormArray.controls.length === 30" class="split-expense--error-message-block"> |
There was a problem hiding this comment.
@Sishhhh, is this updated as per latest design Ashwathi shared?
| @@ -87,6 +88,7 @@ $pink-gradient: linear-gradient(162.38deg, $brand-primary 3.01%, $brand-primary- | |||
| $pink-gradient-2: linear-gradient(97.33deg, #e1eeff 26.8%, #ffe4fb 97.18%); | |||
| $pink-gradient-3: linear-gradient(97.33deg, #ffe4fb 26.8%, #e1eeff 97.18%); | |||
| $pink-gradient-light: linear-gradient(90deg, #ff758c 0%, #ff7eb3 100%); | |||
| $pink-gradient-light-2: linear-gradient(97deg, rgba(225, 238, 255, 0.5) 26.8%, rgba(255, 228, 251, 0.5) 97.18%); | |||
| @@ -90,6 +105,30 @@ | |||
| </div> | |||
| </div> | |||
|
|
|||
| <div class="split-expense--main-block"> | |||
There was a problem hiding this comment.
is this the expense fields block?
| <div class="split-expense--more-actions"> | ||
| <button | ||
| class="btn-outline-secondary split-expense--add-more" | ||
| *ngIf="splitExpensesFormArray.controls.length < 30" |
There was a problem hiding this comment.
We should allow 30th split also right?
There was a problem hiding this comment.
When the split count is 29, this condition will remain true, so the add split button will remain visible, then the user can click on it and the split count will become 30 and the button will disappear
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/app/fyle/split-expense/split-expense.page.scss (1)
273-282: 🧹 Nitpick (assertive)The error message needs some Rajini-style animation, mind it!
Let's add that smooth animation we discussed before to make the error message appear with style!
&--error-message-block { color: $blue-black; border-radius: 4px; background-color: $pale-pink; font-size: 12px; padding: 8px 12px; line-height: 15px; display: flex; align-items: center; margin-top: 3px; border: 1px solid $red-1; + animation: slideIn 0.3s ease-out; } + +@keyframes slideIn { + from { + opacity: 0; + transform: translateY(-10px); + } + to { + opacity: 1; + transform: translateY(0); + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/app/fyle/split-expense/split-expense.page.scss(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (12.x)
🔇 Additional comments (6)
src/app/fyle/split-expense/split-expense.page.scss (6)
45-46: Mind-blowing spacing adjustments, partner!The top bar padding and gap improvements create a more balanced layout. The added bottom padding in the main block ensures proper content separation.
Also applies to: 58-58
88-105: Superstar layout structure, mind it!The new left content and icon container classes create a perfectly balanced layout, like all things should be!
217-220: Now this is what I call a box-shadow, macha!The more actions section has perfect spacing with that powerful shadow effect. Simply superb!
227-229: Perfectly centered actions, just like my punch dialogues!The action buttons are well-structured with proper spacing and icon dimensions.
Also applies to: 235-236, 241-242
249-255: That gradient background is like my signature move!The amount block redesign with the pink gradient looks stylish. But remember, with great style comes great responsibility!
286-289: Error icon dimensions are perfect, like my timing in action scenes!The error icon spacing and dimensions are well-balanced.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/app/fyle/split-expense/split-expense.page.scss (2)
78-81: 🧹 Nitpick (assertive)Style is the way of life, but let's add some Rajini-style flair!
The card styling looks sharp with that tertiary border and shadow. But listen here, we can make it even more powerful with some animation!
&--card { padding: 16px; margin-top: 18px; border-radius: 8px; border: 1px solid $border-tertiary; box-shadow: 0px 0px 4px 2px $shadow-xs; + transition: transform 0.2s ease; + + &:hover { + transform: translateY(-2px); + } }
273-282: 🧹 Nitpick (assertive)Let's make those error messages enter with style!
The error message styling looks good, but let's add that smooth entrance animation we talked about earlier!
&--error-message-block { color: $blue-black; border-radius: 4px; background-color: $pale-pink; font-size: 12px; padding: 8px 12px; line-height: 15px; display: flex; align-items: center; margin-top: 3px; border: 1px solid $red-1; + animation: slideIn 0.3s ease-out; } + +@keyframes slideIn { + from { + opacity: 0; + transform: translateY(-10px); + } + to { + opacity: 1; + transform: translateY(0); + } +}Also applies to: 286-289
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/app/fyle/split-expense/split-expense.page.scss(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (12.x)
🔇 Additional comments (8)
src/app/fyle/split-expense/split-expense.page.scss (8)
45-46: Mind it! These spacing changes are spot on!The reduced bottom padding and added gap create a perfect rhythm in the top bar, like a well-choreographed dance sequence!
58-58: Superstar style spacing, I tell you!That 24px bottom padding gives the main block the perfect stage presence it deserves!
88-90: Perfect alignment, like my punch dialogues!The card header's center alignment and bottom margin create a powerful visual impact!
92-97: This flex layout is as smooth as my entry scene!The gap and alignment create a perfect balance, like a well-timed action sequence!
227-229: These action buttons are as precise as my stunts!The padding and gap provide excellent touch targets, and the icon sizes are perfectly balanced!
Also applies to: 235-236, 241-242
263-264: Typography that packs a punch!The 16px font size with 20px line height creates perfect readability, like a well-delivered dialogue!
249-255: Let's ensure our amounts are as clear as my dialogues!We need to verify if the pink gradient background provides enough contrast with the text colors.
Run this script to check the contrast ratio:
✅ Verification successful
Mind it! The contrast is perfectly balanced, like my stunts!
The combination of $black-light (#5a5d72) text on the light pink gradient background provides excellent readability. The semi-transparent gradient background ensures the text stands out clearly, making it a style worthy of a superstar! 🌟
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check contrast ratio for amount block # Extract color values from theme rg -g 'colors.scss' '\$pink-gradient-light-2|\$black-light|\$blue-black'Length of output: 314
Script:
#!/bin/bash # Check text color properties in the split-expense component rg -g "split-expense.page.scss" "color:|font-color:" -B 2 -A 2Length of output: 6897
99-105: Let's ensure our icons shine like my sunglasses!The icon container looks stylish, but we need to verify if the $off-white background provides enough contrast for accessibility.
Run this script to check the contrast ratio:
✅ Verification successful
Mind it! The icon container is perfectly styled, like my signature walk! 🕶️
The off-white background (#f5f5f5) with Material Design's SVG icons follows the standard pattern that automatically ensures proper contrast. These SVG icons inherit their color from the text color, which is already optimized for accessibility in Material Design. Just like how I make my grand entries with style, these icons will stand out perfectly against the light background!
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check contrast ratio between background and icon colors # Extract color values from theme rg -g 'colors.scss' '\$off-white:|icon.*color'Length of output: 98
Script:
#!/bin/bash # Search for icon-related styling in the component and its related files rg -g '*.scss' -g '*.html' -g '*.ts' 'icon|svg' src/app/fyle/split-expense/ # Also check for any global icon styling patterns rg -g '*.scss' 'icon' src/theme/Length of output: 3307
Script:
#!/bin/bash # Check Material theme and global SVG styling rg -g '*.scss' -g '*.css' 'mat-icon|svg' src/theme/ rg -g '*.scss' -g '*.css' 'mat-icon|svg' src/styles/ # Check the shared icon module for any default colors rg -g '*.ts' -g '*.scss' . src/app/shared/icon/Length of output: 7796
| &--split-count-icon { | ||
| width: 20px; | ||
| height: 20px; |
There was a problem hiding this comment.
Watch out! There's a typo in your dimensions, macha!
The height value has a spelling mistake that could break the layout.
&--split-count-icon {
width: 20px;
- height: 2opx;
+ height: 20px;
}Also applies to: 121-123
| padding: 16px; | ||
| justify-content: space-between; | ||
| gap: 16px; | ||
| box-shadow: 0px 2px 10px 0px $shadow-lg; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
The shadow is too heavy, like a villain's presence!
Let's make it more subtle to maintain the visual hierarchy.
&--more-actions {
display: flex;
align-items: center;
padding: 16px;
justify-content: space-between;
gap: 16px;
- box-shadow: 0px 2px 10px 0px $shadow-lg;
+ box-shadow: 0px 1px 4px 0px $shadow-xs;
}📝 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.
| padding: 16px; | |
| justify-content: space-between; | |
| gap: 16px; | |
| box-shadow: 0px 2px 10px 0px $shadow-lg; | |
| padding: 16px; | |
| justify-content: space-between; | |
| gap: 16px; | |
| box-shadow: 0px 1px 4px 0px $shadow-xs; |
|
Clickup
https://app.clickup.com/t/86cxubdxn
UI Preview
Summary by CodeRabbit
New Features
Bug Fixes
Style