From 13f5ad699ff28556db5138be8f2d5a6ff65bcf35 Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Wed, 7 Oct 2026 12:02:07 +0200 Subject: [PATCH] fix: ctx menu failure and imperative closure edge cases --- .../MessageOverlayHostLayer.tsx | 14 ++++ .../MessageOverlayHostLayer.test.tsx | 59 +++++++++++++- .../__tests__/message-overlay-store.test.tsx | 80 +++++++++++++++++++ .../src/state-store/message-overlay-store.ts | 13 ++- 4 files changed, 161 insertions(+), 5 deletions(-) diff --git a/package/src/contexts/overlayContext/MessageOverlayHostLayer.tsx b/package/src/contexts/overlayContext/MessageOverlayHostLayer.tsx index 96e3d523db..d5b1a2029e 100644 --- a/package/src/contexts/overlayContext/MessageOverlayHostLayer.tsx +++ b/package/src/contexts/overlayContext/MessageOverlayHostLayer.tsx @@ -1,5 +1,6 @@ import React, { useEffect, useMemo, useRef } from 'react'; import { + BackHandler, I18nManager, Platform, Pressable, @@ -101,6 +102,19 @@ export const MessageOverlayHostLayer = () => { } }, [isActive, overlayOpenHint, announce]); + useEffect(() => { + if (!isActive) { + return; + } + + const subscription = BackHandler.addEventListener('hardwareBackPress', () => { + closeOverlay(); + return true; + }); + + return () => subscription.remove(); + }, [isActive]); + // When orientation changes, we dismiss the overlay. While it is possible // (and I have a working PoC of) a repositioning solution, it is pretty // complex and probably bug prone. This is anyway how the iOS native contextual diff --git a/package/src/contexts/overlayContext/__tests__/MessageOverlayHostLayer.test.tsx b/package/src/contexts/overlayContext/__tests__/MessageOverlayHostLayer.test.tsx index 8d0cb06efe..299b17f8a9 100644 --- a/package/src/contexts/overlayContext/__tests__/MessageOverlayHostLayer.test.tsx +++ b/package/src/contexts/overlayContext/__tests__/MessageOverlayHostLayer.test.tsx @@ -1,5 +1,5 @@ import React from 'react'; -import { StyleSheet, Text } from 'react-native'; +import { BackHandler, StyleSheet, Text } from 'react-native'; import Animated from 'react-native-reanimated'; @@ -179,6 +179,63 @@ describe('MessageOverlayHostLayer', () => { expect(overlayStore.getLatestValue().closing).toBe(true); }); + it('closes the overlay on Android back press instead of letting navigation handle it', () => { + type BackPressListener = Parameters[1]; + const listeners: BackPressListener[] = []; + const addEventListenerSpy = jest + .spyOn(BackHandler, 'addEventListener') + .mockImplementation((_eventName, handler) => { + listeners.push(handler); + return { + remove: () => { + listeners.splice(listeners.indexOf(handler), 1); + }, + }; + }); + const pressBack = () => + listeners[listeners.length - 1]?.({ timeStamp: Date.now(), type: 'hardwareBackPress' }); + + render( + + + , + ); + + // Nothing is registered while the overlay is closed, so back goes to navigation. + expect(listeners).toHaveLength(0); + + act(() => { + openOverlay('message-1'); + }); + + let handled: boolean | null | undefined; + act(() => { + handled = pressBack(); + }); + flushAnimationFrameQueue(); + + expect(handled).toBe(true); + expect(overlayStore.getLatestValue().closing).toBe(true); + + // Still consumed while the close animation runs, so the screen doesn't pop underneath. + act(() => { + handled = pressBack(); + }); + flushAnimationFrameQueue(); + expect(handled).toBe(true); + expect(overlayStore.getLatestValue().closing).toBe(true); + + act(() => { + finalizeCloseOverlay(); + }); + + expect(listeners).toHaveLength(0); + // One registration for the whole open -> closing -> closed cycle. + expect(addEventListenerSpy).toHaveBeenCalledTimes(1); + + addEventListenerSpy.mockRestore(); + }); + it('positions and translates the top, message, and bottom hosts using the registered rects', () => { const renderTree = () => ( diff --git a/package/src/state-store/__tests__/message-overlay-store.test.tsx b/package/src/state-store/__tests__/message-overlay-store.test.tsx index b18aef459a..cdd8c903eb 100644 --- a/package/src/state-store/__tests__/message-overlay-store.test.tsx +++ b/package/src/state-store/__tests__/message-overlay-store.test.tsx @@ -9,6 +9,7 @@ import { finalizeCloseOverlay, openOverlay, overlayStore, + scheduleActionOnClose, setClosingPortalLayout, useClosingPortalHostBlacklist, useClosingPortalHostBlacklistState, @@ -28,6 +29,14 @@ const flushAnimationFrameQueue = () => { }); }; +// Runs `fn` and lets the async close-action flush (`await action()` per queued action) settle. +const actAndFlushPromises = async (fn: () => void) => { + await act(async () => { + fn(); + await Promise.resolve(); + }); +}; + const BlacklistRegistrar = ({ enabled = true, hostNames, @@ -125,6 +134,77 @@ describe('message overlay store portal hooks', () => { second.unmount(); }); + it('keeps running queued actions when one of them fails and does not rerun it on the next close', async () => { + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + const failingAction = jest.fn(() => Promise.reject(new Error('action failed'))); + const throwingAction = jest.fn(() => { + throw new Error('sync action failed'); + }); + const followingAction = jest.fn(); + + act(() => { + openOverlay('message-1'); + }); + scheduleActionOnClose(failingAction); + scheduleActionOnClose(throwingAction); + scheduleActionOnClose(followingAction); + + await actAndFlushPromises(finalizeCloseOverlay); + + expect(failingAction).toHaveBeenCalledTimes(1); + expect(throwingAction).toHaveBeenCalledTimes(1); + expect(followingAction).toHaveBeenCalledTimes(1); + expect(consoleWarnSpy).toHaveBeenCalledTimes(2); + + const nextAction = jest.fn(); + + act(() => { + openOverlay('message-2'); + }); + scheduleActionOnClose(nextAction); + + await actAndFlushPromises(finalizeCloseOverlay); + + expect(nextAction).toHaveBeenCalledTimes(1); + expect(failingAction).toHaveBeenCalledTimes(1); + expect(throwingAction).toHaveBeenCalledTimes(1); + + consoleWarnSpy.mockRestore(); + }); + + it('defers actions scheduled during a flush to the next close instead of running them twice', async () => { + let resolveSlowAction: () => void = () => {}; + const slowAction = jest.fn( + () => + new Promise((resolve) => { + resolveSlowAction = resolve; + }), + ); + const laterAction = jest.fn(); + + act(() => { + openOverlay('message-1'); + }); + scheduleActionOnClose(slowAction); + + await actAndFlushPromises(finalizeCloseOverlay); + expect(slowAction).toHaveBeenCalledTimes(1); + + // The overlay reopens and an action is scheduled while the first flush is still awaiting. + act(() => { + openOverlay('message-2'); + }); + scheduleActionOnClose(laterAction); + + await actAndFlushPromises(resolveSlowAction); + expect(laterAction).not.toHaveBeenCalled(); + + await actAndFlushPromises(finalizeCloseOverlay); + + expect(laterAction).toHaveBeenCalledTimes(1); + expect(slowAction).toHaveBeenCalledTimes(1); + }); + it('does not enter closing when closeOverlay is called without an active overlay id', () => { act(() => { closeOverlay(); diff --git a/package/src/state-store/message-overlay-store.ts b/package/src/state-store/message-overlay-store.ts index 9fcd969efc..4334fbb421 100644 --- a/package/src/state-store/message-overlay-store.ts +++ b/package/src/state-store/message-overlay-store.ts @@ -216,12 +216,17 @@ const actionQueueSelector = (nextState: OverlayState) => ({ active: !!nextState. // integrations causing UI issues. overlayStore.subscribeWithSelector(actionQueueSelector, async ({ active }) => { if (!active) { + const actions = actionQueue; + actionQueue = []; + // flush the queue - for (const action of actionQueue) { - await action(); + for (const action of actions) { + try { + await action(); + } catch (error) { + console.warn('A message overlay action failed after the overlay closed:', error); + } } - - actionQueue = []; } });