From 727ac48123ffd606eb44239c537602bf3f640dda Mon Sep 17 00:00:00 2001 From: Csaky Date: Wed, 17 Jan 2024 15:23:28 -0800 Subject: [PATCH 1/2] Skip pop-up blocker Move window.open event to the vue component --- frontend/src/components/object/DownloadObjectButton.vue | 5 +++-- frontend/src/services/objectService.ts | 3 +-- frontend/src/store/objectStore.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/frontend/src/components/object/DownloadObjectButton.vue b/frontend/src/components/object/DownloadObjectButton.vue index 1c0d6e89..f6333121 100644 --- a/frontend/src/components/object/DownloadObjectButton.vue +++ b/frontend/src/components/object/DownloadObjectButton.vue @@ -28,8 +28,9 @@ const displayNoFileDlg = ref(false); const download = () => { if (props.ids.length) { // For now we are looping the supplied IDs (if multiple selected) until there is a batching feature - props.ids.forEach((i) => { - objectStore.downloadObject(i, props.versionId); + props.ids.forEach(async (i) => { + var url = await objectStore.downloadObject(i, props.versionId); + window.open(url, '_blank'); }); } else { displayNoFileDlg.value = true; diff --git a/frontend/src/services/objectService.ts b/frontend/src/services/objectService.ts index 472025ba..b6e25802 100644 --- a/frontend/src/services/objectService.ts +++ b/frontend/src/services/objectService.ts @@ -138,8 +138,7 @@ export default { } }) .then((response) => { - const url = response.data; - window.open(url, '_blank'); + return response.data; }); }, diff --git a/frontend/src/store/objectStore.ts b/frontend/src/store/objectStore.ts index 2664e74e..76919c02 100644 --- a/frontend/src/store/objectStore.ts +++ b/frontend/src/store/objectStore.ts @@ -94,7 +94,7 @@ export const useObjectStore = defineStore('object', () => { async function downloadObject(objectId: string, versionId?: string) { try { appStore.beginIndeterminateLoading(); - await objectService.getObject(objectId, versionId); + return await objectService.getObject(objectId, versionId); } catch (error: any) { toast.error('Downloading object', error); } finally { From 8d6a35bd29781f186dd58428278f6c66a5558f94 Mon Sep 17 00:00:00 2001 From: Csaky Date: Wed, 17 Jan 2024 16:46:43 -0800 Subject: [PATCH 2/2] Add toast warning if new tabs blocked moved window.open() to component for more control --- .../object/DownloadObjectButton.vue | 21 ++++++++++++++----- frontend/src/services/objectService.ts | 18 +++++++--------- frontend/src/store/objectStore.ts | 6 +++--- frontend/tests/unit/store/objectStore.spec.ts | 8 +++---- 4 files changed, 30 insertions(+), 23 deletions(-) diff --git a/frontend/src/components/object/DownloadObjectButton.vue b/frontend/src/components/object/DownloadObjectButton.vue index f6333121..d21d12f2 100644 --- a/frontend/src/components/object/DownloadObjectButton.vue +++ b/frontend/src/components/object/DownloadObjectButton.vue @@ -2,10 +2,12 @@ import { ref } from 'vue'; import { FontAwesomeIcon } from '@fortawesome/vue-fontawesome'; -import { Button, Dialog } from '@/lib/primevue'; +import { Button, Dialog, useToast } from '@/lib/primevue'; import { useObjectStore } from '@/store'; import { ButtonMode } from '@/utils/enums'; +const toast = useToast(); + // Props type Props = { disabled?: boolean; @@ -28,10 +30,19 @@ const displayNoFileDlg = ref(false); const download = () => { if (props.ids.length) { // For now we are looping the supplied IDs (if multiple selected) until there is a batching feature - props.ids.forEach(async (i) => { - var url = await objectStore.downloadObject(i, props.versionId); - window.open(url, '_blank'); - }); + let alerted = false; + for (let i of props.ids) { + // get S3 url + objectStore.getObjectUrl(i, props.versionId).then((url) => { + // open new tab, window or save dialog + const newTab = window.open(url, '_blank'); + // if browser blocked new tab + if (newTab === null && !alerted) { + toast.warn('Downloading Objects', 'Your browser blocked multiple new tabs from opening.', { life: 0 }); + alerted = true; + } + }); + } } else { displayNoFileDlg.value = true; } diff --git a/frontend/src/services/objectService.ts b/frontend/src/services/objectService.ts index b6e25802..daf7ab4b 100644 --- a/frontend/src/services/objectService.ts +++ b/frontend/src/services/objectService.ts @@ -127,19 +127,15 @@ export default { * Get an object * @param {string} objectId The id for the object to get * @param {string} versionId An optional versionId + * @returns {Promise} An axios response */ getObject(objectId: string, versionId?: string) { - // Running in 'url' download mode only, could add options for other modes if needed - return comsAxios() - .get(`${PATH}/${objectId}`, { - params: { - versionId: versionId, - download: 'url' - } - }) - .then((response) => { - return response.data; - }); + return comsAxios().get(`${PATH}/${objectId}`, { + params: { + versionId: versionId, + download: 'url' + } + }); }, /** diff --git a/frontend/src/store/objectStore.ts b/frontend/src/store/objectStore.ts index 76919c02..f2e9dafc 100644 --- a/frontend/src/store/objectStore.ts +++ b/frontend/src/store/objectStore.ts @@ -91,10 +91,10 @@ export const useObjectStore = defineStore('object', () => { } } - async function downloadObject(objectId: string, versionId?: string) { + async function getObjectUrl(objectId: string, versionId?: string) { try { appStore.beginIndeterminateLoading(); - return await objectService.getObject(objectId, versionId); + return objectService.getObject(objectId, versionId).then((response) => response.data); } catch (error: any) { toast.error('Downloading object', error); } finally { @@ -270,7 +270,7 @@ export const useObjectStore = defineStore('object', () => { // Actions createObject, deleteObject, - downloadObject, + getObjectUrl, fetchObjects, findObjectById, headObject, diff --git a/frontend/tests/unit/store/objectStore.spec.ts b/frontend/tests/unit/store/objectStore.spec.ts index 6bc2d20d..4cbdb096 100644 --- a/frontend/tests/unit/store/objectStore.spec.ts +++ b/frontend/tests/unit/store/objectStore.spec.ts @@ -133,11 +133,11 @@ describe('Object Store', () => { }); }); - describe('downloadObject', () => { + describe('getObjectUrl', () => { it('gets the most recent object', async () => { getObjectSpy.mockReturnValue({} as any); - await objectStore.downloadObject(obj.id); + await objectStore.getObjectUrl(obj.id); expect(beginIndeterminateLoadingSpy).toHaveBeenCalledTimes(1); expect(getObjectSpy).toHaveBeenCalledTimes(1); @@ -148,7 +148,7 @@ describe('Object Store', () => { it('gets the object by version', async () => { getObjectSpy.mockReturnValue({} as any); - await objectStore.downloadObject(obj.id, '1'); + await objectStore.getObjectUrl(obj.id, '1'); expect(beginIndeterminateLoadingSpy).toHaveBeenCalledTimes(1); expect(getObjectSpy).toHaveBeenCalledTimes(1); @@ -161,7 +161,7 @@ describe('Object Store', () => { throw new Error(); }); - await objectStore.downloadObject(obj.id); + await objectStore.getObjectUrl(obj.id); expect(beginIndeterminateLoadingSpy).toHaveBeenCalledTimes(1); expect(getObjectSpy).toHaveBeenCalledTimes(1);