feat: initial template for onboarding home page #3383
Conversation
WalkthroughSuperstar, listen up! 🌟 We've just unleashed a powerful new spender onboarding feature that's going to revolutionize user experience! The changes introduce a complete routing and page setup for the spender onboarding process, with a sleek routing module, a stylish page component, and a killer design that'll make users go "Wow!" The implementation includes lazy loading, user-specific greetings, and a smooth loading mechanism that's as smooth as my signature style! Changes
Possibly related PRs
Suggested Labels
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
src/assets/svg/progress-bar.svgis excluded by!**/*.svg
📒 Files selected for processing (6)
src/app/fyle/fyle-routing.module.ts(1 hunks)src/app/fyle/spender-onboarding/spender-onboarding-routing.module.ts(1 hunks)src/app/fyle/spender-onboarding/spender-onboarding.module.ts(1 hunks)src/app/fyle/spender-onboarding/spender-onboarding.page.html(1 hunks)src/app/fyle/spender-onboarding/spender-onboarding.page.scss(1 hunks)src/app/fyle/spender-onboarding/spender-onboarding.page.ts(1 hunks)
🔇 Additional comments (1)
src/app/fyle/fyle-routing.module.ts (1)
49-52: Route naming style mismatch, kabali!
Other routes use underscore, but Angular best practices suggest using hyphens. Let's be consistent with the existing pattern though!
Let's verify the route naming convention:
| isLoading = true; | ||
|
|
||
| userFullName: string; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Make these properties as strong as my style!
Define proper types for your properties, partner! TypeScript is your friend like my fans are to me!
- isLoading = true;
+ isLoading: boolean = true;
- userFullName: string;
+ userFullName: string = '';📝 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.
| isLoading = true; | |
| userFullName: string; | |
| isLoading: boolean = true; | |
| userFullName: string = ''; |
| slot="icon-only" | ||
| ></ion-icon> | ||
| </div> | ||
| <div class="spender-onboarding__skip-cta" (click)="skipConnectCardOnboardingStep()">Skip</div> |
There was a problem hiding this comment.
Mind it! Skip action needs confirmation, machan!
Skipping onboarding is a crucial decision. Let's add a confirmation dialog to prevent accidental skips.
-<div class="spender-onboarding__skip-cta" (click)="skipConnectCardOnboardingStep()">Skip</div>
+<div class="spender-onboarding__skip-cta" (click)="confirmSkip()">Skip</div>Committable suggestion skipped: line range outside the PR's diff.
| @@ -0,0 +1,29 @@ | |||
| <div class="spender-onboarding" *ngIf="!isLoading"> | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Mind blowing! But where's the loading indicator, partner?
When isLoading is true, the user sees nothing! Let's show them some style with a loading spinner, shall we?
-<div class="spender-onboarding" *ngIf="!isLoading">
+<ion-content>
+ <ion-spinner *ngIf="isLoading" class="spender-onboarding__spinner"></ion-spinner>
+ <div class="spender-onboarding" *ngIf="!isLoading">Committable suggestion skipped: line range outside the PR's diff.
| &__user-greeting { | ||
| display: flex; | ||
| gap: 12px; | ||
| align-items: flex-start; | ||
| color: $dark-grey; | ||
| line-height: 1.4; | ||
| padding: 0 24px; | ||
| margin: 12px 0 32px 0; | ||
| height: 42px; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Make it responsive, superstar!
Add media queries for better tablet and desktop experience.
&__user-greeting {
display: flex;
gap: 12px;
align-items: flex-start;
color: $dark-grey;
line-height: 1.4;
padding: 0 24px;
margin: 12px 0 32px 0;
height: 42px;
+ @media (min-width: 768px) {
+ padding: 0 48px;
+ margin: 24px 0 48px 0;
+ }
}📝 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.
| &__user-greeting { | |
| display: flex; | |
| gap: 12px; | |
| align-items: flex-start; | |
| color: $dark-grey; | |
| line-height: 1.4; | |
| padding: 0 24px; | |
| margin: 12px 0 32px 0; | |
| height: 42px; | |
| } | |
| &__user-greeting { | |
| display: flex; | |
| gap: 12px; | |
| align-items: flex-start; | |
| color: $dark-grey; | |
| line-height: 1.4; | |
| padding: 0 24px; | |
| margin: 12px 0 32px 0; | |
| height: 42px; | |
| @media (min-width: 768px) { | |
| padding: 0 48px; | |
| margin: 24px 0 48px 0; | |
| } | |
| } |
|
|
||
| &__option { | ||
| font-weight: normal; | ||
| color: #0077ee; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hardcoded color alert! Use variables, thalaiva!
Replace hardcoded color with theme variable.
- color: #0077ee;
+ color: $primary-color;📝 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.
| color: #0077ee; | |
| color: $primary-color; |
| $item-border-color: #e0e0e0; | ||
| $background-color: #fff; | ||
| $menu-botton-color: #000; | ||
| $toolbar-border: #ababab6b; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Colors should be in theme file, style-u!
Move these color variables to the theme file for better maintainability and consistency.
These color variables should be moved to src/theme/colors.scss:
-$item-border-color: #e0e0e0;
-$background-color: #fff;
-$menu-botton-color: #000;
-$toolbar-border: #ababab6b;Committable suggestion skipped: line range outside the PR's diff.
|
bistaastha
merged commit b003ae1
into
FYLE-86cx2t82k-base-feature-branch
Preview:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation