diff --git a/app/src/DashboardPage.js b/app/src/DashboardPage.js
index 2ff83051..3faf7a5e 100644
--- a/app/src/DashboardPage.js
+++ b/app/src/DashboardPage.js
@@ -30,7 +30,7 @@ import {ProgressPlaceholder} from "./components/Placeholder";
import {refreshFiles} from "./api";
import _ from "lodash";
import {TagsDashboard} from "./Tags";
-import {Upload} from "./components/Upload";
+import {UploadModal} from "./components/Upload";
import {SearchView, useSearch, useSearchSuggestions} from "./components/Search";
import {OutdatedZimsMessage} from "./components/Zim";
import {useSearchFilter, useSearchRecentFiles, useWROLMode} from "./hooks/customHooks";
@@ -269,16 +269,13 @@ export function Getters() {
;
} else if (selectedGetter === 'upload') {
- getterModal = handleSetGetter(null, null)}
- >
- Upload from your device
-
-
-
- ;
+ // The same dialog the file browser opens; it asks for a destination because, unlike the
+ // browser, the dashboard has no selected directory to upload into.
+ getterModal = handleSetGetter(null, null)}
+ disabled={gettersDisabled}
+ />;
}
if (wrolModeEnabled) {
diff --git a/app/src/components/FileBrowser.js b/app/src/components/FileBrowser.js
index 371e4e27..cd267f67 100644
--- a/app/src/components/FileBrowser.js
+++ b/app/src/components/FileBrowser.js
@@ -7,11 +7,8 @@ import {
IconButton,
IconStack,
Modal,
- Panel,
Placeholder,
- Progress,
Skeleton,
- Stack,
Table,
TextInput,
} from "./ui";
@@ -21,24 +18,22 @@ import {
ErrorMessage,
FileIcon,
humanFileSize,
- mimetypeIconName,
- Toggle,
useIsIgnoredDirectory
} from "./Common";
import React, {useContext, useEffect, useState} from "react";
import {IconArrowsMove, IconChevronsUp, IconCursorText, IconTags} from "@tabler/icons-react";
-import {useDropzone} from "react-dropzone";
import {useHotkeys} from "react-hotkeys-hook";
import {deleteFile, ignoreDirectory, makeDirectory, movePaths, renamePath, unignoreDirectory} from "../api";
import _ from 'lodash';
import {SortableTable} from "./SortableTable";
-import {useBrowseFiles, useElementWidth, useMediaDirectory, useUploadFile, useWROLMode} from "../hooks/customHooks";
+import {useBrowseFiles, useElementWidth, useMediaDirectory, useWROLMode} from "../hooks/customHooks";
import {FileRowTagIcon, FilesRefreshButton} from "./Files";
import {FilePreviewContext} from "./FilePreview";
import {DragSelectionContext, DragSelectionProvider, SettingsContext, ThemeContext} from "../contexts/contexts";
import {BulkTagModal} from "./BulkTagModal";
import {InlineErrorBoundary} from "./ErrorBoundary";
import {TaggedDeleteConfirmModal} from "./TaggedDeleteConfirmModal";
+import {UploadModal} from "./Upload";
function depthIndentation(path) {
// Repeated spaces for every folder a path is in.
@@ -456,7 +451,7 @@ export function FileBrowser() {
disabled={wrolModeEnabled || (selectedPathsCount > 0 && !singleDirectorySelected)}
onClick={() => setUploadOpen(true)}
>{label('Upload')}
- setUploadOpen(false)}
destination={singleDirectorySelected && selectedPaths[0] ? selectedPaths[0].slice(0, -1) : ''}
@@ -847,146 +842,3 @@ export function MakeDirectoryModal({open, onClose, parent, onSubmit}) {
}
-
-export function FileBrowserUploadModal({open, onClose, destination, onComplete}) {
- const mediaDirectory = useMediaDirectory();
-
- const {
- setFiles,
- progresses,
- setDestination,
- doClear,
- tagsSelector,
- overwrite,
- setOverwrite,
- overallProgress,
- inProgress,
- } = useUploadFile();
-
- const onDrop = React.useCallback(async (acceptedFiles) => {
- if (acceptedFiles && acceptedFiles.length > 0) {
- setFiles(acceptedFiles);
- }
- }, [setFiles]);
-
- const {getRootProps, getInputProps} = useDropzone({onDrop, disabled: inProgress});
-
- // Set destination when modal opens or destination changes
- useEffect(() => {
- if (open) {
- setDestination(destination);
- }
- }, [open, destination, setDestination]);
-
- // Clear upload state when destination is cleared
- useEffect(() => {
- if (!destination && open) {
- doClear();
- }
- }, [destination, open, doClear]);
-
- const handleClose = () => {
- if (!inProgress) {
- doClear();
- onClose();
- if (onComplete) {
- onComplete();
- }
- }
- };
-
- let progressBars;
- if (!_.isEmpty(progresses)) {
- progressBars = Object.entries(progresses).map(([name, value]) => {
- const {percent, status, type} = value;
- let color = 'grey';
- let statusString;
- if (status === 'complete') {
- color = 'green';
- statusString = 'Complete:';
- } else if (status === 'pending') {
- statusString = 'Pending:';
- } else if (status === 'failed') {
- color = 'red';
- statusString = 'Failed:';
- } else if (status === 'conflicting') {
- color = 'orange';
- statusString = 'Already Exists:';
- }
-
- return
-
-
-
-
-
- })
- }
-
- const displayPath = destination ? `${mediaDirectory}/${destination}` : mediaDirectory;
-
- /*
- * Fullscreen, for the drop target. At 620px the target was a strip about 90px tall in a
- * dialog covering a fifth of the screen -- a small thing to drag a file onto, and the
- * page behind it was the file browser it came from. Given the room, the target takes it.
- */
- return
- Upload to: {displayPath}
-
-
-
- {/* The target takes the room the dialog gives it rather than sitting as a strip
- at the top of it. `min-height` as well as `flex`, so it stays a large target
- once the progress bars below it start claiming their own space. */}
-
-
}
-
-
- {progressBars}
-
-
-
-
-
-
-}
diff --git a/app/src/components/Upload.js b/app/src/components/Upload.js
index ce59bc3c..f8a7cb76 100644
--- a/app/src/components/Upload.js
+++ b/app/src/components/Upload.js
@@ -1,39 +1,22 @@
import React from "react";
import {useDropzone} from "react-dropzone";
-import {Box, Group, Header, Icon, Panel, Progress, Stack} from "./ui";
+import {Box, Button, Group, Header, Icon, Modal, Panel, Progress, Stack} from "./ui";
import {DirectorySearch, mimetypeIconName, RequiredAsterisk, Toggle} from "./Common";
-import {useUploadFile} from "../hooks/customHooks";
+import {useMediaDirectory, useUploadFile} from "../hooks/customHooks";
import _ from "lodash";
-export function Upload({disabled}) {
- const {
- setFiles,
- progresses,
- destination,
- setDestination,
- doClear,
- tagsSelector,
- overwrite,
- setOverwrite,
- overallProgress,
- inProgress,
- } = useUploadFile();
-
- const onDrop = React.useCallback(async (acceptedFiles) => {
- if (acceptedFiles && acceptedFiles.length > 0) {
- setFiles(acceptedFiles);
- }
- }, [destination]);
-
- const {getRootProps, getInputProps} = useDropzone({onDrop, disabled: inProgress});
-
- React.useEffect(() => {
- if (!destination) {
- // Destination was cleared, clear progresses.
- doClear();
- }
- }, [destination]);
+/*
+ * `destination` says where the files go:
+ *
+ * undefined -- ask, with a DirectorySearch.
+ * a string -- upload there. '' is the media directory, a real destination; a falsy check
+ * against it reads the media root as "nowhere".
+ */
+export function UploadForm({
+ askForDestination, destination, setDestination, disabled, inProgress, progresses, overallProgress,
+ tagsSelector, overwrite, setOverwrite, getRootProps, getInputProps,
+}) {
let progressBars;
if (!_.isEmpty(progresses)) {
progressBars = Object.entries(progresses).map(([name, value]) => {
@@ -62,9 +45,18 @@ export function Upload({disabled}) {
})
}
+ // A destination that was asked for and not yet given: there is nowhere to put a file.
+ const canDrop = !askForDestination || !!destination;
+
+ /*
+ * A fragment, and the `
- {destination ?
-
-
+ {/* The target grows into the dialog's leftover height. `min-height` as well as
+ `flex`, so it stays a large target once progress bars claim their own space. */}
+ {canDrop ?
+
+
-
- Click here, or drop files here to upload
-
+ Click here, or drop files here to upload
- : You must search for a directory to place your files
+ // The instruction takes the room the target would, rather than sitting as a line
+ // above an empty dialog.
+ :
+ You must search for a directory to place your files
+
}
-
+ {progresses && Object.keys(progresses).length > 1 &&
+
+
+
}
- {progresses && Object.keys(progresses).length > 1 && }
-
-
+
{progressBars}
}
+
+/**
+ * The upload dialog. Pass a `destination` to upload into it, or leave it off to have the user
+ * search for one.
+ */
+export function UploadModal({open, onClose, destination, onComplete, disabled}) {
+ const mediaDirectory = useMediaDirectory();
+ const asksForDestination = destination === undefined;
+
+ const upload = useUploadFile();
+ const {setFiles, setDestination, doClear, inProgress} = upload;
+
+ const onDrop = React.useCallback(async (acceptedFiles) => {
+ if (acceptedFiles && acceptedFiles.length > 0) {
+ setFiles(acceptedFiles);
+ }
+ }, [setFiles]);
+
+ const {getRootProps, getInputProps} = useDropzone({onDrop, disabled: inProgress});
+
+ // A fixed destination is the caller's to declare; only the searching form sets its own.
+ React.useEffect(() => {
+ if (open && !asksForDestination) {
+ setDestination(destination);
+ }
+ }, [open, destination, asksForDestination, setDestination]);
+
+ /*
+ * The user cleared their search, so the queued files have nowhere to go.
+ *
+ * Only while asking. `''` is a fixed destination -- the media directory -- and clearing on
+ * it fired this effect on every render, because `doClear` is a new function each time and
+ * its own setState scheduled the next render. React caps that at "Maximum update depth
+ * exceeded"; the file browser reached it whenever Upload was pressed with nothing selected.
+ */
+ React.useEffect(() => {
+ if (asksForDestination && !upload.destination && open) {
+ doClear();
+ }
+ }, [asksForDestination, upload.destination, open, doClear]);
+
+ const handleClose = () => {
+ if (inProgress) return;
+ doClear();
+ onClose();
+ if (onComplete) onComplete();
+ };
+
+ // No trailing slash for the media directory itself.
+ const displayPath = asksForDestination ? null
+ : (destination ? `${mediaDirectory}/${destination}` : mediaDirectory);
+
+ // Fullscreen, so the drop target is something you can drag a file onto without aiming.
+ return
+ {displayPath ? `Upload to: ${displayPath}` : 'Upload from your device'}
+
+
+
+
+
+
+
+}
diff --git a/app/src/components/Upload.test.js b/app/src/components/Upload.test.js
new file mode 100644
index 00000000..153467f1
--- /dev/null
+++ b/app/src/components/Upload.test.js
@@ -0,0 +1,90 @@
+import React from 'react';
+import {screen} from '@testing-library/react';
+import {UploadModal} from './Upload';
+import {renderWithProviders} from '../test-utils';
+
+const mockUseUploadFile = {
+ setFiles: jest.fn(),
+ progresses: {},
+ destination: '',
+ setDestination: jest.fn(),
+ doClear: jest.fn(),
+ tagsSelector: null,
+ overwrite: false,
+ setOverwrite: jest.fn(),
+ overallProgress: 0,
+ inProgress: false,
+};
+
+jest.mock('../hooks/customHooks', () => require('../test-utils').mockModule(
+ jest.requireActual('../hooks/customHooks'),
+ {useUploadFile: () => mockUseUploadFile},
+));
+
+jest.mock('react-dropzone', () => ({
+ useDropzone: () => ({getRootProps: () => ({}), getInputProps: () => ({})}),
+}));
+
+// The real `useMediaDirectory`, reading the settings the providers supply.
+const render = ui => renderWithProviders(ui, {settings: {media_directory: '/media/wrolpi'}});
+
+/*
+ * The dashboard and the file browser open the same dialog, and differ only in whether it asks
+ * where the files go. That fork, and the `''` contract underneath it, are what these pin.
+ */
+describe('one upload dialog, two entry points', () => {
+ beforeEach(() => jest.clearAllMocks());
+
+ it('asks for a destination when the caller has none', () => {
+ render( {}}/>);
+
+ expect(screen.getByText('Destination')).toBeInTheDocument();
+ // Nowhere to put a file yet, so the target is replaced by the instruction.
+ expect(screen.getByText(/must search for a directory/)).toBeInTheDocument();
+ expect(screen.queryByText(/drop files here/)).not.toBeInTheDocument();
+ });
+
+ it('uploads into the destination it is given, and names it', () => {
+ render( {}} destination='videos'/>);
+
+ expect(screen.getByText('Upload to: /media/wrolpi/videos')).toBeInTheDocument();
+ expect(screen.queryByText('Destination')).not.toBeInTheDocument();
+ expect(screen.getByText(/drop files here/)).toBeInTheDocument();
+ expect(mockUseUploadFile.setDestination).toHaveBeenCalledWith('videos');
+ });
+
+ /*
+ * '' is the media directory -- where the file browser uploads when no folder is selected,
+ * which is the state its Upload button is enabled in. Every falsy check along this path
+ * reads it as "no destination", so each is asserted separately: the fork, the title, and
+ * what the hook is told.
+ */
+ it('treats the media directory as a destination, not a missing one', () => {
+ render( {}} destination=''/>);
+
+ expect(screen.queryByText('Destination')).not.toBeInTheDocument();
+ expect(screen.getByText(/drop files here/)).toBeInTheDocument();
+ expect(screen.getByText('Upload to: /media/wrolpi')).toBeInTheDocument();
+ expect(mockUseUploadFile.setDestination).toHaveBeenCalledWith('');
+ });
+
+ /*
+ * The clearing effect ran on every render for a `''` destination, and `doClear` sets state,
+ * so each run scheduled the render that ran it again -- "Maximum update depth exceeded" on
+ * the file browser's default Upload. A fixed destination is never cleared, so it is never
+ * the thing that empties the queue.
+ */
+ it('does not clear the queue it is about to upload', () => {
+ const {rerender} = render( {}} destination=''/>);
+ rerender( {}} destination=''/>);
+
+ expect(mockUseUploadFile.doClear).not.toHaveBeenCalled();
+ });
+
+ // Asking is the case that does clear: the user emptied their own search.
+ it('clears the queue when the user clears their search', () => {
+ render( {}}/>);
+
+ expect(mockUseUploadFile.doClear).toHaveBeenCalled();
+ });
+});
diff --git a/app/src/hooks/customHooks.js b/app/src/hooks/customHooks.js
index 0e4a605a..35a98e5d 100644
--- a/app/src/hooks/customHooks.js
+++ b/app/src/hooks/customHooks.js
@@ -1788,7 +1788,9 @@ export const useUploadFile = () => {
}
const doUpload = async () => {
- if (!files || files.length === 0 || !destination) {
+ // `''` is the media directory, which the file browser uploads to when no folder is
+ // selected; only a missing destination means there is nowhere to put the files.
+ if (!files || files.length === 0 || destination === null || destination === undefined) {
return;
}
@@ -1890,14 +1892,17 @@ export const useUploadFile = () => {
setInProgress(false);
}
- const doClear = () => {
+ // Stable across renders. Callers put it in effect dependencies, and every call sets state
+ // with fresh objects -- an unstable identity there re-runs the effect on the render its own
+ // setState caused, which React ends with "Maximum update depth exceeded".
+ const doClear = React.useCallback(() => {
setFiles([]);
setProgresses({});
setTotalSize(0);
setUploadedSize(0);
setOverallProgress(0);
setInProgress(false);
- }
+ }, []);
useEffect(() => {
doUpload()