Merge pull request #323 from bento-platform/fix/file-modal-style-and-…

…view-btn fix(explorer): experiment result view button conditions + bad file view style
bento-platform · Nov 1, 2023 · e2b45ad · e2b45ad
2 parents 9712b33 + 278f869
commit e2b45ad
Show file tree

Hide file tree

Showing 7 changed files with 123 additions and 75 deletions.
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "bento_web",
-  "version": "3.2.0",
+  "version": "3.2.1",
   "description": "Bento platform front-end",
   "main": "src/index.js",
   "dependencies": {

diff --git a/src/components/DownloadButton.js b/src/components/DownloadButton.js
@@ -1,15 +1,32 @@
-import { useSelector } from "react-redux";
 import React, { useCallback } from "react";
-import { Button } from "antd";
+import { useSelector } from "react-redux";
 import PropTypes from "prop-types";
 
-const DownloadButton = ({ disabled, uri, children, size, type }) => {
+import { Button } from "antd";
+
+import { AUDIO_FILE_EXTENSIONS, IMAGE_FILE_EXTENSIONS, VIDEO_FILE_EXTENSIONS } from "./display/FileDisplay";
+
+const BROWSER_RENDERED_EXTENSIONS = [
+    ".pdf",
+    ".txt",
+    ...AUDIO_FILE_EXTENSIONS,
+    ...IMAGE_FILE_EXTENSIONS,
+    ...VIDEO_FILE_EXTENSIONS,
+];
+
+const DownloadButton = ({ disabled, uri, fileName, children, size, type, onClick: propsOnClick, ...props }) => {
     const { accessToken } = useSelector((state) => state.auth);
 
-    const onClick = useCallback(() => {
+    const onClick = useCallback((e) => {
         if (!uri) return;
 
         const form = document.createElement("form");
+        if (fileName && BROWSER_RENDERED_EXTENSIONS.find((ext) => fileName.toLowerCase().endsWith(ext))) {
+            // In Firefox, if we open, e.g., a PDF; it'll open in the PDF viewer instead of downloading.
+            // Here, we force it to open in a new tab if it's render-able by the browser (although Chrome will actually
+            // download the PDF file, so it'll flash a new tab - this is a compromise solution for viewable file types.)
+            form.target = "_blank";
+        }
         form.method = "post";
         form.action = uri;
         form.innerHTML = `<input type="hidden" name="token" value="${accessToken}" />`;
@@ -19,11 +36,14 @@ const DownloadButton = ({ disabled, uri, children, size, type }) => {
         } finally {
             // Even if submit raises for some reason, we still need to clean this up; it has a token in it!
             document.body.removeChild(form);
+
+            // Call the props-passed onClick event handler after hijacking the event and doing our own thing
+            if (propsOnClick) propsOnClick(e);
         }
-    }, [uri, accessToken]);
+    }, [uri, accessToken, propsOnClick]);
 
     return (
-        <Button key="download" icon="download" size={size} type={type} disabled={disabled} onClick={onClick}>
+        <Button key="download" icon="download" size={size} type={type} disabled={disabled} onClick={onClick} {...props}>
             {children === undefined ? "Download" : children}
         </Button>
     );
@@ -38,9 +58,11 @@ DownloadButton.defaultProps = {
 DownloadButton.propTypes = {
     disabled: PropTypes.bool,
     uri: PropTypes.string,
+    fileName: PropTypes.string,
     children: PropTypes.oneOfType([PropTypes.object, PropTypes.string]),
     size: PropTypes.oneOf(["small", "default", "large"]),
     type: PropTypes.oneOf(["primary", "ghost", "dashed", "danger", "link", "default"]),
+    onClick: PropTypes.func,
 };
 
 export default DownloadButton;
diff --git a/src/components/display/FileDisplay.js b/src/components/display/FileDisplay.js
@@ -66,7 +66,7 @@ const LANGUAGE_HIGHLIGHTERS = {
     "CHANGELOG": "plaintext",
 };
 
-const AUDIO_FILE_EXTENSIONS = [
+export const AUDIO_FILE_EXTENSIONS = [
     "3gp",
     "aac",
     "flac",
@@ -78,7 +78,7 @@ const AUDIO_FILE_EXTENSIONS = [
 
 const CSV_LIKE_FILE_EXTENSIONS = ["csv", "tsv"];
 
-const IMAGE_FILE_EXTENSIONS = [
+export const IMAGE_FILE_EXTENSIONS = [
     "apng",
     "avif",
     "bmp",
@@ -90,7 +90,7 @@ const IMAGE_FILE_EXTENSIONS = [
     "webp",
 ];
 
-const VIDEO_FILE_EXTENSIONS = ["mp4", "webm"];
+export const VIDEO_FILE_EXTENSIONS = ["mp4", "webm"];
 
 // TODO: ".bed",
 //  .bed files are basically TSVs, but they can have instructions and can be whitespace-delimited instead

diff --git a/src/components/display/FileModal.js b/src/components/display/FileModal.js
@@ -0,0 +1,39 @@
+import React from "react";
+import PropTypes from "prop-types";
+import { Modal } from "antd";
+
+import FileDisplay from "./FileDisplay";
+
+const FileModal = ({ title, visible, onCancel, hasTriggered, url, fileName, loading }) => (
+    <Modal
+        title={title}
+        visible={visible}
+        onCancel={onCancel}
+        width={1080}
+        style={{
+            display: "flex",
+            justifyContent: "center",
+            alignItems: "center",
+            top: 50,  // down from default of 100; gives a bit more screen real estate
+        }}
+        bodyStyle={{ minWidth: "692px" }}
+        footer={null}
+        // destroyOnClose in order to stop audio/video from playing & avoid memory leaks at the cost of re-fetching:
+        destroyOnClose={true}
+    >
+        {(hasTriggered ?? true) && (
+            <FileDisplay uri={url} fileName={fileName} loading={loading ?? false} />
+        )}
+    </Modal>
+);
+FileModal.propTypes = {
+    title: PropTypes.oneOfType([PropTypes.string, PropTypes.node]).isRequired,
+    visible: PropTypes.bool.isRequired,
+    onCancel: PropTypes.func.isRequired,
+    hasTriggered: PropTypes.bool,
+    url: PropTypes.string,
+    fileName: PropTypes.string,
+    loading: PropTypes.bool,
+};
+
+export default FileModal;
diff --git a/src/components/explorer/IndividualExperiments.js b/src/components/explorer/IndividualExperiments.js
@@ -3,39 +3,28 @@ import { useDispatch, useSelector } from "react-redux";
 import { Route, Switch, useHistory, useParams, useRouteMatch } from "react-router-dom";
 import PropTypes from "prop-types";
 
-import { Button, Descriptions, Icon, Modal, Popover, Table, Typography } from "antd";
+import { Button, Descriptions, Icon, Popover, Table, Tooltip, Typography } from "antd";
 
-import { EM_DASH } from "../../constants";
 import { experimentPropTypesShape, experimentResultPropTypesShape, individualPropTypesShape } from "../../propTypes";
 import { getFileDownloadUrlsFromDrs } from "../../modules/drs/actions";
 import { guessFileType } from "../../utils/guessFileType";
 
 import { useDeduplicatedIndividualBiosamples, useIndividualResources } from "./utils";
+import { VIEWABLE_FILE_EXTENSIONS } from "../display/FileDisplay";
 
 import JsonView from "./JsonView";
 import OntologyTerm from "./OntologyTerm";
 import DownloadButton from "../DownloadButton";
-import FileDisplay, { VIEWABLE_FILE_EXTENSIONS } from "../display/FileDisplay";
+import FileModal from "../display/FileModal";
 
-const ExperimentResultDownloadButton = ({ result }) => {
-    const downloadUrls = useSelector((state) => state.drs.downloadUrlsByFilename);
-
-    const url = downloadUrls[result.filename]?.url;
-    return url ? (
-        <DownloadButton size="small" type="link" uri={url}>{""}</DownloadButton>
-    ) : (
-        <>{EM_DASH}</>
-    );
-};
-ExperimentResultDownloadButton.propTypes = {
-    result: experimentResultPropTypesShape,
-};
 
 const VIEWABLE_FILE_FORMATS = ["PDF", "CSV", "TSV"];
 
 const ExperimentResultActions = ({ result }) => {
+    const { filename } = result;
+
     const downloadUrls = useSelector((state) => state.drs.downloadUrlsByFilename);
-    const url = downloadUrls[result.filename]?.url;
+    const url = downloadUrls[filename]?.url;
 
     const [viewModalVisible, setViewModalVisible] = useState(false);
 
@@ -52,24 +41,29 @@ const ExperimentResultActions = ({ result }) => {
     }, []);
     const onViewCancel = useCallback(() => setViewModalVisible(false), []);
 
-    const resultViewable = VIEWABLE_FILE_FORMATS.includes(result.file_format)
-        || !!VIEWABLE_FILE_EXTENSIONS.find(ext => result.filename.endsWith(ext));
+    const resultViewable = url && (
+        VIEWABLE_FILE_FORMATS.includes(result.file_format) ||
+        !!VIEWABLE_FILE_EXTENSIONS.find(ext => filename.toLowerCase().endsWith(ext)));
 
-    return <>
+    return <div style={{ whiteSpace: "nowrap" }}>
+        {url ? <>
+            <Tooltip title="Download">
+                <DownloadButton size="small" uri={url} fileName={filename}>{""}</DownloadButton>
+            </Tooltip>
+            {" "}
+        </> : null}
         {resultViewable ? <>
-            <Modal
-                title={<span>View: {result.filename}</span>}
+            <FileModal
                 visible={viewModalVisible}
                 onCancel={onViewCancel}
-                width={1080}
-                style={{marginTop: "-50px"}}
-                footer={null}
-            >
-                {hasTriggeredViewModal && (
-                    <FileDisplay uri={url} fileName={result.filename} />
-                )}
-            </Modal>
-            <Button size="small" icon="eye" onClick={onViewClick}>View</Button>{" "}
+                title={<span>View: {result.filename}</span>}
+                url={url}
+                fileName={result.filename}
+                hasTriggered={hasTriggeredViewModal}
+            />
+            <Tooltip title="View">
+                <Button size="small" icon="eye" onClick={onViewClick} />
+            </Tooltip>{" "}
         </> : null}
         <Popover
             placement="leftTop"
@@ -106,9 +100,11 @@ const ExperimentResultActions = ({ result }) => {
             }
             trigger="click"
         >
-            <Button size="small" icon="bars">See details</Button>
+            <Tooltip title="Details">
+                <Button size="small" icon="bars" />
+            </Tooltip>
         </Popover>
-    </>;
+    </div>;
 };
 ExperimentResultActions.propTypes = {
     result: experimentResultPropTypesShape,
@@ -131,12 +127,6 @@ const EXPERIMENT_RESULTS_COLUMNS = [
         title: "Filename",
         dataIndex: "filename",
     },
-    {
-        title: "Download",
-        key: "download",
-        align: "center",
-        render: (_, result) => <ExperimentResultDownloadButton result={result} />,
-    },
     {
         key: "other_details",
         align: "center",

diff --git a/src/components/manager/ManagerDropBoxContent.js b/src/components/manager/ManagerDropBoxContent.js
@@ -30,10 +30,11 @@ import {LAYOUT_CONTENT_STYLE} from "../../styles/layoutContent";
 import DownloadButton from "../DownloadButton";
 import DropBoxTreeSelect from "./DropBoxTreeSelect";
 import DatasetSelectionModal from "./DatasetSelectionModal";
+import FileModal from "../display/FileModal";
 
 import {BENTO_DROP_BOX_FS_BASE_PATH} from "../../config";
 import {STEP_INPUT} from "./workflowCommon";
-import {dropBoxTreeStateToPropsMixinPropTypes, workflowsStateToPropsMixin} from "../../propTypes";
+import { workflowsStateToPropsMixin } from "../../propTypes";
 import {useResourcePermissions} from "../../lib/auth/utils";
 import {getFalse} from "../../utils/misc";
 import {
@@ -46,7 +47,7 @@ import {
 import {RESOURCE_EVERYTHING} from "../../lib/auth/resources";
 import {deleteDropBox, ingestDropBox} from "../../lib/auth/permissions";
 
-import FileDisplay, { VIEWABLE_FILE_EXTENSIONS } from "../display/FileDisplay";
+import { VIEWABLE_FILE_EXTENSIONS } from "../display/FileDisplay";
 
 const DROP_BOX_CONTENT_CONTAINER_STYLE = { display: "flex", flexDirection: "column", gap: 8 };
 const DROP_BOX_ACTION_CONTAINER_STYLE = {
@@ -123,17 +124,6 @@ const stopEvent = event => {
 };
 
 
-const DropBoxFileDisplay = ({file, tree, treeLoading}) => {
-    const urisByFilePath = useMemo(() => generateURIsByRelPath(tree, {}), [tree]);
-    const uri = useMemo(() => urisByFilePath[file], [urisByFilePath, file]);
-
-    return <FileDisplay uri={uri} fileName={file} loading={treeLoading} />;
-};
-DropBoxFileDisplay.propTypes = {
-    file: PropTypes.string,
-    ...dropBoxTreeStateToPropsMixinPropTypes,
-};
-
 const FileUploadForm = Form.create()(({initialUploadFolder, initialUploadFiles, form}) => {
     const getFileListFromEvent = useCallback(e => Array.isArray(e) ? e : e && e.fileList, []);
 
@@ -255,18 +245,20 @@ FileUploadModal.propTypes = {
 const FileContentsModal = ({selectedFilePath, visible, onCancel}) => {
     const {tree, isFetching: treeLoading} = useSelector(state => state.dropBox);
 
+    const urisByFilePath = useMemo(() => generateURIsByRelPath(tree, {}), [tree]);
+    const uri = useMemo(() => urisByFilePath[selectedFilePath], [urisByFilePath, selectedFilePath]);
+
     // destroyOnClose in order to stop audio/video from playing & avoid memory leaks at the cost of re-fetching
-    return <Modal
-        visible={visible}
-        title={selectedFilePath ? `${selectedFilePath.split("/").at(-1)} - contents` : ""}
-        width={1080}
-        style={{marginTop: "-50px"}}
-        footer={null}
-        destroyOnClose={true}
-        onCancel={onCancel}
-    >
-        <DropBoxFileDisplay file={selectedFilePath} tree={tree} treeLoading={treeLoading} />
-    </Modal>;
+    return (
+        <FileModal
+            visible={visible}
+            onCancel={onCancel}
+            title={selectedFilePath ? `${selectedFilePath.split("/").at(-1)} - contents` : ""}
+            url={uri}
+            fileName={selectedFilePath}
+            loading={treeLoading}
+        />
+    );
 };
 FileContentsModal.propTypes = {
     selectedFilePath: PropTypes.string,
@@ -571,7 +563,11 @@ const ManagerDropBoxContent = () => {
                         <Button icon="file-text" onClick={handleViewFile} disabled={!selectedFileViewable}>
                             View
                         </Button>
-                        <DownloadButton disabled={!selectedFileInfoAvailable} uri={filesByPath[fileForInfo]?.uri} />
+                        <DownloadButton
+                            disabled={!selectedFileInfoAvailable}
+                            uri={filesByPath[fileForInfo]?.uri}
+                            fileName={fileForInfo}
+                        />
                     </Button.Group>
 
                     <Button type="danger"

diff --git a/src/components/manager/drs/ManagerDRSContent.js b/src/components/manager/drs/ManagerDRSContent.js
@@ -43,7 +43,7 @@ const DRS_COLUMNS = [
         key: "actions",
         render: (record) => {
             const url = record.access_methods[0]?.access_url?.url;
-            return <DownloadButton disabled={!url} uri={url} />;
+            return <DownloadButton disabled={!url} uri={url} fileName={record.name} size="small" />;
         },
     },
 ];
@@ -122,6 +122,7 @@ const ManagerDRSContent = () => {
                     dataSource={searchResults}
                     loading={loading}
                     bordered={true}
+                    size="middle"
                     expandedRowRender={({ id, description, checksums, access_methods: accessMethods, size }) => (
                         <div style={TABLE_NESTED_DESCRIPTIONS_STYLE} className="table-nested-ant-descriptions">
                             <Descriptions bordered={true}>