-
Notifications
You must be signed in to change notification settings - Fork 201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(schema): File download COMPASS-8704 #6740
Changes from 13 commits
68d0c41
6acb8c3
044b1cf
8b0446f
3dfb68a
cffd3f2
a2b6e3f
ee35eb2
a21f4ab
e5e72cb
2a7010c
2ba7e14
a4344d6
aa24084
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
import path from 'path'; | ||
import fs from 'fs'; | ||
|
||
export const downloadPath = path.join(__dirname, 'downloads'); | ||
|
||
export const waitForFileDownload = async ( | ||
filename: string, | ||
browser: WebdriverIO.Browser | ||
): Promise<{ | ||
fileExists: boolean; | ||
filePath: string; | ||
}> => { | ||
const filePath = `${downloadPath}/${filename}`; | ||
await browser.waitUntil( | ||
function () { | ||
return fs.existsSync(filePath); | ||
}, | ||
{ timeout: 10000, timeoutMsg: 'file not downloaded yet.' } | ||
); | ||
|
||
return { fileExists: fs.existsSync(filePath), filePath }; | ||
}; | ||
|
||
export const cleanUpDownloadedFile = (filename: string) => { | ||
const filePath = `${downloadPath}/${filename}`; | ||
try { | ||
if (fs.existsSync(filePath)) { | ||
fs.unlinkSync(filePath); | ||
} | ||
} catch (err) { | ||
console.error(`Error deleting file: ${(err as Error).message}`); | ||
} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ import { | |
ErrorSummary, | ||
Label, | ||
CancelLoader, | ||
SpinLoader, | ||
} from '@mongodb-js/compass-components'; | ||
import { CodemirrorMultilineEditor } from '@mongodb-js/compass-editor'; | ||
|
||
|
@@ -25,6 +26,7 @@ import { | |
trackSchemaExported, | ||
type SchemaFormat, | ||
type ExportStatus, | ||
downloadSchema, | ||
} from '../stores/schema-export-reducer'; | ||
|
||
const loaderStyles = css({ | ||
|
@@ -80,10 +82,12 @@ const ExportSchemaModal: React.FunctionComponent<{ | |
resultId?: string; | ||
exportFormat: SchemaFormat; | ||
exportedSchema?: string; | ||
filename?: string; | ||
onCancelSchemaExport: () => void; | ||
onChangeSchemaExportFormat: (format: SchemaFormat) => Promise<void>; | ||
onClose: () => void; | ||
onExportedSchemaCopied: () => void; | ||
onSchemaDownload: () => void; | ||
}> = ({ | ||
errorMessage, | ||
exportStatus, | ||
|
@@ -94,6 +98,7 @@ const ExportSchemaModal: React.FunctionComponent<{ | |
onChangeSchemaExportFormat, | ||
onClose, | ||
onExportedSchemaCopied, | ||
onSchemaDownload, | ||
}) => { | ||
const onFormatOptionSelected = useCallback( | ||
(event: ChangeEvent<HTMLInputElement>) => { | ||
|
@@ -178,10 +183,12 @@ const ExportSchemaModal: React.FunctionComponent<{ | |
Cancel | ||
</Button> | ||
<Button | ||
onClick={() => { | ||
/* TODO(COMPASS-8704): download and track with trackSchemaExported */ | ||
}} | ||
variant="primary" | ||
isLoading={exportStatus === 'inprogress'} | ||
loadingIndicator={<SpinLoader />} | ||
disabled={!exportedSchema} | ||
onClick={onSchemaDownload} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we planning on adding a visual indicator that the file is saving & saved? Right now the save dialogue closes and they're back to the export page which they would then cancel out of. Looking at the designs there is an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah! No, thanks for pointing that out, I forgot we had that in the designs. We don't have that control with the link approach as we'd have with file writing. The file download is handled by the browser - luckily it should be pretty much instantaneous. The part that might take more time is actually the stringifying, which we do after the conversion - so we already have indicators for that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the File System API was more widely supported.. but for now it would be too much added complexity for something that should be quick. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Closing the modal makes sense to me. It'll be a nicer ux once we have toasts. |
||
data-testid="schema-export-download-button" | ||
> | ||
Export | ||
</Button> | ||
|
@@ -197,11 +204,14 @@ export default connect( | |
exportFormat: state.schemaExport.exportFormat, | ||
isOpen: state.schemaExport.isOpen, | ||
exportedSchema: state.schemaExport.exportedSchema, | ||
filename: state.schemaExport.filename, | ||
}), | ||
{ | ||
onExportedSchemaCopied: trackSchemaExported, | ||
onExportedSchema: trackSchemaExported, | ||
onCancelSchemaExport: cancelExportSchema, | ||
onChangeSchemaExportFormat: changeExportSchemaFormat, | ||
onClose: closeExportSchema, | ||
onSchemaDownload: downloadSchema, | ||
} | ||
)(ExportSchemaModal); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious why it's different from what we do for the browser below where it's configured through chromeOptions. Asking because we do pass
goog:chromeOptions
to electron already and they seem to be applied correctly 🤔There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm.. this is how I understood the best practices docs, but let me try it out with just the options
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not out of the box, perhaps if we found an option to disable the 'save as' dialog
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, interesting, thanks for checking!