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.

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: add new review-split-expense template #3463

Open
wants to merge 8 commits into
base: feat_split_expense
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<ion-header mode="md" class="review-split-expense-modal">
<ion-toolbar>
<div class="review-split-expense-modal--header">
<ion-buttons>
<ion-button (click)="close()">
<mat-icon class="fy-icon-close" svgIcon="cross"></mat-icon>
</ion-button>
</ion-buttons>
<ion-title>Review split expenses</ion-title>
</div>
</ion-toolbar>
</ion-header>
Sishhhh marked this conversation as resolved.
Show resolved Hide resolved

<ion-content>
<ng-container>
<div *ngFor="let expense of splitExpenses; let i = index" class="review-split-expense-modal--body">
<app-expense-card-v2
[expense]="expense"
[expenseIndex]="i"
[showDt]="false"
(goToTransaction)="goToTransaction($event)"
>
</app-expense-card-v2>
<div class="review-split-expense-modal--divier"></div>
Sishhhh marked this conversation as resolved.
Show resolved Hide resolved
</div>
</ng-container>
</ion-content>
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
@import '../../../../theme/colors.scss';

.review-split-expense-modal {
border-bottom: 1px solid $grey;
margin-bottom: 8px;

&--header {
display: flex;
justify-content: flex-start;
align-items: center;
font-size: 18px;
font-weight: 500;
line-height: 27px;
Sishhhh marked this conversation as resolved.
Show resolved Hide resolved
padding-inline: 12px;
}

&--body {
Sishhhh marked this conversation as resolved.
Show resolved Hide resolved
padding-inline: 16px;
}

&--divier {
Sishhhh marked this conversation as resolved.
Show resolved Hide resolved
height: 1px;
width: 100%;
margin-bottom: 6px;
background-color: $grey;
}
Sishhhh marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add the screenshot of the code coverage report?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2025-02-06 at 2 35 56 PM

import { ModalController } from '@ionic/angular';
import { ReviewSplitExpenseComponent } from './review-split-expense.component';
import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core';
import { expenseData } from 'src/app/core/mock-data/platform/v1/expense.data';

describe('ReviewSplitExpenseComponent', () => {
let component: ReviewSplitExpenseComponent;
let fixture: ComponentFixture<ReviewSplitExpenseComponent>;
let modalControllerSpy: jasmine.SpyObj<ModalController>;

beforeEach(waitForAsync(() => {
const modalSpy = jasmine.createSpyObj('ModalController', ['dismiss']);

TestBed.configureTestingModule({
declarations: [ReviewSplitExpenseComponent],
providers: [{ provide: ModalController, useValue: modalSpy }],
schemas: [CUSTOM_ELEMENTS_SCHEMA],
}).compileComponents();

fixture = TestBed.createComponent(ReviewSplitExpenseComponent);
component = fixture.componentInstance;
modalControllerSpy = TestBed.inject(ModalController) as jasmine.SpyObj<ModalController>;

fixture.detectChanges();
}));

it('should create', () => {
expect(component).toBeTruthy();
});

it('should initialize with split expenses input', () => {
Sishhhh marked this conversation as resolved.
Show resolved Hide resolved
const mockExpenses = [expenseData];
component.splitExpenses = mockExpenses;
fixture.detectChanges();

expect(component.splitExpenses).toEqual(mockExpenses);
});

it('should call modalController.dismiss with correct params on goToTransaction', () => {
const event = { expense: expenseData, expenseIndex: 0 };

component.goToTransaction(event);

expect(modalControllerSpy.dismiss).toHaveBeenCalledWith({
dismissed: true,
action: 'navigate',
expense: expenseData,
});
});

it('should call modalController.dismiss with correct params on close', () => {
component.close();

expect(modalControllerSpy.dismiss).toHaveBeenCalledWith({
dismissed: true,
action: 'close',
});
});
Sishhhh marked this conversation as resolved.
Show resolved Hide resolved
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { Component, Input } from '@angular/core';
import { Expense } from 'src/app/core/models/platform/v1/expense.model';
import { ModalController } from '@ionic/angular';
@Component({
selector: 'app-review-split-expense',
templateUrl: './review-split-expense.component.html',
styleUrls: ['./review-split-expense.component.scss'],
})
export class ReviewSplitExpenseComponent {
@Input() splitExpenses: Expense[];

constructor(private modalController: ModalController) {}

goToTransaction(event: { expense: Expense; expenseIndex: number }): void {
Sishhhh marked this conversation as resolved.
Show resolved Hide resolved
this.modalController.dismiss({
dismissed: true,
action: 'navigate',
expense: event.expense,
});
}
Sishhhh marked this conversation as resolved.
Show resolved Hide resolved

close(): void {
this.modalController.dismiss({
dismissed: true,
action: 'close',
});
}
}
3 changes: 3 additions & 0 deletions src/app/shared/shared.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ import { CardDetailComponent } from './components/spent-cards/card-detail/card-d
import { MaskNumber } from './pipes/mask-number.pipe';
import { PolicyViolationActionComponent } from './components/fy-policy-violation/policy-violation-action/policy-violation-action.component';
import { SplitExpensePolicyViolationComponent } from './components/split-expense-policy-violation/split-expense-policy-violation.component';
import { ReviewSplitExpenseComponent } from './components/review-split-expense/review-split-expense.component';
import { PolicyViolationRuleComponent } from './components/policy-violation-rule/policy-violation-rule.component';
import { FyCurrencyComponent } from './components/fy-currency/fy-currency.component';
import { FyCurrencyChooseCurrencyComponent } from './components/fy-currency/fy-currency-choose-currency/fy-currency-choose-currency.component';
Expand Down Expand Up @@ -249,6 +250,7 @@ import { CCExpenseMerchantInfoModalComponent } from './components/cc-expense-mer
FyPolicyViolationComponent,
PolicyViolationActionComponent,
SplitExpensePolicyViolationComponent,
ReviewSplitExpenseComponent,
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Hey thalaiva, don't forget to export your powerful component!

The ReviewSplitExpenseComponent needs to be added to the exports array if it will be used in other modules.

Add the component to the exports array:

  exports: [
+   ReviewSplitExpenseComponent,
    // ... other exports
  ],
📝 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
ReviewSplitExpenseComponent,
@NgModule({
declarations: [
// ... other declarations
ReviewSplitExpenseComponent,
// ... possibly more declarations
],
exports: [
ReviewSplitExpenseComponent,
// ... other exports
],
imports: [
// ... imports
]
})
export class SharedModule { }

PolicyViolationRuleComponent,
FyCurrencyComponent,
FyCurrencyChooseCurrencyComponent,
Expand Down Expand Up @@ -420,6 +422,7 @@ import { CCExpenseMerchantInfoModalComponent } from './components/cc-expense-mer
DashboardOptInComponent,
MobileNumberCardComponent,
CCExpenseMerchantInfoModalComponent,
ReviewSplitExpenseComponent,
],
providers: [DecimalPipe, DatePipe, HumanizeCurrencyPipe, ImagePicker, FyCurrencyPipe, ReportState, ExactCurrencyPipe],
})
Expand Down
10 changes: 10 additions & 0 deletions src/global.scss
Original file line number Diff line number Diff line change
Expand Up @@ -912,6 +912,16 @@ ion-modal.fy-modal {
}
}

ion-modal.review-split-expense-modal {
&::part(content) {
border-radius: 0 !important;
max-height: 100% !important;
position: absolute;
top: 0 !important;
bottom: 0 !important;
}
}
Comment on lines +915 to +923
Copy link

@coderabbitai coderabbitai bot Feb 5, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Modal Style – A blockbuster design!
My friend, the new styles for ion-modal.review-split-expense-modal are absolutely superstar—ensuring the modal spans the full height of the screen. Just double-check to ensure these overrides (with !important) play well with other modals in the app. Small tweaks now could save troubles later.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Sishhhh, can you check why there's some space at the bottom?

Copy link

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Dimple, that was due to the showBackDrop set as true when we opened the modal from the split-expense page. Setting it to false will remove the extra space from the bottom.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2025-02-06 at 2 53 25 PM


ion-modal.share-report-modal {
&::part(content) {
border-radius: 16px 16px 0 0 !important;
Expand Down