From abcf2b7dad71d98b8662d24a8d51e3e19c7fdbc4 Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Wed, 15 Jul 2015 13:15:05 -0700 Subject: [PATCH] feat(attachments): Tons of tiny fixes to attachments, drag-and-drop attachments to other apps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: consolidate all the "untitled" stuff into a convenience method on the File itself. Previously it'd show "Unnamed Attachment", download as "Untitled" and open as "". Now it's consistent everywhere and chooses names based on the contenttype (Event.ics). Rewrite CSS rules for uploads and attachments to be simpler - remove container divs and classnames from things that have no CSS - switch to using Flexbox so it's not necesary to have so many containers - remove zIndex hacks, apply overflow rules to name div only, so long filenames don't make action button unclickable - consolidate CSS classnames for uploads/attachments - Other style fixes - cursor "default" instead of text insertion on image attachments - cursor "default" on action buttons - image uplaods / attachments with long filenames truncate with ellpsis - attachments are not indented by an extra 15px in message bodies Prevent progress bar overflow (was ending above 100%, 100.12315%...) Update FileDownloadStore so it never creates Download objects when file is downloaded already - Previously, the download itself decided if it would be a no-op, but this meant the download was around for a split second and you'd see progress indicators flash for a moment when opening/saving an attachment. Upgrade FileDownloadStore use of promises Restore Image attachment drag and drop - was broken because the name gradient thing was covering the entire drag region. Allow file attachments to be drag and dropped to the finder and other applications 😍😍😍 Test Plan: Tests still pass Reviewers: evan Reviewed By: evan Differential Revision: https://phab.nylas.com/D1745 --- .../attachments/lib/attachment-component.cjsx | 109 ++++++---- .../lib/image-attachment-component.cjsx | 23 +- .../attachments/stylesheets/attachments.less | 205 ++++++++---------- .../composer/lib/composer-view.cjsx | 23 +- .../composer/lib/file-upload.cjsx | 32 +-- .../composer/lib/image-file-upload.cjsx | 20 +- .../composer/spec/composer-view-spec.cjsx | 24 +- .../composer/stylesheets/composer.less | 1 + .../file-list/lib/file-list.cjsx | 2 +- .../message-list/lib/message-item.cjsx | 4 +- .../stylesheets/message-list.less | 7 + src/components/draggable-img.cjsx | 2 +- src/flux/models/file.coffee | 18 ++ src/flux/stores/file-download-store.coffee | 176 +++++++-------- 14 files changed, 337 insertions(+), 309 deletions(-) diff --git a/internal_packages/attachments/lib/attachment-component.cjsx b/internal_packages/attachments/lib/attachment-component.cjsx index f35f199a6..a7ef006c7 100644 --- a/internal_packages/attachments/lib/attachment-component.cjsx +++ b/internal_packages/attachments/lib/attachment-component.cjsx @@ -1,19 +1,45 @@ _ = require 'underscore' path = require 'path' React = require 'react' -{RetinaImg} = require 'nylas-component-kit' -{Actions, Utils} = require 'nylas-exports' +{RetinaImg, Flexbox} = require 'nylas-component-kit' +{Actions, Utils, FileDownloadStore} = require 'nylas-exports' # Passed in as props from MessageItem and FileDownloadStore # This is empty if the attachment isn't downloading. # @props.download is a FileDownloadStore.Download object # @props.file is a File object +{DragDropMixin} = require 'react-dnd' + +AttachmentDragContainer = React.createClass + displayName: "AttachmentDragContainer" + mixins: [DragDropMixin] + statics: + configureDragDrop: (registerType) => + registerType('attachment', { + dragSource: + beginDrag: (component) => + # Why is event defined in this scope? Magic. We need to use react-dnd + # because otherwise it's global onDragStart listener will cancel the + # drag. We don't actually intend to do a react-dnd drag/drop, but we + # can use this hook to populate the event.dataTransfer + DownloadURL = component.props.downloadUrl + event.dataTransfer.setData("DownloadURL", DownloadURL) + event.dataTransfer.setData("text/nylas-file-url", DownloadURL) + + # This is bogus we don't care about the rest of the react-dnd lifecycle. + return {item: {DownloadURL}} + }) + + render: -> +
+ {@props.children} +
class AttachmentComponent extends React.Component @displayName: 'AttachmentComponent' @propTypes: - file: React.PropTypes.object.isRequired, + file: React.PropTypes.object.isRequired download: React.PropTypes.object removable: React.PropTypes.bool targetPath: React.PropTypes.string @@ -23,67 +49,72 @@ class AttachmentComponent extends React.Component @state = progressPercent: 0 render: => -
- - - - + +
+ + + + - - {@_renderFileActions()} - - - - + - - {@props.file.filename ? "Unnamed Attachment"} - - -
+ {@props.file.displayName()} + {@_renderFileActions()} + +
+ _renderFileActions: => if @props.removable -
+
{@_renderRemoveIcon()}
else if @_isDownloading() and @_canAbortDownload() -
+
{@_renderRemoveIcon()}
else -
+
{@_renderDownloadButton()}
_downloadProgressStyle: => width: "#{@props.download?.percent ? 0}%" - _onClickRemove: => - Actions.removeFile - file: @props.file - messageLocalId: @props.messageLocalId - _canAbortDownload: -> true - _renderRemoveIcon: -> - - - _renderDownloadButton: -> - - - _onClickView: => Actions.fetchAndOpenFile(@props.file) if @_canClickToView() - - _onClickDownload: => Actions.fetchAndSaveFile(@props.file) - - _onClickAbort: => Actions.abortDownload(@props.file, @props.download) - _canClickToView: => not @props.removable and not @_isDownloading() _isDownloading: => @props.download?.state is "downloading" + _renderRemoveIcon: -> + + + _renderDownloadButton: -> + + + _getDragDownloadURL: (event) => + path = FileDownloadStore.pathForFile(@props.file) + return "#{@props.file.contentType}:#{@props.file.displayName()}:file://#{path}" + + _onClickView: => Actions.fetchAndOpenFile(@props.file) if @_canClickToView() + + _onClickRemove: (event) => + Actions.removeFile + file: @props.file + messageLocalId: @props.messageLocalId + event.stopPropagation() # Prevent 'onClickView' + + _onClickDownload: (event) => + Actions.fetchAndSaveFile(@props.file) + event.stopPropagation() # Prevent 'onClickView' + + _onClickAbort: (event) => + Actions.abortDownload(@props.file, @props.download) + event.stopPropagation() # Prevent 'onClickView' + _extension: -> @props.file.filename.split('.').pop() diff --git a/internal_packages/attachments/lib/image-attachment-component.cjsx b/internal_packages/attachments/lib/image-attachment-component.cjsx index a6ec1d330..562ae39a6 100644 --- a/internal_packages/attachments/lib/image-attachment-component.cjsx +++ b/internal_packages/attachments/lib/image-attachment-component.cjsx @@ -7,32 +7,29 @@ class ImageAttachmentComponent extends AttachmentComponent @displayName: 'ImageAttachmentComponent' render: => -
- - - +
+ + + -
- {@_renderFileActions()} -
+ {@_renderFileActions()} -
-
-
{@props.file.filename}
+
+
+
{@props.file.displayName()}
{@_imgOrLoader()}
-
_canAbortDownload: -> false _renderRemoveIcon: -> - + _renderDownloadButton: -> - + _imgOrLoader: -> if @props.download diff --git a/internal_packages/attachments/stylesheets/attachments.less b/internal_packages/attachments/stylesheets/attachments.less index 7fa53b8d1..07c677bc0 100644 --- a/internal_packages/attachments/stylesheets/attachments.less +++ b/internal_packages/attachments/stylesheets/attachments.less @@ -1,33 +1,32 @@ @import "ui-variables"; @import "ui-mixins"; -.attachment-file-wrap { +.file-wrap { cursor: default; display: inline-block; position: relative; font-size: @font-size-small; - margin: 0 @spacing-standard @spacing-standard @spacing-standard; - background: @background-off-primary; - box-shadow: inset 0 0 1px 1px rgba(0,0,0,0.09); - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; + margin: 0 0 @spacing-standard @spacing-standard; width: calc(~"50% - 23px"); - border-radius: 4px; + -webkit-user-drag: element; - &.non-image-attachment { - width: calc(~"50% - 23px"); - margin-left: @spacing-standard; + .inner { + border-radius: 4px; + background: @background-off-primary; + box-shadow: inset 0 0 0 1px rgba(0, 0, 0, 0.09); + padding: 13px @spacing-standard 13px @spacing-standard; + height:54px; } - &:nth-child(even) { - margin-left: 0; + &:hover { + cursor: default; + .inner { + box-shadow: inset 0 0 0 1px rgba(0, 0, 0, 0.18); + } } &.file-upload { - border-radius: 4px; - padding: 13px @spacing-standard 13px @spacing-standard; - .attachment-file-name { + .file-name { color: @text-color-very-subtle; .uploading { color: @text-color; @@ -35,101 +34,66 @@ } } - .attachment-inner-wrap { - border-radius: 4px; - padding: 13px @spacing-standard 13px @spacing-standard; - position: relative; + .progress-bar-wrap { + display: none; + + &.state-downloading, &.state-started, &.state-progress { + display: block; + } + + &.state-completed, &.state-success { + display: block; + .progress-foreground { background: @background-color-success; } + } + + &.state-aborted, &.state-failed { + display: block; + .progress-foreground { background: @background-color-error; } + } + + .progress-foreground { + position: absolute; + left: 0; + bottom: 0; + height: 2px; + width: 0; // Changed by React + z-index: 3; + display: block; + background: @progress-bar-fill; + border-bottom-left-radius:4px; + } + .progress-background { + position: absolute; + left: 0; + bottom: 0; + height: 2px; + width: 100%; + z-index: 2; + display: block; + background: @progress-bar-background; + border-bottom-left-radius:4px; + border-bottom-right-radius:4px; + } + } + + .file-icon { + margin-right: 10px; + flex-shrink:0; + } + .file-name { + font-weight: @font-weight-medium; + flex: 1; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } - - &:hover { - cursor: default; - } - - &.pending, &.started, &.progress, &.downloading { - } - - .attachment-file-icon { - margin-right: 7px; - } - - .attachment-file-name { - font-weight: @font-weight-medium; - vertical-align: middle; - } - .attachment-file-and-name { - position: relative; - z-index: 2; - vertical-align: middle; - } - - .attachment-file-actions { - position: relative; - z-index: 3; - .attachment-icon { - float: right; - margin-left: 10px; - } + .file-action-icon { + margin-left: 10px; + flex-shrink:0; } } -.image-file-upload, -.image-attachment-file-wrap, -.attachment-file-wrap, -.attachment-inner-wrap { - - .attachment-download-bar-wrap { - display: none; - } - - &.downloading, &.started, &.progress { - .attachment-download-bar-wrap { - display: block; - } - } - - &.completed, &.success { - .attachment-download-bar-wrap { - display: block; - } - .attachment-upload-progress { background: @background-color-success; } - } - - &.aborted, &.failed { - .attachment-download-bar-wrap { - display: block; - } - .attachment-upload-progress { background: @background-color-error; } - } - - .attachment-download-progress, - .attachment-upload-progress { - position: absolute; - left: 0; - bottom: 0; - height: 2px; - width: 0; // Changed by React - z-index: 3; - display: block; - background: @progress-bar-fill; - } - .attachment-bar-bg { - position: absolute; - left: 0; - bottom: 0; - height: 2px; - width: 100%; - z-index: 2; - display: block; - background: @progress-bar-background; - } -} - -.image-attachment-file-wrap, -.image-file-upload { - +.file-wrap.file-image-wrap { position: relative; text-align: center; display:inline-block; @@ -137,30 +101,27 @@ margin-bottom: @spacing-standard; margin-right: @spacing-standard; margin-left: @spacing-standard; + width: initial; + max-width: calc(~"100% - 30px"); - .attachment-download-progress, - .attachment-upload-progress { + .progress-foreground, + .progress-foreground { bottom: -2px; } - .attachment-bar-bg { + .progress-background { bottom: -2px; } - .attachment-file-actions { - position: relative; - z-index: 2; + .file-action-icon, .file-name-container, .file-name { + display: none; } - &:hover { - .attachment-file-actions, .attachment-name-container, .attachment-name { + .file-action-icon, .file-name-container, .file-name { display: block; } } - .attachment-file-actions, .attachment-name-container, .attachment-name { - display: none; - } - .attachment-icon { + .file-action-icon { position: absolute; z-index: 2; right: -8px; @@ -169,12 +130,13 @@ border-radius: 0 0 0 3px; } - .attachment-preview { + .file-preview { position: relative; z-index: 1; overflow: hidden; - .attachment-name-container { + .file-name-container { + cursor: default; position: absolute; bottom: 0; top: 0; @@ -185,10 +147,15 @@ background: linear-gradient(to top, rgba(0,0,0,0.75) 0%,rgba(0,0,0,0) 23%); vertical-align:bottom; - .attachment-name { + // Important! file-name-container is on top of the image and prevents you from dragging it. + pointer-events: none; + + .file-name { color: @white; left: @spacing-standard; + right: @spacing-standard; bottom: @spacing-standard; + text-align:left; position: absolute; z-index: 3; } diff --git a/internal_packages/composer/lib/composer-view.cjsx b/internal_packages/composer/lib/composer-view.cjsx index b9f99266f..5edd61c31 100644 --- a/internal_packages/composer/lib/composer-view.cjsx +++ b/internal_packages/composer/lib/composer-view.cjsx @@ -2,6 +2,7 @@ React = require 'react' _ = require 'underscore' {Utils, + File, Actions, DraftStore, UndoManager, @@ -295,7 +296,7 @@ class ComposerView extends React.Component _renderAttachments: -> renderSubset = (arr, attachmentRole, UploadComponent) => arr.map (fileOrUpload) => - if fileOrUpload.object is "file" + if fileOrUpload instanceof File @_attachmentComponent(fileOrUpload, attachmentRole) else @@ -317,9 +318,9 @@ class ComposerView extends React.Component messageLocalId: @props.localId if role is "Attachment" - className = "non-image-attachment attachment-file-wrap" + className = "file-wrap" else - className = "image-attachment-file-wrap" + className = "file-wrap file-image-wrap" e.preventDefault() + + # Accept drops of real files from other applications for file in e.dataTransfer.files Actions.attachFilePath({path: file.path, messageLocalId: @props.localId}) + + # Accept drops from attachment components within the app + if "text/nylas-file-url" in event.dataTransfer.types + downloadURL = event.dataTransfer.getData("text/nylas-file-url") + downloadFilePath = downloadURL.split('file://')[1] + Actions.attachFilePath({path: downloadFilePath, messageLocalId: @props.localId}) + + # Accept drops of images from within the app + if "text/uri-list" in event.dataTransfer.types + uri = event.dataTransfer.getData('text/uri-list') + if uri.indexOf('file://') is 0 + uri = uri.split('file://')[1] + Actions.attachFilePath({path: uri, messageLocalId: @props.localId}) + true _onFilePaste: (path) => diff --git a/internal_packages/composer/lib/file-upload.cjsx b/internal_packages/composer/lib/file-upload.cjsx index 0e2121c19..5f572f527 100644 --- a/internal_packages/composer/lib/file-upload.cjsx +++ b/internal_packages/composer/lib/file-upload.cjsx @@ -1,6 +1,6 @@ path = require 'path' React = require 'react' -{RetinaImg} = require 'nylas-component-kit' +{RetinaImg, Flexbox} = require 'nylas-component-kit' {Utils, Actions, FileUploadStore} = require 'nylas-exports' @@ -9,32 +9,32 @@ class FileUpload extends React.Component @displayName: 'FileUpload' render: => -
- - - - -
- +
+
+
+ +
- - - + - - Uploading: {@_basename()} - - + + Uploading: {@_basename()} + +
+ +
+ +
_uploadProgressStyle: => if @props.uploadData.fileSize <= 0 percent = 0 else - percent = (@props.uploadData.bytesUploaded / @props.uploadData.fileSize) * 100 + percent = Math.min(1, (@props.uploadData.bytesUploaded / @props.uploadData.fileSize)) * 100 width: "#{percent}%" _onClickRemove: => diff --git a/internal_packages/composer/lib/image-file-upload.cjsx b/internal_packages/composer/lib/image-file-upload.cjsx index 1cbaf76a4..cb25dee5d 100644 --- a/internal_packages/composer/lib/image-file-upload.cjsx +++ b/internal_packages/composer/lib/image-file-upload.cjsx @@ -10,22 +10,22 @@ class ImageFileUpload extends FileUpload uploadData: React.PropTypes.object render: => -
-
-
- -
+
+
+
-
-
-
{@props.uploadData.fileName}
+
+
+
{@props.uploadData.fileName}
+
- - +
+ +
module.exports = ImageFileUpload diff --git a/internal_packages/composer/spec/composer-view-spec.cjsx b/internal_packages/composer/spec/composer-view-spec.cjsx index f20b3e9dd..d45195c93 100644 --- a/internal_packages/composer/spec/composer-view-spec.cjsx +++ b/internal_packages/composer/spec/composer-view-spec.cjsx @@ -522,21 +522,18 @@ describe "populated composer", -> describe "A draft with files (attachments) and uploads", -> beforeEach -> - @file1 = + @file1 = new File id: "f_1" - object: "file" filename: "f1.pdf" size: 1230 - @file2 = + @file2 = new File id: "f_2" - object: "file" filename: "f2.jpg" size: 4560 - @file3 = + @file3 = new File id: "f_3" - object: "file" filename: "f3.png" size: 7890 @@ -566,21 +563,14 @@ describe "populated composer", -> expect(Actions.fetchFile.calls.length).toBe 1 expect(Actions.fetchFile.calls[0].args[0]).toBe @file2 - it 'renders the non image file as an attachment', -> - els = ReactTestUtils.scryRenderedComponentsWithTypeAndProps(@composer, InjectedComponent, matching: role: "Attachment") + it 'injects an Attachment component for non image files', -> + els = ReactTestUtils.scryRenderedComponentsWithTypeAndProps(@composer, InjectedComponent, matching: {role: "Attachment"}) expect(els.length).toBe 1 - it 'renders the image file as an attachment', -> - els = ReactTestUtils.scryRenderedComponentsWithTypeAndProps(@composer, InjectedComponent, matching: role: "Attachment:Image") + it 'injects an Attachment:Image component for image files', -> + els = ReactTestUtils.scryRenderedComponentsWithTypeAndProps(@composer, InjectedComponent, matching: {role: "Attachment:Image"}) expect(els.length).toBe 1 - it 'renders the uploads with the correct components', -> - el = ReactTestUtils.findRenderedDOMComponentWithClass(@composer, 'file-upload') - expect(el).toBeDefined() - - el = ReactTestUtils.findRenderedDOMComponentWithClass(@composer, 'image-file-upload') - expect(el).toBeDefined() - describe "when the DraftStore `isSending` isn't stubbed out", -> beforeEach -> DraftStore._pendingEnqueue = {} diff --git a/internal_packages/composer/stylesheets/composer.less b/internal_packages/composer/stylesheets/composer.less index 0f3344016..0c0987c98 100644 --- a/internal_packages/composer/stylesheets/composer.less +++ b/internal_packages/composer/stylesheets/composer.less @@ -182,6 +182,7 @@ // TODO FIXME DRY From stylesheets/message-list.less .attachments-area { padding: 0; + margin: 0; } .token { diff --git a/internal_packages/file-list/lib/file-list.cjsx b/internal_packages/file-list/lib/file-list.cjsx index 0c32040cb..21d8261bb 100644 --- a/internal_packages/file-list/lib/file-list.cjsx +++ b/internal_packages/file-list/lib/file-list.cjsx @@ -25,7 +25,7 @@ class FileList extends React.Component name: "Name" flex: 1 resolver: (file) => -
{file.filename}
+
{file.displayName()}
c2 = new ListTabular.Column name: "Size" diff --git a/internal_packages/message-list/lib/message-item.cjsx b/internal_packages/message-list/lib/message-item.cjsx index f728a1069..df146349e 100644 --- a/internal_packages/message-list/lib/message-item.cjsx +++ b/internal_packages/message-list/lib/message-item.cjsx @@ -226,7 +226,7 @@ class MessageItem extends React.Component otherAttachments = otherAttachments.map (file) => @@ -238,7 +238,7 @@ class MessageItem extends React.Component targetPath: FileDownloadStore.pathForFile(file) diff --git a/internal_packages/message-list/stylesheets/message-list.less b/internal_packages/message-list/stylesheets/message-list.less index 2a8c0e1a6..a22c340cc 100644 --- a/internal_packages/message-list/stylesheets/message-list.less +++ b/internal_packages/message-list/stylesheets/message-list.less @@ -375,6 +375,13 @@ } .attachments-area { padding-top: @spacing-standard; + + // attachments are padded on both sides so that things like the remove "X" can + // overhang them. To make the attachments line up with the body, we need to outdent + margin-left: -@spacing-standard; + margin-right: -@spacing-standard; + + cursor:default; } diff --git a/src/components/draggable-img.cjsx b/src/components/draggable-img.cjsx index f05dc5710..ae21d87a7 100644 --- a/src/components/draggable-img.cjsx +++ b/src/components/draggable-img.cjsx @@ -11,7 +11,7 @@ class DraggableImg extends React.Component constructor: (@props) -> render: => - + _onDragStart: (event) => img = React.findDOMNode(@refs.img) diff --git a/src/flux/models/file.coffee b/src/flux/models/file.coffee index 4029c6646..c645a7e69 100644 --- a/src/flux/models/file.coffee +++ b/src/flux/models/file.coffee @@ -48,4 +48,22 @@ class File extends Model modelKey: 'contentId' jsonKey: 'content_id' + # Public: Files can have empty names, or no name. `displayName` returns the file's + # name if one is present, and falls back to appropriate default name based on + # the contentType. It will always return a non-empty string. + # + displayName: -> + defaultNames = { + 'text/calendar': "Event.ics", + 'image/png': 'Unnamed Image.png' + 'image/jpg': 'Unnamed Image.jpg' + 'image/jpeg': 'Unnamed Image.jpg' + } + if @filename and @filename.length + return @filename + else if defaultNames[@contentType] + return defaultNames[@contentType] + else + return "Unnamed Attachment" + module.exports = File diff --git a/src/flux/stores/file-download-store.coffee b/src/flux/stores/file-download-store.coffee index 92972b39a..456e70d69 100644 --- a/src/flux/stores/file-download-store.coffee +++ b/src/flux/stores/file-download-store.coffee @@ -12,22 +12,32 @@ progress = require 'request-progress' NamespaceStore = require '../stores/namespace-store' NylasAPI = require '../nylas-api' -UNTITLED = "Untitled" +Promise.promisifyAll(fs) + +mkdirpAsync = (folder) -> + new Promise (resolve, reject) -> + mkdirp folder, (err) -> + if err then reject(err) else resolve(folder) class Download constructor: ({@fileId, @targetPath, @filename, @filesize, @progressCallback}) -> + if not @filename or @filename.length is 0 + throw new Error("Download.constructor: You must provide a non-empty filename.") + if not @fileId + throw new Error("Download.constructor: You must provide a fileID to download.") + if not @targetPath + throw new Error("Download.constructor: You must provide a target path to download.") + @percent = 0 @promise = null - if (@filename ? "").trim().length is 0 - @filename = UNTITLED @ state: -> if not @promise 'unstarted' - if @promise.isFulfilled() + else if @promise.isFulfilled() 'finished' - if @promise.isRejected() + else if @promise.isRejected() 'failed' else 'downloading' @@ -49,62 +59,47 @@ class Download return @promise if @promise @promise = new Promise (resolve, reject) => - return reject(new Error("Must pass a fileID to download")) unless @fileId? - return reject(new Error("Must have a target path to download")) unless @targetPath? + namespace = NamespaceStore.current()?.id + stream = fs.createWriteStream(@targetPath) + finished = false + finishedAction = null - fs.exists @targetPath, (exists) => - # Does the file already exist on disk? If so, just resolve immediately. - if exists - fs.stat @targetPath, (err, stats) => - if not err and stats.size >= @filesize - return resolve(@) - else - @_doDownload(resolve, reject) + # We need to watch the request for `success` or `error`, but not fire + # a callback until the stream has ended. These helper functions ensure + # that resolve or reject is only fired once regardless of the order + # these two events (stream end and `success`) happen in. + streamEnded = -> + finished = true + if finishedAction + finishedAction(@) + + onStreamEnded = (action) -> + if finished + action(@) else - @_doDownload(resolve, reject) + finishedAction = action - _doDownload: (resolve, reject) => - namespace = NamespaceStore.current()?.id - stream = fs.createWriteStream(@targetPath) - finished = false - finishedAction = null + NylasAPI.makeRequest + json: false + path: "/n/#{namespace}/files/#{@fileId}/download" + started: (req) => + @request = req + progress(@request, {throtte: 250}) + .on "progress", (progress) => + @percent = progress.percent + @progressCallback() + .on "end", => + # Wait for the file stream to finish writing before we resolve or reject + stream.end(streamEnded) + .pipe(stream) - # We need to watch the request for `success` or `error`, but not fire - # a callback until the stream has ended. These helper functions ensure - # that resolve or reject is only fired once regardless of the order - # these two events (stream end and `success`) happen in. - streamEnded = -> - finished = true - if finishedAction - finishedAction(@) + success: => + # At this point, the file stream has not finished writing to disk. + # Don't resolve yet, or the browser will load only part of the image. + onStreamEnded(resolve) - onStreamEnded = (action) -> - if finished - action(@) - else - finishedAction = action - - NylasAPI.makeRequest - json: false - path: "/n/#{namespace}/files/#{@fileId}/download" - started: (req) => - @request = req - progress(@request, {throtte: 250}) - .on "progress", (progress) => - @percent = progress.percent - @progressCallback() - .on "end", => - # Wait for the file stream to finish writing before we resolve or reject - stream.end(streamEnded) - .pipe(stream) - - success: => - # At this point, the file stream has not finished writing to disk. - # Don't resolve yet, or the browser will load only part of the image. - onStreamEnded(resolve) - - error: => - onStreamEnded(reject) + error: => + onStreamEnded(reject) abort: -> @request?.abort() @@ -130,11 +125,7 @@ FileDownloadStore = Reflux.createStore # pathForFile: (file) -> return undefined unless file - if file.filename and file.filename.length > 0 - downloadFilename = file.filename - else - downloadFilename = file.id - path.join(@_downloadDirectory, file.id, downloadFilename) + path.join(@_downloadDirectory, file.id, file.displayName()) downloadDataForFile: (fileId) -> @_downloads[fileId]?.data() @@ -150,8 +141,8 @@ FileDownloadStore = Reflux.createStore ########### PRIVATE #################################################### - # Returns a promise allowing other actions to be daisy-chained - # to the end of the download operation + # Returns a promise with a Download object, allowing other actions to be + # daisy-chained to the end of the download operation. _startDownload: (file, options = {}) -> @_prepareFolder(file).then => targetPath = @pathForFile(file) @@ -161,31 +152,46 @@ FileDownloadStore = Reflux.createStore download = @_downloads[file.id] return download.run() if download - # create a new download for this file and add it to our queue + # create a new download for this file download = new Download fileId: file.id filesize: file.size - filename: file.filename + filename: file.displayName() targetPath: targetPath progressCallback: => @trigger() - cleanup = => - @_cleanupDownload(download) - Promise.resolve(download) - - @_downloads[file.id] = download - promise = download.run().catch(cleanup).then(cleanup) - @trigger() - return promise - - _prepareFolder: (file) -> - new Promise (resolve, reject) => - folder = path.join(@_downloadDirectory, file.id) - fs.exists folder, (exists) => - if exists then resolve(folder) + # Do we actually need to queue and run the download? Queuing a download + # for an already-downloaded file has side-effects, like making the UI + # flicker briefly. + @_checkForDownloadedFile(file).then (downloaded) => + if downloaded + # If we have the file, just resolve with a resolved download representing the file. + download.promise = Promise.resolve() + return Promise.resolve(download) else - mkdirp folder, (err) => - if err then reject(err) else resolve(folder) + cleanup = => + @_cleanupDownload(download) + Promise.resolve(download) + @_downloads[file.id] = download + @trigger() + return download.run().catch(cleanup).then(cleanup) + + # Returns a promise that resolves with true or false. True if the file has + # been downloaded, false if it should be downloaded. + # + _checkForDownloadedFile: (file) -> + fs.statAsync(@pathForFile(file)).catch (err) => + return Promise.resolve(false) + .then (stats) => + return Promise.resolve(stats.size >= file.size) + + # Checks that the folder for the download is ready. Returns a promise that + # resolves when the download directory for the file has been created. + # + _prepareFolder: (file) -> + targetFolder = path.join(@_downloadDirectory, file.id) + fs.statAsync(targetFolder).catch => + mkdirpAsync(targetFolder) _fetch: (file) -> @_startDownload(file) @@ -226,10 +232,4 @@ FileDownloadStore = Reflux.createStore if not fs.existsSync(downloadDir) downloadDir = os.tmpdir() - path.join(downloadDir, @_filename(file.filename)) - - # Sometimes files can have no name. - _filename: (filename="") -> - if filename.trim().length is 0 - return UNTITLED - else return filename + path.join(downloadDir, file.displayName())