From 80d944c0747175019f0e56eafcfe0bccc2aef7d9 Mon Sep 17 00:00:00 2001 From: rdlabo Date: Wed, 26 Aug 2026 10:41:44 +0900 Subject: [PATCH] fix: centralize synchronous handlers and modal result types --- .../live-update-readiness.provider.spec.ts | 9 +- .../src/lib/offline-auth-bridge.spec.ts | 9 +- projects/kit/src/lib/utils/dom.spec.ts | 117 +++++++++++++++++- projects/kit/src/lib/utils/dom.ts | 52 +++++--- .../editor/src/lib/photo-editor.page.ts | 2 + .../viewer/src/lib/photo-viewer.page.ts | 2 + 6 files changed, 169 insertions(+), 22 deletions(-) diff --git a/projects/kit/live-update/src/live-update-readiness.provider.spec.ts b/projects/kit/live-update/src/live-update-readiness.provider.spec.ts index 1fe5c637..02e19703 100644 --- a/projects/kit/live-update/src/live-update-readiness.provider.spec.ts +++ b/projects/kit/live-update/src/live-update-readiness.provider.spec.ts @@ -10,8 +10,6 @@ const { ready } = vi.hoisted(() => ({ ready: vi.fn() })); vi.mock('@capawesome/capacitor-live-update', () => ({ LiveUpdate: { ready } })); describe('provideLiveUpdateReadiness', () => { - const flushFrame = () => new Promise((resolve) => requestAnimationFrame(() => setTimeout(resolve))); - afterEach(() => { ready.mockReset(); vi.restoreAllMocks(); @@ -22,6 +20,10 @@ describe('provideLiveUpdateReadiness', () => { const stable = new ReplaySubject(1); const routerEvents = new Subject(); vi.spyOn(Capacitor, 'isNativePlatform').mockReturnValue(true); + vi.spyOn(globalThis, 'requestAnimationFrame').mockImplementation((callback) => { + callback(0); + return 1; + }); ready.mockResolvedValue({ previousBundleId: null, currentBundleId: null, @@ -41,8 +43,7 @@ describe('provideLiveUpdateReadiness', () => { expect(ready).not.toHaveBeenCalled(); routerEvents.next(new NavigationEnd(1, '/', '/')); - await flushFrame(); - expect(ready).toHaveBeenCalledOnce(); + await vi.waitFor(() => expect(ready).toHaveBeenCalledOnce()); }); it('does not initialize Live Update on the web', () => { diff --git a/projects/kit/offline/src/lib/offline-auth-bridge.spec.ts b/projects/kit/offline/src/lib/offline-auth-bridge.spec.ts index 53f83e68..6a0bb0df 100644 --- a/projects/kit/offline/src/lib/offline-auth-bridge.spec.ts +++ b/projects/kit/offline/src/lib/offline-auth-bridge.spec.ts @@ -2,7 +2,7 @@ import { provideZonelessChangeDetection, signal } from '@angular/core'; import { TestBed } from '@angular/core/testing'; import type { RouterStateSnapshot } from '@angular/router'; import type { KitAuthAccessLease, KitRemoteAccessRecovery } from '@rdlabo/ionic-angular-kit'; -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { firstValueFrom, of } from 'rxjs'; import { createOfflineAuthBridge, type OfflineAuthExchangeContext, type OfflineRemoteIdentity } from './offline-auth-bridge'; import type { OfflineCoordinatorService } from './offline-coordinator.service'; @@ -68,6 +68,8 @@ function setupBridge( } describe('createOfflineAuthBridge', () => { + afterEach(() => TestBed.resetTestingModule()); + it('orders exchange, prepareRemoteSession, grant, and resumeRemoteSession', async () => { const { bridge, offline, order } = setupBridge(); const { lease } = createLease(); @@ -199,8 +201,9 @@ describe('createOfflineAuthBridge', () => { }), ); - await expect(firstValueFrom(bridge.remoteRecovery!.availability())).resolves.toBe(false); - TestBed.resetTestingModule(); + const availability = firstValueFrom(bridge.remoteRecovery!.availability()); + TestBed.flushEffects(); + await expect(availability).resolves.toBe(false); }); it('uses a custom availability observable when supplied', () => { diff --git a/projects/kit/src/lib/utils/dom.spec.ts b/projects/kit/src/lib/utils/dom.spec.ts index 6274989f..c67f7701 100644 --- a/projects/kit/src/lib/utils/dom.spec.ts +++ b/projects/kit/src/lib/utils/dom.spec.ts @@ -22,10 +22,125 @@ describe('disableHandler', () => { it('re-enables the button even when the work rejects', async () => { const { button, event } = clickEvent(); - await disableHandler(event, Promise.reject(new Error('boom'))); + const result: Promise = disableHandler(event, Promise.reject(new Error('boom'))); + await expect(result).resolves.toBeUndefined(); expect(button.disabled).toBe(false); }); + it('restores the button when reading a foreign thenable throws', async () => { + const { button, event } = clickEvent(); + const work = Object.defineProperty({}, 'then', { + get: () => { + throw new Error('invalid thenable'); + }, + }) as PromiseLike; + + await expect(disableHandler(event, work)).resolves.toBeUndefined(); + expect(button.disabled).toBe(false); + }); + + it('keeps the button disabled until overlapping work has settled', async () => { + const { button, event } = clickEvent(); + let finishFirst!: () => void; + let finishSecond!: () => void; + const first = new Promise((resolve) => (finishFirst = resolve)); + const second = new Promise((resolve) => (finishSecond = resolve)); + + const firstResult = disableHandler(event, first); + const secondResult = disableHandler(event, second); + finishFirst(); + await firstResult; + + expect(button.disabled).toBe(true); + + finishSecond(); + await secondResult; + expect(button.disabled).toBe(false); + }); + + it('restores an initially disabled button after overlapping work settles in reverse order', async () => { + const { button, event } = clickEvent(); + button.disabled = true; + let finishFirst!: () => void; + let rejectSecond!: (reason: unknown) => void; + const first = new Promise((resolve) => (finishFirst = resolve)); + const second = new Promise((_, reject) => (rejectSecond = reject)); + + const firstResult = disableHandler(event, first); + const secondResult = disableHandler(event, second); + rejectSecond(new Error('second failed')); + await secondResult; + + expect(button.disabled).toBe(true); + + finishFirst(); + await firstResult; + expect(button.disabled).toBe(true); + }); + + it('accepts work whose type can be either synchronous or asynchronous', async () => { + const { event } = clickEvent(); + const invoke = (work: void | PromiseLike): void | Promise => disableHandler(event, work); + + await invoke(nextMicrotask()); + }); + + it('supports thenables without requiring a finally method', async () => { + const { button, event } = clickEvent(); + const pending = nextMicrotask(); + const work: PromiseLike = { + then: pending.then.bind(pending), + }; + + const result = disableHandler(event, work); + + expect(button.disabled).toBe(true); + await result; + expect(button.disabled).toBe(false); + }); + + it('accepts synchronous click work without changing the disabled state', () => { + const { button, event } = clickEvent(); + + const result: void = disableHandler(event, undefined); + + expect(result).toBeUndefined(); + expect(button.disabled).toBe(false); + }); + + it('uses the current target when a nested element is clicked', async () => { + const button = document.createElement('button'); + const icon = document.createElement('span'); + button.appendChild(icon); + const event = { target: icon, currentTarget: button } as unknown as Event; + + const result = disableHandler(event, nextMicrotask()); + expect(button.disabled).toBe(true); + await result; + expect(button.disabled).toBe(false); + }); + + it('restores a detached target after work settles', async () => { + const { button, event } = clickEvent(); + document.body.appendChild(button); + const result = disableHandler(event, nextMicrotask()); + button.remove(); + + await result; + expect(button.disabled).toBe(false); + }); + + it('prevents form navigation for synchronous submit work', () => { + const form = document.createElement('form'); + const preventDefault = vi.fn(); + const event = { type: 'submit', target: form, currentTarget: form, preventDefault } as unknown as SubmitEvent; + + const result = disableHandler(event, undefined); + + expect(result).toBeUndefined(); + expect(preventDefault).toHaveBeenCalledOnce(); + }); + it('prevents form navigation and disables a native submitter', async () => { const form = document.createElement('form'); const button = document.createElement('button'); diff --git a/projects/kit/src/lib/utils/dom.ts b/projects/kit/src/lib/utils/dom.ts index d6d46706..8b32a87a 100644 --- a/projects/kit/src/lib/utils/dom.ts +++ b/projects/kit/src/lib/utils/dom.ts @@ -1,4 +1,10 @@ type DisableableElement = HTMLElement & { disabled: boolean }; +interface DisableState { + count: number; + original: boolean; +} + +const disableStates = new WeakMap(); const isDisableable = (element: EventTarget | null): element is DisableableElement => element instanceof HTMLElement && 'disabled' in element; @@ -36,6 +42,25 @@ const getDisableTargets = (event: Event): DisableableElement[] => { return [...new Set(targets)]; }; +const acquireDisableTarget = (target: DisableableElement): void => { + const state = disableStates.get(target); + if (state) { + state.count += 1; + return; + } + disableStates.set(target, { count: 1, original: target.disabled }); + target.disabled = true; +}; + +const releaseDisableTarget = (target: DisableableElement): void => { + const state = disableStates.get(target); + if (!state) return; + state.count -= 1; + if (state.count > 0) return; + target.disabled = state.original; + disableStates.delete(target); +}; + /** * Disable the controls that triggered an event while an async operation runs. * @@ -47,27 +72,26 @@ const getDisableTargets = (event: Event): DisableableElement[] => { * controls always recover; handle errors inside `work` when the caller needs to react. * * @param event - The click or submit event that triggered the operation. - * @param work - The async operation to run while the controls are disabled. - * @returns A Promise that resolves once the work has settled and the controls have been restored. + * @param work - The operation result. Promise-like work keeps controls disabled until it settles; + * void work still shares this entry point and submit prevention without a synthetic async boundary. + * @returns A Promise for asynchronous work, otherwise void. * @example * ```html *
* Save * ``` */ -export const disableHandler = (event: Event, work: Promise): Promise => { +export function disableHandler(event: Event, work: void): void; +export function disableHandler(event: Event, work: PromiseLike): Promise; +export function disableHandler(event: Event, work: void | PromiseLike): void | Promise; +export function disableHandler(event: Event, work: void | PromiseLike): void | Promise { if (event.type === 'submit') event.preventDefault(); + if (work === undefined) return; + const targets = getDisableTargets(event); - const disabledStates = targets.map((target) => target.disabled); - targets.forEach((target) => (target.disabled = true)); + targets.forEach(acquireDisableTarget); - return work - .then( - () => undefined, - () => undefined, - ) - .finally(() => { - targets.forEach((target, index) => (target.disabled = disabledStates[index])); - }); -}; + const restore = () => targets.forEach(releaseDisableTarget); + return new Promise((resolve) => resolve(work)).then(restore, restore); +} diff --git a/projects/photo-editor/editor/src/lib/photo-editor.page.ts b/projects/photo-editor/editor/src/lib/photo-editor.page.ts index 7ee0285e..a2aac6ff 100644 --- a/projects/photo-editor/editor/src/lib/photo-editor.page.ts +++ b/projects/photo-editor/editor/src/lib/photo-editor.page.ts @@ -36,6 +36,8 @@ import { dictionaryForEditor, initializeEditorIcons } from './internals'; }) /** Ionic modal page for cropping, rotating, filtering, and saving a photo. */ export class PhotoEditorPage implements OnDestroy, ViewDidEnter, ViewDidLeave { + declare static modalReturn: PhotoEditorResult; + protected readonly modalCtrl = inject(ModalController); readonly #config = inject(PHOTO_EDITOR_CONFIG); diff --git a/projects/photo-editor/viewer/src/lib/photo-viewer.page.ts b/projects/photo-editor/viewer/src/lib/photo-viewer.page.ts index 5e876f60..4c75be80 100644 --- a/projects/photo-editor/viewer/src/lib/photo-viewer.page.ts +++ b/projects/photo-editor/viewer/src/lib/photo-viewer.page.ts @@ -30,6 +30,8 @@ import { dictionaryForViewer, initializeViewerIcons, ionComponents } from './int }) /** Ionic modal page for zooming, browsing, and optionally deleting photos. */ export class PhotoViewerPage implements OnInit, OnDestroy { + declare static modalReturn: PhotoViewerResult; + readonly imageUrls = input.required(); readonly index = input(0, { transform: coerceNumberProperty,