From dcca32a780dd5ca2e476b2688ba524f4b9e177dd Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 31 Oct 2026 13:25:53 +0100 Subject: [PATCH] fix: give the Files recent and starred menus a Cancel and let Back close them --- .../__tests__/alert-buttons.test.ts | 68 +++++++++++ src/screens/FilesScreen.tsx | 115 ++++++++++++++---- 2 files changed, 162 insertions(+), 21 deletions(-) create mode 100644 src/components/__tests__/alert-buttons.test.ts diff --git a/src/components/__tests__/alert-buttons.test.ts b/src/components/__tests__/alert-buttons.test.ts new file mode 100644 index 00000000..8f2a81d2 --- /dev/null +++ b/src/components/__tests__/alert-buttons.test.ts @@ -0,0 +1,68 @@ +// An Android alert shows at most three buttons and drops the rest. The Files +// tab's recent-file menu was an Alert with Open, Show in folder, Remove from +// recent and Cancel: Cancel fell off, and since an Android alert also ignores +// Back and outside taps, the menu could only be left by picking an action. +// Menus with more than three choices are sheets. +import { describe, expect, it } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; + +const SRC = path.resolve(__dirname, '../..'); + +function sourceFiles(dir: string): string[] { + return fs.readdirSync(dir, { withFileTypes: true }).flatMap((e) => { + const p = path.join(dir, e.name); + if (e.isDirectory()) return e.name === '__tests__' ? [] : sourceFiles(p); + return /\.tsx?$/.test(e.name) ? [p] : []; + }); +} + +/** Each Alert.alert(...) call with its arguments. */ +function alertCalls(source: string): { call: string; line: number }[] { + const out: { call: string; line: number }[] = []; + let at = source.indexOf('Alert.alert('); + while (at >= 0) { + let depth = 0; + let i = at + 'Alert.alert'.length; + for (; i < source.length; i++) { + if (source[i] === '(') depth++; + else if (source[i] === ')' && --depth === 0) break; + } + out.push({ call: source.slice(at, i + 1), line: source.slice(0, at).split('\n').length }); + at = source.indexOf('Alert.alert(', i); + } + return out; +} + +function buttonCount(call: string): number { + return (call.match(/\btext:/g) ?? []).length; +} + +describe('alerts', () => { + it('have at most three buttons, all of which Android shows', () => { + const offenders = sourceFiles(SRC).flatMap((file) => + alertCalls(fs.readFileSync(file, 'utf8')) + .filter(({ call }) => buttonCount(call) > 3) + .map(({ line }) => `${path.relative(SRC, file)}:${line}`)); + expect(offenders).toEqual([]); + }); + + it('counts the buttons of a call', () => { + const [call] = alertCalls("Alert.alert(name, undefined, [{ text: t('a', 'A'), onPress: () => f(1) }, { text: 'B' }]);"); + expect(buttonCount(call.call)).toBe(2); + }); +}); + +describe('the Files recent and starred menus', () => { + const source = fs.readFileSync(path.join(SRC, 'screens/FilesScreen.tsx'), 'utf8'); + + it('open a sheet with a Cancel row that Back and an outside tap close', () => { + const body = source.slice(source.indexOf('function PlaceActionsSheet'), source.indexOf('interface FolderPickerSheetProps')); + expect(body).toContain('onRequestClose={onClose}'); + expect(body).toContain('onPress={onClose}'); + expect(body).toContain("t('files.cancel', 'Cancel')"); + const recent = source.slice(source.indexOf('const recentActions'), source.indexOf('const starredActions')); + expect(recent).toContain('setPlaceMenu('); + expect(recent).not.toContain('Alert.alert'); + }); +}); diff --git a/src/screens/FilesScreen.tsx b/src/screens/FilesScreen.tsx index 92aa7fbd..b62cab75 100644 --- a/src/screens/FilesScreen.tsx +++ b/src/screens/FilesScreen.tsx @@ -22,6 +22,7 @@ import { FileAudio, FileArchive, FileSpreadsheet, FileCode2, LayoutGrid, List as ListIcon, MoreVertical, Pencil, Share2, Trash2, Upload, Users, X, Download, Search, ArrowUpDown, ArrowUp, ArrowDown, FolderInput, Copy, Check, Eye, ExternalLink, FileSymlink, RotateCcw, Star, Clock, + FolderOpen, type AppIcon, } from '../components/icons'; import { useFocusEffect } from '@react-navigation/native'; import { File, Paths } from 'expo-file-system'; @@ -255,6 +256,8 @@ export default function FilesScreen() { const [renameTarget, setRenameTarget] = useState(null); const [renameValue, setRenameValue] = useState(''); const [actionsTarget, setActionsTarget] = useState(null); + // The menu of a starred folder or a recent file. + const [placeMenu, setPlaceMenu] = useState(null); const [shareTarget, setShareTarget] = useState(null); const [moveTarget, setMoveTarget] = useState(null); const [previewTarget, setPreviewTarget] = useState(null); @@ -1324,29 +1327,41 @@ export default function FilesScreen() { openFolderStack(place.stack); if (!reveal && !isSymlink(place.node)) openFile({ ...place.node, displayName: place.node.name }); }; + // A sheet, not an Alert: an Android alert shows at most three buttons, so + // the recent file's Cancel was dropped, and it ignores Back and outside + // taps. Same items as the webmail's sidebar menu (files-sidebar.tsx). const recentActions = (id: string, name: string) => { - Alert.alert(name, undefined, [ - { text: t('files.open', 'Open'), onPress: () => openRecent(id) }, - { text: t('files.show_in_folder', 'Show in folder'), onPress: () => openRecent(id, true) }, - { - text: t('files.remove_from_recent', 'Remove from recent'), - onPress: () => { - if (activeAccountId) useFilesPlacesStore.getState().removeRecentFiles(activeAccountId, [id]); + setPlaceMenu({ + title: name, + actions: [ + { key: 'open', label: t('files.open', 'Open'), icon: Eye, onPress: () => openRecent(id) }, + { key: 'reveal', label: t('files.show_in_folder', 'Show in folder'), icon: FolderOpen, onPress: () => openRecent(id, true) }, + { + key: 'remove', + label: t('files.remove_from_recent', 'Remove from recent'), + icon: X, + onPress: () => { + if (activeAccountId) useFilesPlacesStore.getState().removeRecentFiles(activeAccountId, [id]); + }, }, - }, - { text: t('files.cancel', 'Cancel'), style: 'cancel' }, - ]); + ], + }); }; - const starredActions = (folderPath: string, name: string) => { - Alert.alert(name, undefined, [ - { - text: t('files.unstar', 'Remove star'), - onPress: () => { - if (activeAccountId) useFilesPlacesStore.getState().toggleFavorite(activeAccountId, folderPath); + const starredActions = (folderPath: string, name: string, stack: FolderStep[]) => { + setPlaceMenu({ + title: name, + actions: [ + { key: 'open', label: t('files.open', 'Open'), icon: FolderOpen, onPress: () => openFolderStack(stack) }, + { + key: 'unstar', + label: t('files.unstar', 'Remove star'), + icon: Star, + onPress: () => { + if (activeAccountId) useFilesPlacesStore.getState().toggleFavorite(activeAccountId, folderPath); + }, }, - }, - { text: t('files.cancel', 'Cancel'), style: 'cancel' }, - ]); + ], + }); }; const placesHeader = showPlaces && (starredFolders.length > 0 || recentFiles.length > 0) ? ( @@ -1359,7 +1374,7 @@ export default function FilesScreen() { openFolderStack(stack)} - onLongPress={() => starredActions(folderPath, name)} + onLongPress={() => starredActions(folderPath, name, stack)} delayLongPress={300} style={({ pressed }) => [styles.fileRow, pressed && { backgroundColor: c.surfaceHover }]} accessibilityRole="button" @@ -1367,7 +1382,7 @@ export default function FilesScreen() { {name} starredActions(folderPath, name)} + onPress={() => starredActions(folderPath, name, stack)} hitSlop={8} style={styles.rowMore} accessibilityRole="button" @@ -1656,6 +1671,8 @@ export default function FilesScreen() { onPick={(destinationId) => moveTarget && void performMove(moveTarget, destinationId)} /> + setPlaceMenu(null)} /> + void; +} + +interface PlaceMenu { + title: string; + actions: PlaceAction[]; +} + +/** + * The actions of a starred folder or a recent file, with a Cancel row. Back + * and a tap outside close it too. + */ +function PlaceActionsSheet({ menu, t, onClose }: { menu: PlaceMenu | null; t: Translate; onClose: () => void }) { + // Centred on a wide window, clear of side insets in landscape. + const sheetInsets = useSheetInsets(); + const bottomInset = useSheetBottomInset(!!menu); + const c = useColors(); + const styles = React.useMemo(() => makeSheetStyles(c), [c]); + if (!menu) return null; + return ( + + + + + + {menu.title} + {menu.actions.map((a) => { + const Icon = a.icon; + return ( + { onClose(); a.onPress(); }} + > + + {a.label} + + ); + })} + + + {t('files.cancel', 'Cancel')} + + + + + + + ); +} + interface FolderPickerSheetProps { target: FileRow | null; allNodes: FileNode[];