Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
64 changes: 38 additions & 26 deletions src/app/features/customer/booking/pages/cars-page/cars-page.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import {
AfterRenderRef,
ChangeDetectionStrategy,
Component,
EnvironmentInjector,
ViewChild,
computed,
DestroyRef,
inject,
runInInjectionContext,
signal,
afterNextRender,
} from '@angular/core';
Expand Down Expand Up @@ -45,6 +48,8 @@ export class CarsPage {
private readonly route = inject(ActivatedRoute);
private readonly router = inject(Router);
private readonly destroyRef = inject(DestroyRef);
private readonly environmentInjector = inject(EnvironmentInjector);
private pendingRenderRef: AfterRenderRef | null = null;

readonly steps: BookingStep[] = [
{ key: 'details', label: 'Thông tin xe' },
Expand Down Expand Up @@ -98,6 +103,11 @@ export class CarsPage {

this.setActiveStep(targetIndex, { updateFragment: false });
});

this.destroyRef.onDestroy(() => {
this.pendingRenderRef?.destroy();
this.pendingRenderRef = null;
});
}

private handlePaymentReturn(): void {
Expand Down Expand Up @@ -163,11 +173,9 @@ export class CarsPage {
if (currentIndex === 1 && !this.paymentStarted()) {
return;
}

if (currentIndex >= this.steps.length - 1) {
return;
}
Comment on lines 176 to 178

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] Blank lines removed between validation checks reduce readability. Consider keeping the blank lines to visually separate the different validation conditions for better code maintainability.

Copilot uses AI. Check for mistakes.

this.setActiveStep(currentIndex + 1);
}

Expand All @@ -194,39 +202,43 @@ export class CarsPage {
const previousIndex = this.activeStep();
const boundedIndex = Math.max(0, Math.min(this.steps.length - 1, nextIndex));
const shouldUpdateFragment = options?.updateFragment !== false;
const shouldResetPayment = boundedIndex < previousIndex && boundedIndex <= 1;
const targetIsStart = boundedIndex === 0;

this.activeStep.set(boundedIndex);

if (boundedIndex === 0) {
this.paymentStarted.set(false);
}
if (boundedIndex < previousIndex && boundedIndex <= 1) {
this.paymentStarted.set(false);
}
this.pendingRenderRef?.destroy();

const renderRef = afterNextRender({
read: () => {
if (!this.matStepper) {
return;
}
runInInjectionContext(this.environmentInjector, () => {
this.pendingRenderRef = afterNextRender({
write: () => {
if (!this.matStepper) {
return;
}

const desiredIndex = this.activeStep();
this.matStepper.selectedIndex = desiredIndex;
const desiredIndex = this.activeStep();
this.matStepper.selectedIndex = desiredIndex;

const actualIndex = this.matStepper.selectedIndex;
if (shouldUpdateFragment) {
const key = this.steps[actualIndex]?.key;
if (key) {
this.updateFragment(key);
const actualIndex = this.matStepper.selectedIndex;
if (shouldUpdateFragment || actualIndex !== desiredIndex) {

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

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

The condition for updating the fragment has changed behavior. Previously, the fragment would only be updated when shouldUpdateFragment was true. Now it also updates when actualIndex !== desiredIndex, even if updateFragment: false was explicitly passed. This could cause unintended fragment updates when the stepper index reconciliation happens, potentially conflicting with the updateFragment: false option used in the fragment subscriber (line 104).

Suggested change
if (shouldUpdateFragment || actualIndex !== desiredIndex) {
if (shouldUpdateFragment) {

Copilot uses AI. Check for mistakes.
const key = this.steps[actualIndex]?.key;
if (key) {
this.updateFragment(key);
}
}
}

if (actualIndex !== desiredIndex) {
this.activeStep.set(actualIndex);
}
},
if (actualIndex !== desiredIndex) {
this.activeStep.set(actualIndex);
}

if (targetIsStart || (shouldResetPayment && actualIndex <= 1)) {

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

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

The payment reset logic has changed. Previously, paymentStarted would be reset to false when boundedIndex <= 1 and moving backward. Now it checks actualIndex <= 1 instead. This means if the stepper cannot navigate to the desired index (e.g., validation prevents it), the payment state might not be reset as expected. Consider using boundedIndex <= 1 in the condition to maintain the original behavior.

Suggested change
if (targetIsStart || (shouldResetPayment && actualIndex <= 1)) {
if (targetIsStart || (shouldResetPayment && boundedIndex <= 1)) {

Copilot uses AI. Check for mistakes.
this.paymentStarted.set(false);
}

this.pendingRenderRef = null;
},
});
});
this.destroyRef.onDestroy(() => renderRef.destroy());
}

private updateFragment(step: StepKey): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,15 +160,15 @@ <h2 class="checkout-card__title flex items-center gap-2">

<!-- Actions -->
<div class="checkout-actions flex gap-4 justify-end">
<button
<!-- <button
matButton="outlined"
type="button"
(click)="previousStep.emit()"
[disabled]="isLoading()"
aria-label="Quay lại"
>
Quay lại
</button>
</button> -->
Comment on lines +163 to +171

Copilot AI Nov 12, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] Consider removing the commented-out code instead of leaving it in the codebase. Commented code can cause confusion and clutter. If you need to preserve this for reference, rely on version control history instead.

Copilot uses AI. Check for mistakes.
<button
matButton="filled"
type="button"
Expand Down
Loading