Skip to content
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.

Sign up for GitHub

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: initial template for onboarding home page #3383

Merged
merged 1 commit into from
Dec 22, 2024
Merged

feat: initial template for onboarding home page #3383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/app/fyle/fyle-routing.module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ const routes: Routes = [
path: 'add_edit_expense',
loadChildren: () => import('./add-edit-expense/add-edit-expense.module').then((m) => m.AddEditExpensePageModule),
},
{
path: 'spender_onboarding',
loadChildren: () =>
import('./spender-onboarding/spender-onboarding.module').then((m) => m.SpenderOnboardingPageModule),
},
{
path: 'team_reports',
loadChildren: () => import('./team-reports/team-reports.module').then((m) => m.TeamReportsPageModule),
Expand Down
16 changes: 16 additions & 0 deletions src/app/fyle/spender-onboarding/spender-onboarding-routing.module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { NgModule } from '@angular/core';
import { Routes, RouterModule } from '@angular/router';
import { SpenderOnboardingPage } from './spender-onboarding.page';

const routes: Routes = [
{
path: '',
component: SpenderOnboardingPage,
},
];
bistaastha marked this conversation as resolved.
Show resolved Hide resolved

@NgModule({
imports: [RouterModule.forChild(routes)],
exports: [RouterModule],
})
export class SpenderOnboardingRoutingModule {}
14 changes: 14 additions & 0 deletions src/app/fyle/spender-onboarding/spender-onboarding.module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { NgModule } from '@angular/core';
import { SharedModule } from 'src/app/shared/shared.module';
import { SpenderOnboardingPage } from './spender-onboarding.page';
import { IonicModule } from '@ionic/angular';
import { FormsModule } from '@angular/forms';
import { CommonModule } from '@angular/common';
import { SpenderOnboardingRoutingModule } from './spender-onboarding-routing.module';
import { MatButtonModule } from '@angular/material/button';
bistaastha marked this conversation as resolved.
Show resolved Hide resolved

@NgModule({
imports: [SharedModule, CommonModule, FormsModule, IonicModule, MatButtonModule, SpenderOnboardingRoutingModule],
declarations: [SpenderOnboardingPage],
})
export class SpenderOnboardingPageModule {}
29 changes: 29 additions & 0 deletions src/app/fyle/spender-onboarding/spender-onboarding.page.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<div class="spender-onboarding" *ngIf="!isLoading">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

<ion-buttons slot="start" class="spender-onboarding__menu-icon-container">
<app-fy-menu-icon class="spender-onboarding__menubutton" auto-hide="false"></app-fy-menu-icon>
</ion-buttons>
<div class="spender-onboarding__user-greeting">
<div class="spender-onboarding__wave">👋</div>
<div class="spender-onboarding_user-greeting-text">
<div class="spender-onboarding__user-name">Hello {{userFullName}},</div>
<div class="spender-onboarding__content">Let’s get started!</div>
</div>
</div>
<div class="spender-onboarding__step-tracker">
<div class="spender-onboarding__stepper-container">
<div>
<ion-icon
class="spender-onboarding__progress-bar"
[src]="'/assets/svg/progress-bar.svg'"
slot="icon-only"
></ion-icon>
<ion-icon
class="spender-onboarding__progress-bar spender-onboarding__progress-bar-right"
[src]="'/assets/svg/progress-bar.svg'"
slot="icon-only"
></ion-icon>
bistaastha marked this conversation as resolved.
Show resolved Hide resolved
</div>
<div class="spender-onboarding__skip-cta" (click)="skipConnectCardOnboardingStep()">Skip</div>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

</div>
</div>
</div>
104 changes: 104 additions & 0 deletions src/app/fyle/spender-onboarding/spender-onboarding.page.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
@import '../../../theme/colors.scss';
$item-border-color: #e0e0e0;
$background-color: #fff;
$menu-botton-color: #000;
$toolbar-border: #ababab6b;
Comment on lines +2 to +5
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.


.spender-onboarding {
background: $border-info-lighter;
height: 100%;

&__progress-bar {
width: 50px;
}

&__progress-bar-right {
margin-left: 4px;
}

&__menu-icon-container {
padding: 20px 16px;
}

&__step-tracker {
padding: 24px;
border-radius: 32px 32px 0px 0px;
border: 1px solid $border-info-lighter;
background: $pure-white;
box-shadow: -14px -6px 24px 0px #00000008;
}

&__stepper-container {
display: flex;
justify-content: space-between;
align-items: center;
}

&__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;
}
Comment on lines +37 to +46
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.

Suggested change
&__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;
}
}


&__wave {
font-size: 32px;
align-self: center;
}

&__user-name {
font-size: 16px;
font-weight: 500;
}

&__content {
font-size: 14px;
font-weight: 400;
}

&__menubutton {
color: $menu-botton-color;
}

&__title {
line-height: 1.3;
color: $menu-botton-color;
}

&__zero-state {
height: 100%;
display: flex;
align-items: center;
justify-content: center;
}

&__option {
font-weight: normal;
color: #0077ee;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
color: #0077ee;
color: $primary-color;

font-size: 16px;
}

&__select-state {
display: flex;
justify-content: flex-start;
align-items: center;
min-width: 120px;
}

&__select-state-checkbox {
margin-right: 8px;
}

&__skip-cta {
color: $blue-black;
text-align: center;
font-size: 14px;
font-style: normal;
font-weight: 500;
line-height: 1.3;
}
}
Empty file added src/app/fyle/spender-onboarding/spender-onboarding.page.spec.ts
View file
Open in desktop
Empty file.
32 changes: 32 additions & 0 deletions src/app/fyle/spender-onboarding/spender-onboarding.page.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { Component } from '@angular/core';
import { from, map, switchMap } from 'rxjs';
import { ExtendedOrgUser } from 'src/app/core/models/extended-org-user.model';
import { LoaderService } from 'src/app/core/services/loader.service';
import { OrgUserService } from 'src/app/core/services/org-user.service';

@Component({
selector: 'app-spender-onboarding',
templateUrl: './spender-onboarding.page.html',
styleUrls: ['./spender-onboarding.page.scss'],
})
export class SpenderOnboardingPage {
isLoading = true;

userFullName: string;
Comment on lines +13 to +15
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.

Suggested change
isLoading = true;
userFullName: string;
isLoading: boolean = true;
userFullName: string = '';


constructor(private loaderService: LoaderService, private orgUserService: OrgUserService) {}

ionViewWillEnter(): void {
this.isLoading = true;
from(this.loaderService.showLoader())
.pipe(
switchMap(() => this.orgUserService.getCurrent()),
map((eou: ExtendedOrgUser) => {
this.userFullName = eou.us.full_name;
this.isLoading = false;
this.loaderService.hideLoader();
})
)
.subscribe();
}
bistaastha marked this conversation as resolved.
Show resolved Hide resolved
}
5 changes: 5 additions & 0 deletions src/assets/svg/progress-bar.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading