From 747402fcbac15abc55136f58bcde3117c7341e2b Mon Sep 17 00:00:00 2001 From: Anuraj Jit Saikia Date: Tue, 18 Aug 2026 21:13:37 +0530 Subject: [PATCH] fix(call): keep Leave pill, drop screenshot, gate share stage Presentation layout now waits for a real local or remote MediaStream so Share no longer flashes a black stage. Screenshot capture is gone from the overflow menu and the client; the dedicated red Leave pill stays, with Leave kept as a secondary menu path. --- client/src/components/ControlBar.test.tsx | 49 +++++- client/src/components/ControlBar.tsx | 18 +-- client/src/hooks/useScreenShare.test.tsx | 25 ++- client/src/hooks/useScreenShare.ts | 8 +- client/src/pages/RoomPage.tsx | 27 +--- client/src/utils/capture-screenshot.test.js | 74 --------- client/src/utils/capture-screenshot.ts | 165 -------------------- client/src/utils/video-composite.ts | 6 +- client/tsconfig.json | 1 - 9 files changed, 83 insertions(+), 290 deletions(-) delete mode 100644 client/src/utils/capture-screenshot.test.js delete mode 100644 client/src/utils/capture-screenshot.ts diff --git a/client/src/components/ControlBar.test.tsx b/client/src/components/ControlBar.test.tsx index 6884fc9..1182102 100644 --- a/client/src/components/ControlBar.test.tsx +++ b/client/src/components/ControlBar.test.tsx @@ -57,7 +57,6 @@ function makeProps(overrides: Partial = {}): ControlBarProps { pipActive: false, onTogglePip: vi.fn(), onCopyLink: vi.fn(), - onScreenshot: vi.fn(), onLeave: vi.fn(), micGain: 1, onMicGainChange: vi.fn(), @@ -260,9 +259,51 @@ describe('ControlBar', () => { }); }); - // A11y baseline (#164): toggles expose pressed state, menu triggers expose - // popup/expanded state, and local state flips are narrated via a polite - // live region — the contract screen readers depend on. + describe('leave pill and overflow menu', () => { + it('keeps a dedicated Leave pill visible and usable', () => { + const props = renderBar(); + + const leave = btn('Leave call'); + expect(leave).toBeVisible(); + fireEvent.click(leave); + expect(props.onLeave).toHaveBeenCalledTimes(1); + }); + + it('groups overflow actions and has no screenshot entry', () => { + const props = renderBar({ pipSupported: true, pipActive: false, soundEnabled: true }); + + fireEvent.click(btn('More options')); + const menu = screen.getByRole('menu'); + + expect(within(menu).queryByText('Take screenshot')).not.toBeInTheDocument(); + expect(within(menu).queryByText(/screenshot/i)).not.toBeInTheDocument(); + + expect(within(menu).getByRole('menuitem', { name: /copy meeting link/i })).toBeInTheDocument(); + expect(within(menu).getByRole('menuitem', { name: /open mini player/i })).toBeInTheDocument(); + expect(within(menu).getByRole('menuitem', { name: /sound effects/i })).toHaveTextContent('On'); + expect(within(menu).getByRole('menuitem', { name: /leave call/i })).toBeInTheDocument(); + + const items = within(menu).getAllByRole('menuitem').map((el) => el.textContent); + expect(items).toEqual([ + 'Copy meeting link', + 'Open mini player', + 'Sound effectsOn', + 'Leave call', + ]); + + fireEvent.click(within(menu).getByRole('menuitem', { name: /copy meeting link/i })); + expect(props.onCopyLink).toHaveBeenCalledTimes(1); + }); + + it('keeps Leave in the overflow menu as a secondary path', () => { + const props = renderBar(); + + fireEvent.click(btn('More options')); + fireEvent.click(within(screen.getByRole('menu')).getByRole('menuitem', { name: /leave call/i })); + expect(props.onLeave).toHaveBeenCalledTimes(1); + }); + }); + describe('accessibility', () => { function renderRerenderable(overrides: Partial = {}) { const view = render( diff --git a/client/src/components/ControlBar.tsx b/client/src/components/ControlBar.tsx index acc73bb..e5881bb 100644 --- a/client/src/components/ControlBar.tsx +++ b/client/src/components/ControlBar.tsx @@ -23,7 +23,6 @@ import { MoreVert as MoreVertIcon, PanTool as PanToolIcon, PeopleAlt as PeopleAltIcon, - PhotoCamera as PhotoCameraIcon, PictureInPictureAlt as PipIcon, PresentToAll as PresentIcon, CancelPresentation as StopPresentIcon, @@ -77,7 +76,7 @@ export interface ControlBarProps { layoutMode?: LayoutMode; onLayoutChange: (mode: LayoutMode) => void; soundEnabled: boolean; onToggleSound: () => void; pipSupported: boolean; pipActive: boolean; onTogglePip: () => void; - onCopyLink: () => void; onScreenshot: () => void; onLeave: () => void; + onCopyLink: () => void; onLeave: () => void; micGain?: number; onMicGainChange: (value: number) => void; outputVolume?: number; onOutputVolumeChange: (value: number) => void; showPinToggle?: boolean; pinned?: boolean; onTogglePin?: () => void; @@ -138,7 +137,6 @@ export default function ControlBar({ soundEnabled, onToggleSound, pipSupported, pipActive, onTogglePip, onCopyLink, - onScreenshot, onLeave, micGain = 1, onMicGainChange, outputVolume = 1, onOutputVolumeChange, @@ -459,26 +457,18 @@ export default function ControlBar({ {isMobile && } { closeMore(); onCopyLink(); }}> - Copy joining link + Copy meeting link - {onScreenshot && ( - { closeMore(); onScreenshot(); }}> - - Take screenshot - - )} {pipSupported && ( { closeMore(); onTogglePip(); }}> {pipActive ? 'Close mini player' : 'Open mini player'} )} + { onToggleSound(); }}> {soundEnabled ? : } - Sound effects - - {soundEnabled ? 'On' : 'Off'} - + { closeMore(); onLeave(); }} sx={{ color: 'error.main' }}> diff --git a/client/src/hooks/useScreenShare.test.tsx b/client/src/hooks/useScreenShare.test.tsx index 540a6ea..a432681 100644 --- a/client/src/hooks/useScreenShare.test.tsx +++ b/client/src/hooks/useScreenShare.test.tsx @@ -76,8 +76,31 @@ describe('useScreenShare', () => { localScreenStream: null, })); expect(result.current.shares).toEqual([]); - // hasScreen still reflects the sharing intent. + // Sharing requested, picker/produce still pending — no presentation stage. + expect(result.current.hasScreen).toBe(false); + }); + + it('keeps hasScreen false when sharing is requested but no stream exists yet', () => { + const { result, rerender } = renderHook((props: ScreenShareOptions) => useScreenShare(props), { + initialProps: { + ...baseProps, + isScreenSharing: true, + localScreenStream: null, + } as ScreenShareOptions, + }); + + expect(result.current.hasScreen).toBe(false); + expect(result.current.pinnedShare).toBeNull(); + + const local = stream('local'); + rerender({ + ...baseProps, + isScreenSharing: true, + localScreenStream: local, + localScreenSurface: 'window', + }); expect(result.current.hasScreen).toBe(true); + expect(result.current.pinnedShare?.stream).toBe(local); }); it('prefers a remote share over the local one when nothing is pinned', () => { diff --git a/client/src/hooks/useScreenShare.ts b/client/src/hooks/useScreenShare.ts index 95ec826..c5a7b67 100644 --- a/client/src/hooks/useScreenShare.ts +++ b/client/src/hooks/useScreenShare.ts @@ -32,7 +32,8 @@ export interface ScreenShareEntry { // the first remote share, then the first share, then null. // This replaces the previous setState-in-effect that reset // `pinnedShareKey` whenever the list changed. -// • hasScreen — whether the presentation layout should take the stage. +// • hasScreen — whether a real screen MediaStream is on stage (local +// share only counts once localScreenStream exists). // • showScreenAnyway — opt-in reveal past the local "infinity mirror" guard, // reset whenever the local share stops so a fresh share // re-arms the guard. @@ -89,7 +90,10 @@ export function useScreenShare({ ?? shares[0] ?? null; - const hasScreen = isScreenSharing || remoteScreenEntries.length > 0; + // Intent (`isScreenSharing`) is not enough: clicking Share can flip that + // true before getDisplayMedia returns a stream. Presentation/black stage + // only belongs on a live local or remote MediaStream. + const hasScreen = shares.length > 0; // Opt-in reveal past the local "infinity mirror" guard. Keyed to the *specific* // local stream the user revealed, so the guard re-arms automatically: stopping diff --git a/client/src/pages/RoomPage.tsx b/client/src/pages/RoomPage.tsx index 20f6ed8..3d62c50 100644 --- a/client/src/pages/RoomPage.tsx +++ b/client/src/pages/RoomPage.tsx @@ -45,7 +45,6 @@ import type { TranscriptState, } from '@a-meet/contracts'; import { playSound, isSoundEnabled, toggleSound } from '../services/sounds'; -import { copyMeetingScreenshot, downloadMeetingScreenshot } from '../utils/capture-screenshot'; import { appLogger } from '../utils/logger'; import { downloadTranscript, mergeTranscriptEntries } from '../utils/transcript'; @@ -585,35 +584,12 @@ export default function RoomPage() { pushNote({ kind: 'event', variant: 'info', text: "Couldn't open the mini player" }), ); }; - // Capture the current meeting view (camera tiles + any on-stage share) and - // copy it to the clipboard as a PNG. Falls back to a file download when the - // browser can't write images to the clipboard (e.g. Firefox). - async function handleScreenshot() { - const tiles = cameraTiles().map(({ key, stream, name, videoOn, audioOn, mirror }) => - ({ key, stream, name, videoOn, audioOn, mirror })); - // Prefix the share key so it can't collide with the 'local' camera tile. - const share = pinnedShare - ? { key: `share-${pinnedShare.key}`, stream: pinnedShare.stream, name: pinnedShare.name } - : null; - try { - await copyMeetingScreenshot({ tiles, share }); - playSound('toggleOn'); - pushNote({ kind: 'event', variant: 'info', text: 'Screenshot copied to clipboard' }); - } catch { - try { - await downloadMeetingScreenshot({ tiles, share }, `a-meet-${roomId}`); - pushNote({ kind: 'event', variant: 'info', text: 'Screenshot saved' }); - } catch { - pushNote({ kind: 'event', variant: 'info', text: "Couldn't capture a screenshot" }); - } - } - } async function handleCopyLink() { const link = `${window.location.origin}/lobby/${roomId}`; try { await navigator.clipboard.writeText(link); - pushNote({ kind: 'event', variant: 'info', text: 'Joining link copied' }); + pushNote({ kind: 'event', variant: 'info', text: 'Meeting link copied' }); } catch { pushNote({ kind: 'event', variant: 'info', text: 'Press the link button to copy' }); } @@ -1416,7 +1392,6 @@ export default function RoomPage() { soundEnabled={soundEnabled} onToggleSound={handleToggleSound} pipSupported={pipSupported} pipActive={pipActive} onTogglePip={handleTogglePip} onCopyLink={handleCopyLink} - onScreenshot={handleScreenshot} onLeave={handleLeave} micGain={micGain} onMicGainChange={setMicGain} outputVolume={outputVolume} onOutputVolumeChange={setOutputVolume} diff --git a/client/src/utils/capture-screenshot.test.js b/client/src/utils/capture-screenshot.test.js deleted file mode 100644 index cef8e6f..0000000 --- a/client/src/utils/capture-screenshot.test.js +++ /dev/null @@ -1,74 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { - captureMeetingScreenshot, - copyMeetingScreenshot, - downloadMeetingScreenshot, -} from './capture-screenshot'; - -const contextStub = { - fillStyle: '', - font: '', - textAlign: '', - textBaseline: '', - fillRect: vi.fn(), - fillText: vi.fn(), -}; - -describe('capture screenshot utilities', () => { - beforeEach(() => { - vi.spyOn(HTMLCanvasElement.prototype, 'getContext').mockReturnValue(contextStub); - vi.spyOn(HTMLCanvasElement.prototype, 'toBlob').mockImplementation(function toBlob(callback, type) { - callback(new Blob(['png'], { type })); - }); - }); - - afterEach(() => { - vi.restoreAllMocks(); - vi.unstubAllGlobals(); - document.body.innerHTML = ''; - }); - - it('captures the meeting view as a PNG blob and removes its hidden host', async () => { - const before = document.body.childElementCount; - - const blob = await captureMeetingScreenshot(); - - expect(blob).toBeInstanceOf(Blob); - expect(blob.type).toBe('image/png'); - expect(document.body.childElementCount).toBe(before); - }); - - it('reports unsupported clipboard image writes before capturing', async () => { - vi.stubGlobal('ClipboardItem', undefined); - - await expect(copyMeetingScreenshot()).rejects.toThrow('clipboard-image-unsupported'); - }); - - it('downloads a PNG fallback with the requested filename prefix', async () => { - const anchors = []; - const createElement = document.createElement.bind(document); - vi.spyOn(document, 'createElement').mockImplementation((tagName, options) => { - const element = createElement(tagName, options); - if (tagName === 'a') anchors.push(element); - return element; - }); - vi.spyOn(HTMLElement.prototype, 'click').mockImplementation(() => {}); - Object.defineProperty(URL, 'createObjectURL', { - configurable: true, - value: vi.fn(() => 'blob:screenshot'), - }); - Object.defineProperty(URL, 'revokeObjectURL', { - configurable: true, - value: vi.fn(), - }); - vi.spyOn(Date, 'now').mockReturnValue(1234); - - await downloadMeetingScreenshot({}, 'a-meet-room'); - - expect(anchors).toHaveLength(1); - expect(anchors[0].href).toBe('blob:screenshot'); - expect(anchors[0].download).toBe('a-meet-room-1234.png'); - expect(HTMLElement.prototype.click).toHaveBeenCalled(); - expect(URL.revokeObjectURL).toHaveBeenCalledWith('blob:screenshot'); - }); -}); diff --git a/client/src/utils/capture-screenshot.ts b/client/src/utils/capture-screenshot.ts deleted file mode 100644 index 73e1f85..0000000 --- a/client/src/utils/capture-screenshot.ts +++ /dev/null @@ -1,165 +0,0 @@ -// In-call screenshot: composite the current camera tiles (and a screen share, -// if one is on stage) onto a high-resolution offscreen canvas and hand back a -// PNG blob. Used to copy the meeting view to the clipboard. -// -// We can't read pixels out of the live