diff --git a/docs/FUTURE_FEATURES.md b/docs/FUTURE_FEATURES.md index 10922e6..0c2b146 100644 --- 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 diff --git a/src/app/pages/cart/cart.component.html b/src/app/pages/cart/cart.component.html index 6efc9b3..a6df206 100644 --- a/src/app/pages/cart/cart.component.html +++ b/src/app/pages/cart/cart.component.html @@ -195,9 +195,15 @@ -@if (showPaymentPopup()) { -
- } -
+ - @if (showBankPaymentPopup() && bankPaymentFrameUrl()) { -
- -
- } - +@if (showBankPaymentPopup() && bankPaymentFrameUrl()) { + + + + } diff --git a/src/app/pages/cart/cart.component.scss b/src/app/pages/cart/cart.component.scss index 882c39b..3f1c608 100644 --- 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%; - padding: 40px; - position: relative; - max-height: 90vh; - overflow-y: auto; - border: 1px solid var(--border-color); - box-shadow: var(--shadow-lg); + app-dialog.payment-dialog .app-dialog-panel__body { + padding: 40px; + } } // Deduplicated: this rule previously appeared twice (identical) - kept a single definition. @@ -970,25 +958,23 @@ } -.bank-payment-modal-overlay { - position: fixed; - inset: 0; - z-index: 1001; - display: flex; - align-items: center; - justify-content: center; - padding: 24px; - background: rgba(17, 24, 39, 0.55); -} +:host ::ng-deep { + app-dialog.bank-payment-dialog .app-dialog-backdrop { + background: rgba(17, 24, 39, 0.55); + } -.bank-payment-modal { - position: relative; - 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 { + width: min(960px, 92vw); + max-width: min(960px, 92vw); + height: min(760px, 86vh); + 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 { @@ -1041,9 +1027,9 @@ display: flex; } - .payment-modal { + :host ::ng-deep app-dialog.payment-dialog .app-dialog-panel__body { padding: 24px; - padding-top: 56px; // �������������� ������ ������ ��� ������ �������� + padding-top: 56px; } .close-modal-btn { @@ -1078,14 +1064,20 @@ grid-template-columns: 1fr; } - .bank-payment-modal-overlay { - padding: 12px; - } + :host ::ng-deep { + app-dialog.bank-payment-dialog .app-dialog-backdrop { + padding: 12px; + } - .bank-payment-modal { - width: 94vw; - height: 82vh; - padding: 52px 10px 10px; + 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 { diff --git a/src/app/pages/cart/cart.component.ts b/src/app/pages/cart/cart.component.ts index 9e71d84..cf00c2e 100644 --- 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; - @ViewChild('bankPaymentModalPanel') private bankPaymentModalPanel?: ElementRef; - 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(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(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 { diff --git a/src/app/shared/ui/dialog/dialog.component.html b/src/app/shared/ui/dialog/dialog.component.html index 24408c7..7f0a7c7 100644 --- 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()" > diff --git a/src/app/shared/ui/dialog/dialog.component.scss b/src/app/shared/ui/dialog/dialog.component.scss index 2b8500b..069b85f 100644 --- 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; diff --git a/src/app/shared/ui/dialog/dialog.component.ts b/src/app/shared/ui/dialog/dialog.component.ts index 88032eb..876630d 100644 --- 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(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(null); readonly size = input('md'); + // Both default true (existing behavior for every current consumer). + readonly closeOnEscape = input(true); + readonly closeOnBackdropClick = input(true); readonly closed = output(); @@ -60,7 +67,9 @@ export class DialogComponent implements OnChanges, AfterViewInit { return; } if (event.key === 'Escape') { - this.requestClose(); + if (this.closeOnEscape()) { + this.requestClose(); + } return; } if (event.key === 'Tab') { @@ -73,7 +82,9 @@ export class DialogComponent implements OnChanges, AfterViewInit { } protected handleBackdropClick(): void { - this.requestClose(); + if (this.closeOnBackdropClick()) { + this.requestClose(); + } } private focusPanel(): void {