refactor: migrate cart payment modals to shared app-dialog primitive

Third attempt, done properly this time - first two were reverted (one stopped cleanly on real conflicts, one botched sequencing and deleted the old focus-trap before finishing the swap). DialogComponent gains closeOnEscape/closeOnBackdropClick (default true, backward-compatible with its 13 other call sites) and ariaLabel (for dialogs with no visible title header). FOCUSABLE_SELECTOR now includes iframe for the bank-payment panel's focus trap. Cart wires closeOnBackdropClick=false on both dialogs (in-flight payment shouldn't cancel on a stray click) and closeOnEscape tied to the bank popup's open state, so Escape closes the nested bank iframe first and falls back to the QR view - matches the original priority exactly. Original geometry (500px QR modal/40px padding, 960x760 bank modal/ 56-16-16 padding, both mobile breakpoints) preserved via :host ::ng-deep overrides scoped per dialog instance - same pattern already used by product-carousel-widget.component.ts. cart.component.ts loses ~90 lines of hand-rolled ViewChild/HostListener/ focus-trap code - app-dialog owns all of it now. Verified live in browser: dialog sizing/padding/aria-label correct at mobile+desktop, backdrop-click confirmed inert, Escape-priority confirmed (bank closes first, then QR), initial focus lands on close button. 83/83 tests pass, tsc/build clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-06 11:13:33 +04:00
parent 6d075fc5b9
commit 7a2f2a452f
7 changed files with 93 additions and 163 deletions
--- a/docs/FUTURE_FEATURES.md
+++ b/docs/FUTURE_FEATURES.md
@@ -4,15 +4,13 @@ Nice-to-have, non-blocking work — no client decision needed, just not worth do

 ## Cart payment modal → `app-dialog` migration

-`.payment-modal`/`.bank-payment-modal` on the cart page are custom overlays with their own focus-trap (added during the WCAG audit) rather than the shared `app-dialog` primitive. Functionally and accessibly complete as-is. Attempted 2026-08-06 and deliberately stopped, not just deferred — real conflicts found, not reluctance:
+**Done, 2026-08-06.** `.payment-modal`/`.bank-payment-modal` on the cart page now render through the shared `app-dialog` primitive instead of hand-rolled overlays. Two earlier same-session attempts were reverted before landing (one stopped cleanly after finding real conflicts, one botched the sequencing — deleted the old focus-trap before finishing the swap); this pass fixed the actual API gaps first, then migrated, then verified live in a browser before shipping:

- Cart's modals have **no backdrop-click-to-close** today (intentional — an in-flight QR/bank payment shouldn't cancel on a stray click); `app-dialog` always closes on backdrop click, no opt-out existed.
- The QR/status modal and the bank-iframe modal can be **open simultaneously** (nested), with Escape closing only the top one and falling back to the QR view underneath, payment polling untouched. `app-dialog` has no stacking/priority concept — two instances would both react to one Escape.
- The bank iframe needs a full-bleed `min(960px,92vw)×min(760px,86vh)` panel with zero padding; `app-dialog`'s largest preset caps at 800px with fixed `1.5rem` padding.
-
-`app-dialog` was extended with `closeOnEscape`/`closeOnBackdropClick` inputs (default `true`, backward-compatible with its other 13 call sites) to close the first two gaps, but the actual cart-template migration was reverted before landing — a same-session follow-up attempt deleted the hand-rolled focus-trap/Escape-key code from `cart.component.ts` without finishing the `cart.component.html` swap to `app-dialog`, which would have shipped live checkout code with no keyboard focus-trap at all. Caught before commit, fully reverted (`dialog.component.ts`, `cart.component.ts`, `cart.component.html` all back to original — the `closeOnEscape`/`closeOnBackdropClick` API extension did not ship either, since it had no consumer left).
-
-Next attempt should land the `DialogComponent` API extension as its own small, independently-reviewed change first, then do the cart template migration as a second, separate, carefully-tested change — not both in one pass.
+- `DialogComponent` gained `closeOnEscape`/`closeOnBackdropClick` inputs (default `true`, backward-compatible with its other 13 call sites) and an `ariaLabel` input (for dialogs with no visible title header — cart's modals render their own close button in content instead). `FOCUSABLE_SELECTOR` now includes `iframe` (needed for the bank-payment panel's focus trap).
+- Cart wires `[closeOnBackdropClick]="false"` on both dialogs (in-flight payment shouldn't cancel on a stray click) and `[closeOnEscape]="!showBankPaymentPopup()"` on the QR/status dialog (so Escape closes the bank iframe first, falls back to the QR view, matches the original nested-modal priority).
+- Exact original geometry (500px QR modal, 40px padding; 960×760 bank iframe modal, 56/16/16 padding, both mobile breakpoints) preserved via `:host ::ng-deep` overrides on `.app-dialog-panel`/`.app-dialog-panel__body`/`.app-dialog-backdrop`, scoped per-instance via `.payment-dialog`/`.bank-payment-dialog` host classes — same `::ng-deep` pattern already used by `product-carousel-widget.component.ts`.
+- `cart.component.ts` lost its hand-rolled `@ViewChild`/`@HostListener`/focus-trap methods (~90 lines) — `app-dialog` owns all of that now.
+- Verified live: both dialogs render at correct size/padding/aria-label at mobile and desktop breakpoints, backdrop-click confirmed inert, Escape-priority confirmed (closes bank first, then QR), initial focus confirmed landing on the close button. 83/83 tests pass, tsc/build clean.

 ## Angular 22 upgrade

--- a/src/app/pages/cart/cart.component.html
+++ b/src/app/pages/cart/cart.component.html
@@ -195,9 +195,15 @@
 </div>

 <!-- Payment Popup Modal -->
-@if (showPaymentPopup()) {
-  <div class="payment-modal-overlay">
-    <div class="payment-modal" #paymentModalPanel role="dialog" aria-modal="true" [attr.aria-label]="'cart.checkout' | translate" tabindex="-1">
+<app-dialog
+  class="payment-dialog"
+  [open]="showPaymentPopup()"
+  size="md"
+  [ariaLabel]="'cart.checkout' | translate"
+  [closeOnBackdropClick]="false"
+  [closeOnEscape]="!showBankPaymentPopup()"
+  (closed)="closePaymentPopup()"
+>
      <button class="close-modal-btn" (click)="closePaymentPopup()" [attr.aria-label]="'cart.close' | translate">
        <app-icon name="x" [size]="20" />
      </button>
@@ -276,19 +282,22 @@
          <p class="auto-close">{{ 'cart.autoClose' | translate }}</p>
        </div>
      }
-    </div>
+</app-dialog>

@if (showBankPaymentPopup() && bankPaymentFrameUrl()) {
-      <div class="bank-payment-modal-overlay">
-        <div class="bank-payment-modal" #bankPaymentModalPanel role="dialog" aria-modal="true" [attr.aria-label]="'cart.payViaCard' | translate" tabindex="-1">
+  <app-dialog
+    class="bank-payment-dialog"
+    [open]="showBankPaymentPopup()"
+    size="lg"
+    [ariaLabel]="'cart.payViaCard' | translate"
+    [closeOnBackdropClick]="false"
+    (closed)="closeBankPaymentPopup()"
+  >
    <button class="close-modal-btn" (click)="closeBankPaymentPopup()" [attr.aria-label]="'cart.close' | translate">
      <app-icon name="x" [size]="20" />
    </button>
    <iframe [src]="bankPaymentFrameUrl()" class="bank-payment-frame" [attr.title]="'common.bankCardPayment' | translate"></iframe>
-        </div>
-      </div>
-    }
-  </div>
+  </app-dialog>
 }

 <app-telegram-login />
--- a/src/app/pages/cart/cart.component.scss
+++ b/src/app/pages/cart/cart.component.scss
@@ -640,32 +640,20 @@
  }
 }

-// Payment modal styles
-.payment-modal-overlay {
-  position: fixed;
-  top: 0;
-  left: 0;
-  right: 0;
-  bottom: 0;
-  background: rgba(0, 0, 0, 0.6);
-  display: flex;
-  align-items: center;
-  justify-content: center;
-  z-index: 1000;
-  padding: 20px;
+// Payment / bank-payment modals render through the shared app-dialog
+// primitive (focus-trap, Escape, backdrop-click, ARIA all owned there —
+// see closeOnEscape/closeOnBackdropClick usage in the template). These
+// ::ng-deep overrides restore the exact panel geometry the old hand-rolled
+// modals had, which app-dialog's generic size presets/padding don't match.
+:host ::ng-deep {
+  app-dialog.payment-dialog .app-dialog-panel {
+    max-width: 500px;
+    border: 1px solid var(--border-color);
  }

-.payment-modal {
-  background: var(--bg-primary);
-  border-radius: var(--radius-lg, 13px);
-  max-width: 500px;
-  width: 100%;
+  app-dialog.payment-dialog .app-dialog-panel__body {
    padding: 40px;
-  position: relative;
-  max-height: 90vh;
-  overflow-y: auto;
-  border: 1px solid var(--border-color);
-  box-shadow: var(--shadow-lg);
+  }
 }

 // Deduplicated: this rule previously appeared twice (identical) - kept a single definition.
@@ -970,27 +958,25 @@

 }

-.bank-payment-modal-overlay {
-  position: fixed;
-  inset: 0;
-  z-index: 1001;
-  display: flex;
-  align-items: center;
-  justify-content: center;
-  padding: 24px;
+:host ::ng-deep {
+  app-dialog.bank-payment-dialog .app-dialog-backdrop {
    background: rgba(17, 24, 39, 0.55);
  }

-.bank-payment-modal {
-  position: relative;
+  app-dialog.bank-payment-dialog .app-dialog-panel {
    width: min(960px, 92vw);
+    max-width: min(960px, 92vw);
    height: min(760px, 86vh);
-  padding: 56px 16px 16px;
-  background: var(--bg-primary);
-  border-radius: var(--radius-lg, 13px);
    box-shadow: 0 18px 60px rgba(0, 0, 0, 0.28);
  }

+  app-dialog.bank-payment-dialog .app-dialog-panel__body {
+    height: 100%;
+    box-sizing: border-box;
+    padding: 56px 16px 16px;
+  }
+}
+
 .bank-payment-frame {
  width: 100%;
  height: 100%;
@@ -1041,9 +1027,9 @@
    display: flex;
  }

-  .payment-modal {
+  :host ::ng-deep app-dialog.payment-dialog .app-dialog-panel__body {
    padding: 24px;
-    padding-top: 56px; // <20><><EFBFBD><EFBFBD><EFBFBD><EFBFBD><EFBFBD><EFBFBD><EFBFBD><EFBFBD><EFBFBD><EFBFBD><EFBFBD><EFBFBD> <20><><EFBFBD><EFBFBD><EFBFBD><EFBFBD> <20><><EFBFBD><EFBFBD><EFBFBD><EFBFBD> <20><><EFBFBD> <20><><EFBFBD><EFBFBD><EFBFBD><EFBFBD> <20><><EFBFBD><EFBFBD><EFBFBD><EFBFBD><EFBFBD><EFBFBD>
+    padding-top: 56px;
  }

  .close-modal-btn {
@@ -1078,15 +1064,21 @@
    grid-template-columns: 1fr;
  }

-  .bank-payment-modal-overlay {
+  :host ::ng-deep {
+    app-dialog.bank-payment-dialog .app-dialog-backdrop {
      padding: 12px;
    }

-  .bank-payment-modal {
+    app-dialog.bank-payment-dialog .app-dialog-panel {
      width: 94vw;
+      max-width: 94vw;
      height: 82vh;
+    }
+
+    app-dialog.bank-payment-dialog .app-dialog-panel__body {
      padding: 52px 10px 10px;
    }
+  }

  .payment-active h2 {
    font-size: var(--font-size-2xl, 1.25rem);
--- a/src/app/pages/cart/cart.component.ts
+++ b/src/app/pages/cart/cart.component.ts
@@ -1,4 +1,4 @@
-import { Component, ChangeDetectionStrategy, signal, OnDestroy, inject, ElementRef, ViewChild, HostListener, effect } from '@angular/core';
+import { Component, ChangeDetectionStrategy, signal, OnDestroy, inject } from '@angular/core';
 import { DecimalPipe } from '@angular/common';
 import { Router, RouterLink } from '@angular/router';
 import { FormsModule } from '@angular/forms';
@@ -21,15 +21,13 @@ import { ConfigService } from '../../core/config/config.service';
 import { TenantResolverService } from '../../core/config/tenant-resolver.service';
 import { UserNotificationService } from '../../features/website/user-experience/services/user-notification.service';
 import { ConfirmDialogComponent } from '../../shared/ui/confirm-dialog/confirm-dialog.component';
+import { DialogComponent } from '../../shared/ui/dialog/dialog.component';

 type PaymentMethod = 'qr' | 'card';

-const MODAL_FOCUSABLE_SELECTOR =
-  'a[href], button:not([disabled]), textarea:not([disabled]), input:not([disabled]), select:not([disabled]), iframe, [tabindex]:not([tabindex="-1"])';
-
@Component({
  selector: 'app-cart',
-  imports: [DecimalPipe, RouterLink, FormsModule, DeliverySelectorComponent, TelegramLoginComponent, LangRoutePipe, TranslatePipe, IconComponent, EmptyStateComponent, ButtonComponent, ConfirmDialogComponent],
+  imports: [DecimalPipe, RouterLink, FormsModule, DeliverySelectorComponent, TelegramLoginComponent, LangRoutePipe, TranslatePipe, IconComponent, EmptyStateComponent, ButtonComponent, ConfirmDialogComponent, DialogComponent],
  templateUrl: './cart.component.html',
  styleUrls: ['./cart.component.scss'],
  changeDetection: ChangeDetectionStrategy.OnPush
@@ -82,13 +80,6 @@ export class CartComponent implements OnDestroy {
  private configService = inject(ConfigService);
  private tenantResolver = inject(TenantResolverService);

-  // Focus management for the custom payment/bank-payment modals (not
-  // app-dialog — they own a real multi-step state machine). Mirrors
-  // app-dialog's confirmed-correct focus-trap/Escape/return-focus pattern.
-  @ViewChild('paymentModalPanel') private paymentModalPanel?: ElementRef<HTMLElement>;
-  @ViewChild('bankPaymentModalPanel') private bankPaymentModalPanel?: ElementRef<HTMLElement>;
-  private previouslyFocusedBeforeModal: HTMLElement | null = null;
-
  constructor(
    private cartService: CartService,
    private apiService: ApiService,
@@ -103,78 +94,6 @@ export class CartComponent implements OnDestroy {
    this.totalWithDelivery = this.cartService.totalWithDelivery;
    this.hasDeliveryPrice = this.cartService.hasDeliveryPrice;
    this.allRequiredDeliveriesSelected = this.cartService.allRequiredDeliveriesSelected;
-
-    effect(() => {
-      const isOpen = this.showPaymentPopup();
-      if (isOpen) {
-        this.previouslyFocusedBeforeModal ??= document.activeElement as HTMLElement | null;
-        queueMicrotask(() => this.focusActiveModalPanel());
-      } else if (this.previouslyFocusedBeforeModal) {
-        this.previouslyFocusedBeforeModal.focus();
-        this.previouslyFocusedBeforeModal = null;
-      }
-    });
-
-    effect(() => {
-      if (this.showBankPaymentPopup()) {
-        queueMicrotask(() => this.focusActiveModalPanel());
-      }
-    });
-  }
-
-  @HostListener('document:keydown', ['$event'])
-  protected handleModalKeydown(event: KeyboardEvent): void {
-    if (!this.showPaymentPopup()) {
-      return;
-    }
-    if (event.key === 'Escape') {
-      if (this.showBankPaymentPopup()) {
-        this.closeBankPaymentPopup();
-      } else {
-        this.closePaymentPopup();
-      }
-      return;
-    }
-    if (event.key === 'Tab') {
-      this.trapModalFocus(event);
-    }
-  }
-
-  private activeModalPanel(): HTMLElement | undefined {
-    return this.showBankPaymentPopup()
-      ? this.bankPaymentModalPanel?.nativeElement
-      : this.paymentModalPanel?.nativeElement;
-  }
-
-  private focusActiveModalPanel(): void {
-    const panel = this.activeModalPanel();
-    if (!panel) {
-      return;
-    }
-    const focusable = panel.querySelectorAll<HTMLElement>(MODAL_FOCUSABLE_SELECTOR);
-    (focusable[0] ?? panel).focus();
-  }
-
-  private trapModalFocus(event: KeyboardEvent): void {
-    const panel = this.activeModalPanel();
-    if (!panel) {
-      return;
-    }
-    const focusable = Array.from(panel.querySelectorAll<HTMLElement>(MODAL_FOCUSABLE_SELECTOR));
-    if (focusable.length === 0) {
-      return;
-    }
-    const first = focusable[0];
-    const last = focusable[focusable.length - 1];
-    const active = document.activeElement;
-
-    if (event.shiftKey && active === first) {
-      event.preventDefault();
-      last.focus();
-    } else if (!event.shiftKey && active === last) {
-      event.preventDefault();
-      first.focus();
-    }
  }

  requestLogin(): void {
--- a/src/app/shared/ui/dialog/dialog.component.html
+++ b/src/app/shared/ui/dialog/dialog.component.html
@@ -8,7 +8,7 @@
      [class.app-dialog-panel--lg]="size() === 'lg'"
      role="dialog"
      aria-modal="true"
-      [attr.aria-label]="titleText()"
+      [attr.aria-label]="titleText() ?? ariaLabel()"
      tabindex="-1"
      (click)="$event.stopPropagation()"
    >
--- a/src/app/shared/ui/dialog/dialog.component.scss
+++ b/src/app/shared/ui/dialog/dialog.component.scss
@@ -11,6 +11,7 @@
 }

 .app-dialog-panel {
+  position: relative;
  width: 100%;
  max-height: 90vh;
  overflow-y: auto;
--- a/src/app/shared/ui/dialog/dialog.component.ts
+++ b/src/app/shared/ui/dialog/dialog.component.ts
@@ -15,7 +15,7 @@ import { TranslatePipe } from '../../../i18n/translate.pipe';
 export type DialogSize = 'sm' | 'md' | 'lg';

 const FOCUSABLE_SELECTOR =
-  'a[href], button:not([disabled]), textarea:not([disabled]), input:not([disabled]), select:not([disabled]), [tabindex]:not([tabindex="-1"])';
+  'a[href], button:not([disabled]), textarea:not([disabled]), input:not([disabled]), select:not([disabled]), iframe, [tabindex]:not([tabindex="-1"])';

@Component({
  selector: 'app-dialog',
@@ -28,7 +28,14 @@ const FOCUSABLE_SELECTOR =
 export class DialogComponent implements OnChanges, AfterViewInit {
  readonly open = input(false);
  readonly titleText = input<string | null>(null);
+  // Accessible name for dialogs that skip the visible titled header (a
+  // consumer-rendered close button inside the content instead). Ignored
+  // when titleText is set - the header already supplies the name.
+  readonly ariaLabel = input<string | null>(null);
  readonly size = input<DialogSize>('md');
+  // Both default true (existing behavior for every current consumer).
+  readonly closeOnEscape = input(true);
+  readonly closeOnBackdropClick = input(true);

  readonly closed = output<void>();

@@ -60,7 +67,9 @@ export class DialogComponent implements OnChanges, AfterViewInit {
      return;
    }
    if (event.key === 'Escape') {
+      if (this.closeOnEscape()) {
        this.requestClose();
+      }
      return;
    }
    if (event.key === 'Tab') {
@@ -73,8 +82,10 @@ export class DialogComponent implements OnChanges, AfterViewInit {
  }

  protected handleBackdropClick(): void {
+    if (this.closeOnBackdropClick()) {
      this.requestClose();
    }
+  }

  private focusPanel(): void {
    const panel = this.panelRef?.nativeElement;