From eb0a1d0ee0784f1d114854ffdbbe7f56cc1bdf10 Mon Sep 17 00:00:00 2001 From: Void Date: Thu, 12 Nov 2026 13:07:44 +0000 Subject: [PATCH 1/3] File zone share: panel state machine apart from React, with component tests Co-Authored-By: Claude Sonnet 5.5 --- docs/file-zone.md | 1 + src/app/features/albums/ShareSet.test.tsx | 99 ++++++++ src/app/features/albums/ShareSet.tsx | 176 +++++++------- .../features/albums/shareController.test.ts | 216 ++++++++++++++++++ src/app/features/albums/shareController.ts | 86 +++++++ src/app/features/albums/shareSet.ts | 2 +- 6 files changed, 494 insertions(+), 86 deletions(-) create mode 100644 src/app/features/albums/ShareSet.test.tsx create mode 100644 src/app/features/albums/shareController.test.ts create mode 100644 src/app/features/albums/shareController.ts diff --git a/docs/file-zone.md b/docs/file-zone.md index 7f380d69..7990485d 100644 --- a/docs/file-zone.md +++ b/docs/file-zone.md @@ -114,6 +114,7 @@ Matrix options checked: (a) a thread, chosen: stock, rendered by every client, a Client, `src/app/features/albums`: - `shareSet.ts`: `sendShareSet(port, title, files)`. Sends the root, then each file in pick order, each reply falling back to the previous event (`threadRelation`). A file whose upload fails is named in the result and the rest still go; a root that fails to send throws before any upload. The root alone is not an album in the list (`/threads` returns only roots that have a reply), so a share where every upload failed leaves a lone text message in the timeline. After a send the panel keeps only the failed files picked; a retry makes a second album. Ports are injected, so the order and failure rules are tested without a homeserver (`shareSet.test.ts` reads the sent events back through `buildAlbums`). +- `shareController.ts`: the panel's state machine (picked files, phase, the one-send-per-room guard, the encrypted refusal, failed files kept by position), apart from React so tests drive it without a DOM; `shareController.test.ts` and `ShareSet.test.tsx` (static render of each state, the flag) pin it. No DOM test environment exists in this repo, so a real tap on a real input is covered by the phone check, not by tests. - `ShareSet.tsx`: phone layout. Idle, it is one text row, "Share files". Picking files (any number from the native picker; a second pick replaces the first) mounts a title field, the picked names on one dim line, and Send / Cancel as full-width rows; progress is "Sending 2 of 5". It is shown only on the album list (kept mounted but hidden while an album is open, so a running send keeps its state), one send at a time per room (tracked outside the component; leaving the page does not stop a send, it only loses its progress and result), and never in an encrypted zone (re-checked when Send is tapped) (the plaintext `info.sha256` the Mind verifies against is written in unencrypted rooms only). - Content per file comes from `getMsgContent` in `features/room/msgContent.ts`, now also used by the composer, so a shared file and an attached file carry the same `info` including `sha256`. diff --git a/src/app/features/albums/ShareSet.test.tsx b/src/app/features/albums/ShareSet.test.tsx new file mode 100644 index 00000000..7f70591e --- /dev/null +++ b/src/app/features/albums/ShareSet.test.tsx @@ -0,0 +1,99 @@ +import React from 'react'; +import { renderToStaticMarkup } from 'react-dom/server'; +import { describe, expect, it } from 'vitest'; +import { ClientConfigProvider } from '../../hooks/useClientConfig'; +import type { ClientConfig } from '../../hooks/useClientConfig'; +import { ShareState } from './shareController'; +import { ShareSetView, shareEnabled, useShareEnabled } from './ShareSet'; + +const text = (html: string) => html.replace(/<[^>]*>/g, ' ').replace(/\s+/g, ' ').trim(); +const noop = () => undefined; +const file = (name: string) => ({ name, type: 'audio/ogg' } as File); + +const view = (over: Partial> = {}) => + renderToStaticMarkup( + + ); + +describe('the share panel, drawn', () => { + it('idle: one "Share files" row and no title, names, Send or Cancel', () => { + const html = view(); + expect(text(html)).toBe('Share files ›'); + expect(html).not.toContain('aria-label="Album title"'); + expect(html).toContain('type="file"'); + expect(html).toContain('multiple'); + expect(html).toContain('hidden'); + }); + + it('picked: the title field, the names on one line, Send N files and Cancel; no pick row', () => { + const html = view({ files: [file('a.ogg'), file('b.ogg')] }); + expect(html).toContain('aria-label="Album title"'); + expect(html).toContain('placeholder="Shared files"'); + expect(text(html)).toBe('a.ogg · b.ogg Send 2 files Cancel'); + expect(text(html)).not.toContain('Share files'); + }); + + it('one picked file reads "Send 1 file"', () => { + expect(text(view({ files: [file('a.ogg')] }))).toContain('Send 1 file Cancel'); + }); + + it('sending: progress replaces Send and Cancel, and the title is locked', () => { + const html = view({ files: [file('a'), file('b'), file('c')], phase: { kind: 'sending', done: 2 } }); + expect(text(html)).toContain('Sending 2 of 3…'); + expect(text(html)).not.toContain('Send 3 files'); + expect(text(html)).not.toContain('Cancel'); + expect(html).toMatch(/aria-label="Album title"[^>]*disabled|disabled[^>]*aria-label="Album title"/); + }); + + it('the encrypted note shows above the kept pick', () => { + const html = view({ + files: [file('a.ogg')], + phase: { kind: 'done', text: 'Not shared: this room is encrypted.' }, + }); + expect(text(html)).toBe('Not shared: this room is encrypted. a.ogg Send 1 file Cancel'); + }); + + it('after a full success the idle row and the note show together', () => { + expect(text(view({ phase: { kind: 'done', text: 'Shared 2 files.' } }))).toBe( + 'Share files › Shared 2 files.' + ); + }); +}); + +describe('the flag', () => { + it('off, missing or empty offers no share', () => { + expect(shareEnabled({}, false)).toBe(false); + expect(shareEnabled({ fileZoneShare: {} }, false)).toBe(false); + expect(shareEnabled({ fileZoneShare: { enabled: false } }, false)).toBe(false); + }); + + it('on offers it, except in an encrypted room', () => { + expect(shareEnabled({ fileZoneShare: { enabled: true } }, false)).toBe(true); + expect(shareEnabled({ fileZoneShare: { enabled: true } }, true)).toBe(false); + }); + + it('the hook reads the client config and the room, as AlbumView mounts it', () => { + const room = (encrypted: boolean) => ({ hasEncryptionStateEvent: () => encrypted } as never); + function Probe({ encrypted }: { encrypted: boolean }) { + return {String(useShareEnabled(room(encrypted)))}; + } + const probe = (config: ClientConfig, encrypted: boolean) => + text( + renderToStaticMarkup( + + + + ) + ); + expect(probe({}, false)).toBe('false'); + expect(probe({ fileZoneShare: { enabled: true } }, false)).toBe('true'); + expect(probe({ fileZoneShare: { enabled: true } }, true)).toBe('false'); + }); +}); diff --git a/src/app/features/albums/ShareSet.tsx b/src/app/features/albums/ShareSet.tsx index 1d9c4302..0e22fbee 100644 --- a/src/app/features/albums/ShareSet.tsx +++ b/src/app/features/albums/ShareSet.tsx @@ -1,4 +1,4 @@ -import React, { useRef, useState } from 'react'; +import React, { useMemo, useRef, useSyncExternalStore } from 'react'; import { Text } from 'folds'; import { useQueryClient } from '@tanstack/react-query'; import { Room } from 'matrix-js-sdk'; @@ -8,104 +8,55 @@ import { uploadContent } from '../../utils/matrix'; import { safeFile } from '../../utils/mimeTypes'; import { getMsgContent } from '../room/msgContent'; import { FILE_HASH_KEY, hashForSend } from './fileReceipt'; -import { DEFAULT_TITLE, remainingAfter, resultNote, sendShareSet } from './shareSet'; +import { ShareController, ShareState, createShareController } from './shareController'; +import { DEFAULT_TITLE, sendShareSet } from './shareSet'; import * as css from './AlbumView.css'; /** Off unless `fileZoneShare.enabled` is true. An encrypted zone is never offered the share. */ +export const shareEnabled = ( + config: { fileZoneShare?: { enabled?: boolean } }, + encrypted: boolean +) => !!config.fileZoneShare?.enabled && !encrypted; + export const useShareEnabled = (room: Room): boolean => - !!useClientConfig().fileZoneShare?.enabled && !room.hasEncryptionStateEvent(); + shareEnabled(useClientConfig(), room.hasEncryptionStateEvent()); -/** Rooms with a share in flight. Module-level: leaving the page unmounts the panel, not the send. */ -const sending = new Set(); - -type Phase = { kind: 'idle' } | { kind: 'sending'; done: number } | { kind: 'done'; text: string }; +type ShareViewProps = { + state: ShareState; + onPickClick: () => void; + onPicked: (files: File[]) => void; + onTitle: (title: string) => void; + onSend: () => void; + onCancel: () => void; + inputRef?: React.Ref; +}; /** * One row when idle. Picking files (any number, one picker) opens the panel: a title, the picked - * names, Send. Sending makes one album of them (shareSet.ts). Nothing here is visible until used. + * names, Send. Nothing but the row is visible until files are picked. */ -export function ShareSet({ room }: { room: Room }) { - const mx = useMatrixClient(); - const queryClient = useQueryClient(); - const input = useRef(null); - const [files, setFiles] = useState([]); - const [title, setTitle] = useState(''); - const [phase, setPhase] = useState({ kind: 'idle' }); - - const reset = () => { - setFiles([]); - setTitle(''); - if (input.current) input.current.value = ''; - }; - - const send = async () => { - // One send at a time, and never into a room that has turned encrypted since the panel mounted. - if (sending.has(room.roomId)) return; - if (room.hasEncryptionStateEvent()) { - setPhase({ kind: 'done', text: 'Not shared: this room is encrypted.' }); - return; - } - sending.add(room.roomId); - const items = files.map((file) => ({ name: file.name, mimeType: file.type, file })); - setPhase({ kind: 'sending', done: 0 }); - let settled = 0; - try { - const res = await sendShareSet( - { - send: async (content) => (await mx.sendMessage(room.roomId, content as any)).event_id, - contentOf: async (f) => { - const file = safeFile(f.file); - const mxc = await new Promise((resolve, reject) => { - uploadContent(mx, file, { onSuccess: resolve, onError: reject }); - }); - const content = await getMsgContent( - mx, - { - file, - originalFile: f.file, - metadata: { markedAsSpoiler: false }, - encInfo: undefined, - }, - mxc - ); - const sha256 = await hashForSend(f.file); - if (sha256) content.info = { ...content.info, [FILE_HASH_KEY]: sha256 }; - return content; - }, - onFile: () => { - settled += 1; - setPhase({ kind: 'sending', done: settled }); - }, - }, - title, - items - ); - queryClient.invalidateQueries({ queryKey: ['albums', room.roomId] }); - const left = remainingAfter(items, res).map((i) => i.file); - setFiles(left); - if (left.length === 0) reset(); - setPhase({ kind: 'done', text: resultNote(res) }); - } catch (e: any) { - setPhase({ kind: 'done', text: `Could not share: ${e?.message ?? 'send failed'}` }); - } finally { - sending.delete(room.roomId); - } - }; - +export function ShareSetView({ + state, + onPickClick, + onPicked, + onTitle, + onSend, + onCancel, + inputRef, +}: ShareViewProps) { + const { files, title, phase, epoch } = state; return (
{ - setFiles(Array.from(e.target.files ?? [])); - setPhase({ kind: 'idle' }); - }} + onChange={(e) => onPicked(Array.from(e.target.files ?? []))} /> {files.length === 0 && phase.kind !== 'sending' && ( - )} @@ -122,7 +73,7 @@ export function ShareSet({ room }: { room: Room }) { placeholder={DEFAULT_TITLE} aria-label="Album title" disabled={phase.kind === 'sending'} - onChange={(e) => setTitle(e.target.value)} + onChange={(e) => onTitle(e.target.value)} /> {files.map((f) => f.name).join(' · ')} @@ -133,12 +84,12 @@ export function ShareSet({ room }: { room: Room }) { ) : ( <> - - @@ -148,3 +99,58 @@ export function ShareSet({ room }: { room: Room }) {
); } + +export function ShareSet({ room }: { room: Room }) { + const mx = useMatrixClient(); + const queryClient = useQueryClient(); + const input = useRef(null); + const controller: ShareController = useMemo( + () => + createShareController({ + roomId: room.roomId, + isEncrypted: () => room.hasEncryptionStateEvent(), + onSent: () => queryClient.invalidateQueries({ queryKey: ['albums', room.roomId] }), + run: (files, title, onFile) => + sendShareSet( + { + send: async (content) => (await mx.sendMessage(room.roomId, content as any)).event_id, + contentOf: async (f) => { + const file = safeFile(f.file); + const mxc = await new Promise((resolve, reject) => { + uploadContent(mx, file, { onSuccess: resolve, onError: reject }); + }); + const content = await getMsgContent( + mx, + { + file, + originalFile: f.file, + metadata: { markedAsSpoiler: false }, + encInfo: undefined, + }, + mxc + ); + const sha256 = await hashForSend(f.file); + if (sha256) content.info = { ...content.info, [FILE_HASH_KEY]: sha256 }; + return content; + }, + onFile, + }, + title, + files.map((file) => ({ name: file.name, mimeType: file.type, file })) + ), + }), + [mx, room, queryClient] + ); + const state = useSyncExternalStore(controller.subscribe, controller.getState); + return ( + input.current?.click()} + onPicked={controller.pick} + onTitle={controller.setTitle} + onSend={controller.send} + onCancel={controller.cancel} + /> + ); +} diff --git a/src/app/features/albums/shareController.test.ts b/src/app/features/albums/shareController.test.ts new file mode 100644 index 00000000..813c65b2 --- /dev/null +++ b/src/app/features/albums/shareController.test.ts @@ -0,0 +1,216 @@ +import { describe, expect, it } from 'vitest'; +import { ENCRYPTED_NOTE, Picked, ShareDeps, createShareController } from './shareController'; +import { ShareResult } from './shareSet'; + +const file = (name: string): Picked => ({ name, type: 'audio/ogg' }); +const ok = (n: number): ShareResult => ({ albumId: '$r', sent: n, failed: [], failedAt: [] }); + +/** A run the test settles by hand, so "in flight" is a state the test can stand in. */ +const hand = () => { + const calls: { files: Picked[]; title: string; onFile: () => void }[] = []; + let release: (r: ShareResult) => void = () => undefined; + let fail: (e: Error) => void = () => undefined; + const run: ShareDeps['run'] = (files, title, onFile) => { + calls.push({ files, title, onFile }); + return new Promise((resolve, reject) => { + release = resolve; + fail = reject; + }); + }; + return { calls, run, release: (r: ShareResult) => release(r), fail: (e: Error) => fail(e) }; +}; + +const make = (over: Partial> = {}, roomId = '!zone:x') => { + const h = hand(); + const sentTo: string[] = []; + const c = createShareController({ + roomId, + isEncrypted: () => false, + run: h.run, + onSent: () => sentTo.push(roomId), + ...over, + }); + return { c, h, sentTo }; +}; + +const tick = () => new Promise((r) => setTimeout(r, 0)); + +describe('picking', () => { + it('starts idle with nothing picked, and a pick holds the files in order', () => { + const { c } = make(); + expect(c.getState().files).toEqual([]); + expect(c.getState().phase).toEqual({ kind: 'idle' }); + c.pick([file('a'), file('b')]); + expect(c.getState().files.map((f) => f.name)).toEqual(['a', 'b']); + }); + + it('Cancel empties the pick and the title and bumps the input epoch', () => { + const { c } = make(); + c.pick([file('a')]); + c.setTitle('Take 4'); + const before = c.getState().epoch; + c.cancel(); + expect(c.getState()).toMatchObject({ files: [], title: '', epoch: before + 1 }); + }); + + it('subscribers hear every change and stop hearing after unsubscribe', () => { + const { c } = make(); + let heard = 0; + const off = c.subscribe(() => { + heard += 1; + }); + c.pick([file('a')]); + off(); + c.pick([]); + expect(heard).toBe(1); + }); +}); + +describe('the Send guard', () => { + it('a double tap starts one send', async () => { + const { c, h } = make(); + c.pick([file('a'), file('b')]); + const first = c.send(); + const second = c.send(); + expect(h.calls).toHaveLength(1); + h.release(ok(2)); + await Promise.all([first, second]); + expect(h.calls).toHaveLength(1); + }); + + it('a re-mounted panel for the same room cannot start a second send while one is in flight', async () => { + const { c, h } = make(); + c.pick([file('a')]); + const first = c.send(); + // The page was left and returned to: a new controller, nothing picked in it but a new pick. + const again = make({}, '!zone:x'); + again.c.pick([file('b')]); + await again.c.send(); + expect(again.h.calls).toHaveLength(0); + h.release(ok(1)); + await first; + // Once the first has settled the room is free again. + const next = again.c.send(); + expect(again.h.calls).toHaveLength(1); + again.h.release(ok(1)); + await next; + }); + + it('another room is not blocked by a send in flight', async () => { + const a = make({}, '!a:x'); + const b = make({}, '!b:x'); + a.c.pick([file('a')]); + b.c.pick([file('b')]); + const sendA = a.c.send(); + const sendB = b.c.send(); + expect(b.h.calls).toHaveLength(1); + a.h.release(ok(1)); + b.h.release(ok(1)); + await Promise.all([sendA, sendB]); + }); + + it('sends nothing when nothing is picked', async () => { + const { c, h } = make(); + await c.send(); + expect(h.calls).toHaveLength(0); + }); + + it('a send that throws frees the room and says so', async () => { + const { c, h } = make(); + c.pick([file('a')]); + const p = c.send(); + h.fail(new Error('M_FORBIDDEN')); + await p; + expect(c.getState().phase).toEqual({ kind: 'done', text: 'Could not share: M_FORBIDDEN' }); + expect(c.getState().files).toHaveLength(1); + const retry = c.send(); + expect(h.calls).toHaveLength(2); + h.release(ok(1)); + await retry; + }); +}); + +describe('progress and results', () => { + it('counts files as they settle, passing the title through', async () => { + const { c, h } = make(); + c.pick([file('a'), file('b')]); + c.setTitle('Take 4'); + const p = c.send(); + expect(c.getState().phase).toEqual({ kind: 'sending', done: 0 }); + expect(h.calls[0].title).toBe('Take 4'); + h.calls[0].onFile(); + expect(c.getState().phase).toEqual({ kind: 'sending', done: 1 }); + h.release(ok(2)); + await p; + expect(c.getState().phase).toEqual({ kind: 'done', text: 'Shared 2 files.' }); + }); + + it('full success clears the pick, tells the room list, and remounts the input', async () => { + const { c, h, sentTo } = make(); + c.pick([file('a')]); + const epoch = c.getState().epoch; + const p = c.send(); + h.release(ok(1)); + await p; + expect(c.getState()).toMatchObject({ files: [], title: '', epoch: epoch + 1 }); + expect(sentTo).toEqual(['!zone:x']); + }); + + it('failed files stay picked by position, so a same-named twin that went is not re-sent', async () => { + const { c, h } = make(); + const twins = [file('a.ogg'), file('a.ogg'), file('b.ogg')]; + c.pick(twins); + const p = c.send(); + h.release({ albumId: '$r', sent: 2, failed: ['a.ogg'], failedAt: [1] }); + await p; + expect(c.getState().files).toEqual([twins[1]]); + expect(c.getState().phase).toEqual({ kind: 'done', text: 'Shared 2. Not sent: a.ogg' }); + }); + + it('when every upload failed everything stays picked and the note says nothing was shared', async () => { + const { c, h } = make(); + const picked = [file('a'), file('b')]; + c.pick(picked); + const p = c.send(); + h.release({ albumId: '$r', sent: 0, failed: ['a', 'b'], failedAt: [0, 1] }); + await p; + expect(c.getState().files).toEqual(picked); + expect(c.getState().phase).toEqual({ + kind: 'done', + text: 'Nothing was shared. Not sent: a, b', + }); + }); + + it('a new pick after a result drops the result note', async () => { + const { c, h } = make(); + c.pick([file('a')]); + const p = c.send(); + h.release(ok(1)); + await p; + c.pick([file('b')]); + expect(c.getState().phase).toEqual({ kind: 'idle' }); + }); +}); + +describe('an encrypted room', () => { + it('says so, sends nothing and keeps the pick', async () => { + const { c, h } = make({ isEncrypted: () => true }); + c.pick([file('a')]); + await c.send(); + await tick(); + expect(ENCRYPTED_NOTE).toBe('Not shared: this room is encrypted.'); + expect(c.getState().phase).toEqual({ kind: 'done', text: ENCRYPTED_NOTE }); + expect(c.getState().files).toHaveLength(1); + expect(h.calls).toHaveLength(0); + }); + + it('is checked at Send, not at mount: a room that turns encrypted after the pick is refused', async () => { + let encrypted = false; + const { c, h } = make({ isEncrypted: () => encrypted }); + c.pick([file('a')]); + encrypted = true; + await c.send(); + expect(h.calls).toHaveLength(0); + expect(c.getState().phase).toMatchObject({ kind: 'done', text: ENCRYPTED_NOTE }); + }); +}); diff --git a/src/app/features/albums/shareController.ts b/src/app/features/albums/shareController.ts new file mode 100644 index 00000000..6674b5c7 --- /dev/null +++ b/src/app/features/albums/shareController.ts @@ -0,0 +1,86 @@ +// The share panel's state machine, apart from React and Matrix: what is picked, what phase the +// send is in, and the rules that keep a room to one send at a time. ShareSet.tsx draws it. +import { ShareResult, remainingAfter, resultNote } from './shareSet'; + +export type Picked = { name: string; type: string }; + +export type SharePhase = + | { kind: 'idle' } + | { kind: 'sending'; done: number } + | { kind: 'done'; text: string }; + +export type ShareState = { + files: F[]; + title: string; + phase: SharePhase; + /** Bumps whenever the pick is emptied, so the file input can be remounted and re-pick the same files. */ + epoch: number; +}; + +export const ENCRYPTED_NOTE = 'Not shared: this room is encrypted.'; + +export type ShareDeps = { + roomId: string; + isEncrypted: () => boolean; + /** Sends the files as one album; reports each settled file through onFile. */ + run: (files: F[], title: string, onFile: () => void) => Promise; + /** Called after a send that reached the room, whatever it left unsent. */ + onSent?: () => void; +}; + +/** Rooms with a share in flight. Module-level: leaving the page unmounts the panel, not the send. */ +const sending = new Set(); + +export const createShareController = (deps: ShareDeps) => { + let state: ShareState = { files: [], title: '', phase: { kind: 'idle' }, epoch: 0 }; + const listeners = new Set<() => void>(); + const set = (patch: Partial>) => { + state = { ...state, ...patch }; + listeners.forEach((l) => l()); + }; + + const clear = () => set({ files: [], title: '', epoch: state.epoch + 1 }); + + const send = async () => { + if (sending.has(deps.roomId) || state.files.length === 0) return; + if (deps.isEncrypted()) { + set({ phase: { kind: 'done', text: ENCRYPTED_NOTE } }); + return; + } + sending.add(deps.roomId); + const { files, title } = state; + set({ phase: { kind: 'sending', done: 0 } }); + let settled = 0; + try { + const res = await deps.run(files, title, () => { + settled += 1; + set({ phase: { kind: 'sending', done: settled } }); + }); + deps.onSent?.(); + const left = remainingAfter(files, res); + if (left.length === 0) clear(); + else set({ files: left }); + set({ phase: { kind: 'done', text: resultNote(res) } }); + } catch (e: any) { + set({ phase: { kind: 'done', text: `Could not share: ${e?.message ?? 'send failed'}` } }); + } finally { + sending.delete(deps.roomId); + } + }; + + return { + getState: () => state, + subscribe: (l: () => void) => { + listeners.add(l); + return () => { + listeners.delete(l); + }; + }, + pick: (files: F[]) => set({ files, phase: { kind: 'idle' } }), + setTitle: (title: string) => set({ title }), + cancel: clear, + send, + }; +}; + +export type ShareController = ReturnType>; diff --git a/src/app/features/albums/shareSet.ts b/src/app/features/albums/shareSet.ts index 803fbb87..3e572595 100644 --- a/src/app/features/albums/shareSet.ts +++ b/src/app/features/albums/shareSet.ts @@ -72,7 +72,7 @@ export const sendShareSet = async ( * makes a second album (adding to an existing one is not built), and a share where nothing went * leaves its root as a lone message the album list does not show (it lists threads with replies). */ -export const remainingAfter = (files: F[], result: ShareResult): F[] => +export const remainingAfter = (files: T[], result: ShareResult): T[] => files.filter((_, i) => result.failedAt.includes(i)); export const resultNote = (result: ShareResult): string => { -- 2.47.3 From 86a68839b02bfe45c5673de2d91e4139a9edf83b Mon Sep 17 00:00:00 2001 From: Void Date: Thu, 12 Nov 2026 13:08:20 +0000 Subject: [PATCH 2/3] File zone share tests: lint Co-Authored-By: Claude Sonnet 5.5 --- src/app/features/albums/shareController.test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/app/features/albums/shareController.test.ts b/src/app/features/albums/shareController.test.ts index 813c65b2..f29091dc 100644 --- a/src/app/features/albums/shareController.test.ts +++ b/src/app/features/albums/shareController.test.ts @@ -33,8 +33,6 @@ const make = (over: Partial> = {}, roomId = '!zone:x') => { return { c, h, sentTo }; }; -const tick = () => new Promise((r) => setTimeout(r, 0)); - describe('picking', () => { it('starts idle with nothing picked, and a pick holds the files in order', () => { const { c } = make(); @@ -148,7 +146,7 @@ describe('progress and results', () => { it('full success clears the pick, tells the room list, and remounts the input', async () => { const { c, h, sentTo } = make(); c.pick([file('a')]); - const epoch = c.getState().epoch; + const { epoch } = c.getState(); const p = c.send(); h.release(ok(1)); await p; @@ -197,7 +195,6 @@ describe('an encrypted room', () => { const { c, h } = make({ isEncrypted: () => true }); c.pick([file('a')]); await c.send(); - await tick(); expect(ENCRYPTED_NOTE).toBe('Not shared: this room is encrypted.'); expect(c.getState().phase).toEqual({ kind: 'done', text: ENCRYPTED_NOTE }); expect(c.getState().files).toHaveLength(1); -- 2.47.3 From cf99096d3c1a2e5bdbfe52347c9863faab38e653 Mon Sep 17 00:00:00 2001 From: Void Date: Thu, 12 Nov 2026 13:15:47 +0000 Subject: [PATCH 3/3] File zone share: controller built once per mount, one notification on success, tests for the surviving mutations Co-Authored-By: Claude Sonnet 5.5 --- src/app/features/albums/ShareSet.test.tsx | 15 +++- src/app/features/albums/ShareSet.tsx | 76 ++++++++++--------- .../features/albums/shareController.test.ts | 56 +++++++++++++- src/app/features/albums/shareController.ts | 7 +- 4 files changed, 108 insertions(+), 46 deletions(-) diff --git a/src/app/features/albums/ShareSet.test.tsx b/src/app/features/albums/ShareSet.test.tsx index 7f70591e..bfa9a619 100644 --- a/src/app/features/albums/ShareSet.test.tsx +++ b/src/app/features/albums/ShareSet.test.tsx @@ -6,7 +6,11 @@ import type { ClientConfig } from '../../hooks/useClientConfig'; import { ShareState } from './shareController'; import { ShareSetView, shareEnabled, useShareEnabled } from './ShareSet'; -const text = (html: string) => html.replace(/<[^>]*>/g, ' ').replace(/\s+/g, ' ').trim(); +const text = (html: string) => + html + .replace(/<[^>]*>/g, ' ') + .replace(/\s+/g, ' ') + .trim(); const noop = () => undefined; const file = (name: string) => ({ name, type: 'audio/ogg' } as File); @@ -45,11 +49,16 @@ describe('the share panel, drawn', () => { }); it('sending: progress replaces Send and Cancel, and the title is locked', () => { - const html = view({ files: [file('a'), file('b'), file('c')], phase: { kind: 'sending', done: 2 } }); + const html = view({ + files: [file('a'), file('b'), file('c')], + phase: { kind: 'sending', done: 2 }, + }); expect(text(html)).toContain('Sending 2 of 3…'); expect(text(html)).not.toContain('Send 3 files'); expect(text(html)).not.toContain('Cancel'); - expect(html).toMatch(/aria-label="Album title"[^>]*disabled|disabled[^>]*aria-label="Album title"/); + expect(html).toMatch( + /aria-label="Album title"[^>]*disabled|disabled[^>]*aria-label="Album title"/ + ); }); it('the encrypted note shows above the kept pick', () => { diff --git a/src/app/features/albums/ShareSet.tsx b/src/app/features/albums/ShareSet.tsx index 0e22fbee..7b507d92 100644 --- a/src/app/features/albums/ShareSet.tsx +++ b/src/app/features/albums/ShareSet.tsx @@ -1,4 +1,4 @@ -import React, { useMemo, useRef, useSyncExternalStore } from 'react'; +import React, { useRef, useState, useSyncExternalStore } from 'react'; import { Text } from 'folds'; import { useQueryClient } from '@tanstack/react-query'; import { Room } from 'matrix-js-sdk'; @@ -104,42 +104,46 @@ export function ShareSet({ room }: { room: Room }) { const mx = useMatrixClient(); const queryClient = useQueryClient(); const input = useRef(null); - const controller: ShareController = useMemo( - () => - createShareController({ - roomId: room.roomId, - isEncrypted: () => room.hasEncryptionStateEvent(), - onSent: () => queryClient.invalidateQueries({ queryKey: ['albums', room.roomId] }), - run: (files, title, onFile) => - sendShareSet( - { - send: async (content) => (await mx.sendMessage(room.roomId, content as any)).event_id, - contentOf: async (f) => { - const file = safeFile(f.file); - const mxc = await new Promise((resolve, reject) => { - uploadContent(mx, file, { onSuccess: resolve, onError: reject }); - }); - const content = await getMsgContent( - mx, - { - file, - originalFile: f.file, - metadata: { markedAsSpoiler: false }, - encInfo: undefined, - }, - mxc - ); - const sha256 = await hashForSend(f.file); - if (sha256) content.info = { ...content.info, [FILE_HASH_KEY]: sha256 }; - return content; - }, - onFile, + // Built once per mount (AlbumView keys the panel by room): the pick lives in the controller, so a + // rerender with a new client or Room object must not build another. Live values go through `live`. + const live = useRef({ mx, room, queryClient }); + live.current = { mx, room, queryClient }; + const [controller]: [ShareController, unknown] = useState(() => + createShareController({ + roomId: room.roomId, + isEncrypted: () => live.current.room.hasEncryptionStateEvent(), + onSent: () => + live.current.queryClient.invalidateQueries({ queryKey: ['albums', room.roomId] }), + run: (files, title, onFile) => + sendShareSet( + { + send: async (content) => + (await live.current.mx.sendMessage(room.roomId, content as any)).event_id, + contentOf: async (f) => { + const file = safeFile(f.file); + const mxc = await new Promise((resolve, reject) => { + uploadContent(live.current.mx, file, { onSuccess: resolve, onError: reject }); + }); + const content = await getMsgContent( + live.current.mx, + { + file, + originalFile: f.file, + metadata: { markedAsSpoiler: false }, + encInfo: undefined, + }, + mxc + ); + const sha256 = await hashForSend(f.file); + if (sha256) content.info = { ...content.info, [FILE_HASH_KEY]: sha256 }; + return content; }, - title, - files.map((file) => ({ name: file.name, mimeType: file.type, file })) - ), - }), - [mx, room, queryClient] + onFile, + }, + title, + files.map((file) => ({ name: file.name, mimeType: file.type, file })) + ), + }) ); const state = useSyncExternalStore(controller.subscribe, controller.getState); return ( diff --git a/src/app/features/albums/shareController.test.ts b/src/app/features/albums/shareController.test.ts index f29091dc..3cbc03ce 100644 --- a/src/app/features/albums/shareController.test.ts +++ b/src/app/features/albums/shareController.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { ENCRYPTED_NOTE, Picked, ShareDeps, createShareController } from './shareController'; +import { Picked, ShareDeps, createShareController } from './shareController'; import { ShareResult } from './shareSet'; const file = (name: string): Picked => ({ name, type: 'audio/ogg' }); @@ -190,13 +190,58 @@ describe('progress and results', () => { }); }); +describe('what a send leaves behind', () => { + it('keeps the title when some files failed, so the retry has the same name', async () => { + const { c, h } = make(); + c.pick([file('a'), file('b')]); + c.setTitle('Take 4'); + const p = c.send(); + h.release({ albumId: '$r', sent: 1, failed: ['b'], failedAt: [1] }); + await p; + expect(c.getState().title).toBe('Take 4'); + }); + + it('tells the room list only after the send settled, and not when it threw', async () => { + const { c, h, sentTo } = make(); + c.pick([file('a')]); + const p = c.send(); + expect(sentTo).toEqual([]); + h.release(ok(1)); + await p; + expect(sentTo).toEqual(['!zone:x']); + + const bad = make(); + bad.c.pick([file('a')]); + const q = bad.c.send(); + bad.h.fail(new Error('boom')); + await q; + expect(bad.sentTo).toEqual([]); + }); + + it('a success lands in one notification: the pick is never empty while still sending', async () => { + const { c, h } = make(); + c.pick([file('a')]); + const p = c.send(); + const seen: string[] = []; + c.subscribe(() => { + const st = c.getState(); + seen.push(`${st.files.length}:${st.phase.kind}`); + }); + h.release(ok(1)); + await p; + expect(seen).toEqual(['0:done']); + }); +}); + describe('an encrypted room', () => { it('says so, sends nothing and keeps the pick', async () => { const { c, h } = make({ isEncrypted: () => true }); c.pick([file('a')]); await c.send(); - expect(ENCRYPTED_NOTE).toBe('Not shared: this room is encrypted.'); - expect(c.getState().phase).toEqual({ kind: 'done', text: ENCRYPTED_NOTE }); + expect(c.getState().phase).toEqual({ + kind: 'done', + text: 'Not shared: this room is encrypted.', + }); expect(c.getState().files).toHaveLength(1); expect(h.calls).toHaveLength(0); }); @@ -208,6 +253,9 @@ describe('an encrypted room', () => { encrypted = true; await c.send(); expect(h.calls).toHaveLength(0); - expect(c.getState().phase).toMatchObject({ kind: 'done', text: ENCRYPTED_NOTE }); + expect(c.getState().phase).toMatchObject({ + kind: 'done', + text: 'Not shared: this room is encrypted.', + }); }); }); diff --git a/src/app/features/albums/shareController.ts b/src/app/features/albums/shareController.ts index 6674b5c7..b6219c72 100644 --- a/src/app/features/albums/shareController.ts +++ b/src/app/features/albums/shareController.ts @@ -58,9 +58,10 @@ export const createShareController = (deps: ShareDeps) => { }); deps.onSent?.(); const left = remainingAfter(files, res); - if (left.length === 0) clear(); - else set({ files: left }); - set({ phase: { kind: 'done', text: resultNote(res) } }); + const phase: SharePhase = { kind: 'done', text: resultNote(res) }; + // One notification: no state is ever drawn with the pick emptied and the send still running. + if (left.length === 0) set({ files: [], title: '', epoch: state.epoch + 1, phase }); + else set({ files: left, phase }); } catch (e: any) { set({ phase: { kind: 'done', text: `Could not share: ${e?.message ?? 'send failed'}` } }); } finally { -- 2.47.3