From 341ff1ab46470410b28c16bd5da6ca1b8cb2dfc1 Mon Sep 17 00:00:00 2001 From: Erwan Leboucher Date: Thu, 30 Jul 2026 12:51:00 +0200 Subject: [PATCH] Fix mobile members swipe dismissal --- .../SwipeableOverlayWrapper.test.tsx | 118 +++++++++++++ .../components/SwipeableOverlayWrapper.tsx | 157 +++++++++++++----- .../room-settings/RoomSettings.test.tsx | 67 +++++++- .../features/room-settings/RoomSettings.tsx | 13 +- .../room-settings/RoomSettingsRenderer.tsx | 8 +- src/app/features/room/RoomView.tsx | 2 +- src/app/state/hooks/roomSettings.ts | 11 +- src/app/state/roomSettings.ts | 2 + 8 files changed, 322 insertions(+), 56 deletions(-) create mode 100644 src/app/components/SwipeableOverlayWrapper.test.tsx diff --git a/src/app/components/SwipeableOverlayWrapper.test.tsx b/src/app/components/SwipeableOverlayWrapper.test.tsx new file mode 100644 index 0000000000..854da9e2ec --- /dev/null +++ b/src/app/components/SwipeableOverlayWrapper.test.tsx @@ -0,0 +1,118 @@ +import { fireEvent, render, screen } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { animate } from 'framer-motion'; +import { SwipeableOverlayWrapper } from './SwipeableOverlayWrapper'; + +vi.mock('$utils/platform', () => ({ + isMobileOrTablet: () => true, +})); + +vi.mock('framer-motion', () => { + const animateMock = vi.fn<(...args: unknown[]) => Promise>(() => Promise.resolve()); + return { + animate: animateMock, + motion: { div: 'div' }, + useMotionValue: (initial: number) => { + let value = initial; + return { + get: () => value, + set: (next: number) => { + value = next; + }, + stop: vi.fn<() => void>(), + }; + }, + }; +}); + +const touchList = (target: HTMLElement, clientX: number, clientY: number) => { + const point = { identifier: 0, target, clientX, clientY, pageX: clientX, pageY: clientY }; + return { touches: [point], targetTouches: [point], changedTouches: [point] }; +}; + +function renderWrapper(direction: 'left' | 'right' | 'both', onClose: () => void) { + render( + +
+ + ); + return screen.getByTestId('content'); +} + +describe('SwipeableOverlayWrapper', () => { + beforeEach(() => { + Object.defineProperty(window, 'innerWidth', { configurable: true, value: 320 }); + vi.mocked(animate).mockClear(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('closes after a single horizontal move past the distance threshold', async () => { + const onClose = vi.fn<() => void>(); + const content = renderWrapper('both', onClose); + + fireEvent.touchStart(content, touchList(content, 260, 100)); + fireEvent.touchMove(content, touchList(content, 100, 100)); + fireEvent.touchEnd(content, { + ...touchList(content, 100, 100), + touches: [], + targetTouches: [], + }); + + await Promise.resolve(); + + expect(animate).toHaveBeenCalledWith(expect.anything(), -320, { + duration: 0.22, + ease: 'easeOut', + }); + expect(onClose).toHaveBeenCalledOnce(); + }); + + it('leaves a vertical member-list scroll alone', () => { + const onClose = vi.fn<() => void>(); + const content = renderWrapper('both', onClose); + + fireEvent.touchStart(content, touchList(content, 160, 100)); + fireEvent.touchMove(content, touchList(content, 165, 260)); + fireEvent.touchEnd(content, { + ...touchList(content, 165, 260), + touches: [], + targetTouches: [], + }); + + expect(onClose).not.toHaveBeenCalled(); + expect(animate).not.toHaveBeenCalled(); + }); + + it('does not close a cancelled horizontal gesture', () => { + const onClose = vi.fn<() => void>(); + const content = renderWrapper('both', onClose); + + fireEvent.touchStart(content, touchList(content, 260, 100)); + fireEvent.touchMove(content, touchList(content, 100, 100)); + fireEvent.touchCancel(content, { touches: [], targetTouches: [] }); + + expect(onClose).not.toHaveBeenCalled(); + expect(animate).toHaveBeenCalledWith(expect.anything(), 0, { + duration: 0.22, + ease: 'easeOut', + }); + }); + + it('does not close on a disallowed swipe direction', () => { + const onClose = vi.fn<() => void>(); + const content = renderWrapper('right', onClose); + + fireEvent.touchStart(content, touchList(content, 260, 100)); + fireEvent.touchMove(content, touchList(content, 100, 100)); + fireEvent.touchEnd(content, { + ...touchList(content, 100, 100), + touches: [], + targetTouches: [], + }); + + expect(onClose).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/components/SwipeableOverlayWrapper.tsx b/src/app/components/SwipeableOverlayWrapper.tsx index 6efae98d2b..885681649d 100644 --- a/src/app/components/SwipeableOverlayWrapper.tsx +++ b/src/app/components/SwipeableOverlayWrapper.tsx @@ -1,12 +1,31 @@ import type { ReactNode } from 'react'; +import { useRef } from 'react'; import { animate, motion, useMotionValue } from 'framer-motion'; -import { useDrag } from '@use-gesture/react'; import { isMobileOrTablet } from '$utils/platform'; +const SETTLE_MS = 220; +const LOCK_THRESHOLD_PX = 8; +const COMMIT_FRACTION = 0.22; +const VELOCITY_THRESHOLD = 0.45; // px per ms + +const getViewportWidth = () => document.documentElement.clientWidth || window.innerWidth; + +type GestureMode = 'pending' | 'vertical' | 'horizontal' | 'blocked'; + +type ActiveGesture = { + startX: number; + startY: number; + lastX: number; + lastTime: number; + velocityX: number; + mode: GestureMode; + lockOffset: number; +}; + interface SwipeableOverlayWrapperProps { children: ReactNode; onClose: () => void; - direction: 'left' | 'right'; + direction: 'left' | 'right' | 'both'; } export function SwipeableOverlayWrapper({ @@ -15,55 +34,51 @@ export function SwipeableOverlayWrapper({ direction, }: SwipeableOverlayWrapperProps) { const x = useMotionValue(0); + const gestureRef = useRef(); + const closeCommittedRef = useRef(false); - const bind = useDrag( - ({ first, active, offset: [ox], velocity: [vx], direction: [dx], event, cancel }) => { - if (first && event && 'target' in event && event.target instanceof HTMLElement) { - if (event.target.closest('[data-gestures="ignore"]')) { - cancel(); - return; - } - } + const acceptsLeft = direction !== 'right'; + const acceptsRight = direction !== 'left'; - if (!isMobileOrTablet()) return; + const clampOffset = (val: number, viewportWidth: number) => { + let v = val; + if (!acceptsLeft) v = Math.max(0, v); + if (!acceptsRight) v = Math.min(0, v); + return Math.max(-viewportWidth, Math.min(viewportWidth, v)); + }; - event.stopPropagation(); + const finish = (commitEligible: boolean) => { + const gesture = gestureRef.current; + gestureRef.current = undefined; + if (!gesture || gesture.mode !== 'horizontal') return; - let val = ox; + if (commitEligible) { + const viewportWidth = getViewportWidth(); + const val = x.get(); + const swipedLeft = + acceptsLeft && + val < 0 && + (val <= -viewportWidth * COMMIT_FRACTION || gesture.velocityX <= -VELOCITY_THRESHOLD); + const swipedRight = + acceptsRight && + val > 0 && + (val >= viewportWidth * COMMIT_FRACTION || gesture.velocityX >= VELOCITY_THRESHOLD); - if (direction === 'left' && val > 0) val = 0; - if (direction === 'right' && val < 0) val = 0; - - if (active) { - // Take over any settling spring; offset is seeded from the live position. - if (first) x.stop(); - x.set(val); - } else { - const swipeThreshold = 100; - const velocityThreshold = 0.5; - - const swipedLeft = - direction === 'left' && (val < -swipeThreshold || (vx > velocityThreshold && dx < 0)); - const swipedRight = - direction === 'right' && (val > swipeThreshold || (vx > velocityThreshold && dx > 0)); - - if (swipedLeft || swipedRight) { + if (swipedLeft || swipedRight) { + closeCommittedRef.current = true; + const target = swipedLeft ? -viewportWidth : viewportWidth; + void animate(x, target, { duration: SETTLE_MS / 1000, ease: 'easeOut' }).then(() => { + if (!closeCommittedRef.current) return; onClose(); - } - - animate(x, 0, { type: 'spring', stiffness: 400, damping: 40 }); + closeCommittedRef.current = false; + animate(x, 0, { duration: SETTLE_MS / 1000, ease: 'easeOut' }); + }); + return; } - }, - { - axis: 'x', - bounds: direction === 'left' ? { left: -300, right: 0 } : { left: 0, right: 300 }, - rubberband: true, - filterTaps: true, - pointer: { capture: true }, - eventOptions: { passive: true }, - from: () => [x.get(), 0], } - ); + + animate(x, 0, { duration: SETTLE_MS / 1000, ease: 'easeOut' }); + }; if (!isMobileOrTablet()) { return ( @@ -83,7 +98,59 @@ export function SwipeableOverlayWrapper({ return (
{ + if (closeCommittedRef.current) return; + if (event.touches.length !== 1) { + finish(false); + return; + } + const touch = event.touches[0]; + if (!touch) return; + const blocked = + event.target instanceof HTMLElement && + event.target.closest('[data-gestures="ignore"]') !== null; + gestureRef.current = { + startX: touch.clientX, + startY: touch.clientY, + lastX: touch.clientX, + lastTime: event.timeStamp, + velocityX: 0, + mode: blocked ? 'blocked' : 'pending', + lockOffset: 0, + }; + }} + onTouchMove={(event) => { + const gesture = gestureRef.current; + const touch = event.touches[0]; + if (!gesture || !touch || gesture.mode === 'blocked' || gesture.mode === 'vertical') { + return; + } + + const distanceX = touch.clientX - gesture.startX; + const distanceY = touch.clientY - gesture.startY; + const elapsed = event.timeStamp - gesture.lastTime; + if (elapsed > 0) { + gesture.velocityX = (touch.clientX - gesture.lastX) / elapsed; + gesture.lastX = touch.clientX; + gesture.lastTime = event.timeStamp; + } + + if (gesture.mode === 'pending') { + if (Math.max(Math.abs(distanceX), Math.abs(distanceY)) < LOCK_THRESHOLD_PX) return; + if (Math.abs(distanceY) >= Math.abs(distanceX)) { + gesture.mode = 'vertical'; + return; + } + gesture.mode = 'horizontal'; + // Take over any settling spring; offset is seeded from the live position. + x.stop(); + gesture.lockOffset = x.get(); + } + + x.set(clampOffset(gesture.lockOffset + distanceX, getViewportWidth())); + }} + onTouchEnd={() => finish(true)} + onTouchCancel={() => finish(false)} style={{ overflow: 'hidden', display: 'flex', @@ -91,6 +158,8 @@ export function SwipeableOverlayWrapper({ flexGrow: 1, height: '100%', width: '100%', + touchAction: 'pan-y', + overscrollBehaviorX: 'none', }} > ({ Appearance: mkSection('Appearance section'), })); +// Captures the SwipeableOverlayWrapper onClose (i.e. RoomSettings' swipe-back handler) +// so tests can trigger it without simulating touch gestures. +let capturedSwipeBack: (() => void) | undefined; +let capturedSwipeDirection: 'left' | 'right' | 'both' | undefined; +vi.mock('$components/SwipeableOverlayWrapper', () => ({ + SwipeableOverlayWrapper: ({ + children, + direction, + onClose, + }: { + children: ReactNode; + direction: 'left' | 'right' | 'both'; + onClose: () => void; + }) => { + capturedSwipeBack = onClose; + capturedSwipeDirection = direction; + return
{children}
; + }, +})); + // ── Render helper ── function renderRoomSettings({ isSpace = false, screenSize = ScreenSize.Desktop, initialPage, + openedViaSwipe, }: { isSpace?: boolean; screenSize?: ScreenSize; initialPage?: RoomSettingsPage; + openedViaSwipe?: boolean; } = {}) { mockRoom = createMockRoom(isSpace); @@ -120,7 +142,11 @@ function renderRoomSettings({ render( - + ); @@ -211,6 +237,43 @@ describe('RoomSettings menu', () => { expect(screen.getByRole('button', { name: 'General' })).toBeInTheDocument(); }); + it('swipe closes the overlay directly when opened via swipe', () => { + const { requestClose } = renderRoomSettings({ + isSpace: false, + screenSize: ScreenSize.Mobile, + initialPage: RoomSettingsPage.MembersPage, + openedViaSwipe: true, + }); + + expect(screen.getByRole('heading', { name: 'Members section' })).toBeInTheDocument(); + expect(capturedSwipeDirection).toBe('both'); + + expect(capturedSwipeBack).toBeDefined(); + act(() => capturedSwipeBack?.()); + + expect(requestClose).toHaveBeenCalled(); + // Still on the sections page (never navigated back to the section list) + expect(screen.getByRole('heading', { name: 'Members section' })).toBeInTheDocument(); + }); + + it('rightward swipe goes back to the section list when not opened via swipe', () => { + const { requestClose } = renderRoomSettings({ + isSpace: false, + screenSize: ScreenSize.Mobile, + initialPage: RoomSettingsPage.MembersPage, + }); + + expect(screen.getByRole('heading', { name: 'Members section' })).toBeInTheDocument(); + expect(capturedSwipeDirection).toBe('right'); + + expect(capturedSwipeBack).toBeDefined(); + act(() => capturedSwipeBack?.()); + + expect(requestClose).not.toHaveBeenCalled(); + expect(screen.queryByRole('heading', { name: 'Members section' })).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Members' })).toBeInTheDocument(); + }); + it('visible flag gates section content', () => { // Render for a non-space room, but try to force the Appearance section // by starting on AppearancePage. The SettingsShell's `visible !== false` diff --git a/src/app/features/room-settings/RoomSettings.tsx b/src/app/features/room-settings/RoomSettings.tsx index 025616464b..e88917ba9a 100644 --- a/src/app/features/room-settings/RoomSettings.tsx +++ b/src/app/features/room-settings/RoomSettings.tsx @@ -48,10 +48,10 @@ function makeAbbreviationsComponent(isSpace: boolean) { return AbbreviationsWrapper; } -function makeSwipeWrapper(onClose: () => void) { +function makeSwipeWrapper(direction: 'left' | 'right' | 'both', onClose: () => void) { function SwipeWrapper(children: ReactNode) { return ( - + {children} ); @@ -104,10 +104,11 @@ const sectionIdToPage: Record = { type RoomSettingsProps = { initialPage?: RoomSettingsPage; + openedViaSwipe?: boolean; requestClose: () => void; }; -export function RoomSettings({ initialPage, requestClose }: RoomSettingsProps) { +export function RoomSettings({ initialPage, openedViaSwipe, requestClose }: RoomSettingsProps) { const room = useRoom(); const isSpace = room.isSpaceRoom(); const mx = useMatrixClient(); @@ -142,6 +143,10 @@ export function RoomSettings({ initialPage, requestClose }: RoomSettingsProps) { const handleSwipeBack = () => { if (screenSize !== ScreenSize.Mobile) return; + if (openedViaSwipe) { + requestClose(); + return; + } if (activePage !== undefined) { setActivePage(undefined); return; @@ -212,7 +217,7 @@ export function RoomSettings({ initialPage, requestClose }: RoomSettingsProps) { ); - const swipeWrapper = makeSwipeWrapper(handleSwipeBack); + const swipeWrapper = makeSwipeWrapper(openedViaSwipe ? 'both' : 'right', handleSwipeBack); return ( diff --git a/src/app/features/room-settings/RoomSettingsRenderer.tsx b/src/app/features/room-settings/RoomSettingsRenderer.tsx index e66de208ac..6874fcbafb 100644 --- a/src/app/features/room-settings/RoomSettingsRenderer.tsx +++ b/src/app/features/room-settings/RoomSettingsRenderer.tsx @@ -10,7 +10,7 @@ type RenderSettingsProps = { state: RoomSettingsState; }; function RenderSettings({ state }: RenderSettingsProps) { - const { roomId, spaceId, page } = state; + const { roomId, spaceId, page, openedViaSwipe } = state; const closeSettings = useCloseRoomSettings(); const allJoinedRooms = useAllJoinedRoomsSet(); const getRoom = useGetRoom(allJoinedRooms); @@ -23,7 +23,11 @@ function RenderSettings({ state }: RenderSettingsProps) { - + diff --git a/src/app/features/room/RoomView.tsx b/src/app/features/room/RoomView.tsx index de6e852c2c..cf0068ce38 100644 --- a/src/app/features/room/RoomView.tsx +++ b/src/app/features/room/RoomView.tsx @@ -136,7 +136,7 @@ export function RoomView({ eventId }: { eventId?: string }) { const handleOpenMembers = useCallback(() => { if (screenSize === ScreenSize.Mobile) { - openSettings(room.roomId, space?.roomId, RoomSettingsPage.MembersPage); + openSettings(room.roomId, space?.roomId, RoomSettingsPage.MembersPage, { viaSwipe: true }); } }, [screenSize, openSettings, room.roomId, space?.roomId]); diff --git a/src/app/state/hooks/roomSettings.ts b/src/app/state/hooks/roomSettings.ts index 82204e9d77..4f800fe4c7 100644 --- a/src/app/state/hooks/roomSettings.ts +++ b/src/app/state/hooks/roomSettings.ts @@ -20,13 +20,18 @@ export const useCloseRoomSettings = (): CloseCallback => { return close; }; -type OpenCallback = (roomId: string, space?: string, page?: RoomSettingsPage) => void; +type OpenCallback = ( + roomId: string, + space?: string, + page?: RoomSettingsPage, + options?: { viaSwipe?: boolean } +) => void; export const useOpenRoomSettings = (): OpenCallback => { const setSettings = useSetAtom(roomSettingsAtom); const open: OpenCallback = useCallback( - (roomId, spaceId, page) => { - setSettings({ roomId, spaceId, page }); + (roomId, spaceId, page, options) => { + setSettings({ roomId, spaceId, page, openedViaSwipe: options?.viaSwipe }); }, [setSettings] ); diff --git a/src/app/state/roomSettings.ts b/src/app/state/roomSettings.ts index cfeaef7ba0..a431da1518 100644 --- a/src/app/state/roomSettings.ts +++ b/src/app/state/roomSettings.ts @@ -17,6 +17,8 @@ export type RoomSettingsState = { page?: RoomSettingsPage; roomId: string; spaceId?: string; + /** True when opened via the chat-level leftward swipe (mobile fullscreen). */ + openedViaSwipe?: boolean; }; export const roomSettingsAtom = atom(undefined);