-
Notifications
You must be signed in to change notification settings - Fork 1k
Add support for dragging threads into composer as .eml attachments #2800
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
base: master
Are you sure you want to change the base?
Changes from 2 commits
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 |
|---|---|---|
| @@ -1,9 +1,16 @@ | ||
| import React from 'react'; | ||
| import { Actions, AttachmentStore, Message } from 'mailspring-exports'; | ||
| import { AttachmentItem } from 'mailspring-component-kit'; | ||
| import { localized, Actions, AttachmentStore, Message } from 'mailspring-exports'; | ||
| import { AttachmentItem, RetinaImg } from 'mailspring-component-kit'; | ||
|
|
||
| export const AttachmentsArea: React.FunctionComponent<{ draft: Message }> = (props) => { | ||
| export const AttachmentsArea: React.FunctionComponent<{ | ||
| draft: Message; | ||
| // Number of dragged-in threads whose .eml files are still being fetched from | ||
| // the sync engine. They aren't files on the draft yet, so they're rendered | ||
| // here as placeholders to show the drop was accepted. | ||
| attachingThreadCount?: number; | ||
| }> = (props) => { | ||
| const { files, headerMessageId } = props.draft; | ||
| const attachingThreadCount = props.attachingThreadCount || 0; | ||
|
|
||
| return ( | ||
| <div className="attachments-area"> | ||
|
|
@@ -20,6 +27,16 @@ export const AttachmentsArea: React.FunctionComponent<{ draft: Message }> = (pro | |
| onRemoveAttachment={() => Actions.removeAttachment(headerMessageId, file)} | ||
| /> | ||
| ))} | ||
| {attachingThreadCount > 0 && ( | ||
| <div className="attaching-messages"> | ||
| <RetinaImg name="inline-loading-spinner.gif" mode={RetinaImg.Mode.ContentPreserve} /> | ||
| <span> | ||
| {attachingThreadCount === 1 | ||
| ? localized('Attaching message…') | ||
|
Collaborator
Author
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. Can we collapse this into just
Collaborator
Author
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. I think we do have to keep the state at the composer-view layer for tracking when we're done, just remove it from this component |
||
| : localized('Attaching %1$@ messages…', attachingThreadCount)} | ||
| </span> | ||
| </div> | ||
| )} | ||
| </div> | ||
| ); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ import { | |
| DraftStore, | ||
| DraftEditingSession, | ||
| MessageWithEditorState, | ||
| EmlUtils, | ||
| File, | ||
| } from 'mailspring-exports'; | ||
| import { webUtils } from 'electron'; | ||
|
|
@@ -41,6 +42,7 @@ interface ComposerViewState { | |
| quotedTextHidden: boolean; | ||
| quotedTextPresent: boolean; | ||
| isDropping: boolean; | ||
| attachingThreadCount: number; | ||
| } | ||
| // The ComposerView is a unique React component because it (currently) is a | ||
| // singleton. Normally, the React way to do things would be to re-render the | ||
|
|
@@ -78,6 +80,7 @@ export default class ComposerView extends React.Component<ComposerViewProps, Com | |
|
|
||
| this.state = { | ||
| isDropping: false, | ||
| attachingThreadCount: 0, | ||
| quotedTextPresent: hasBlockquote(draft.bodyEditorState), | ||
| quotedTextHidden: hideQuotedTextByDefault(draft), | ||
| }; | ||
|
|
@@ -202,7 +205,7 @@ export default class ComposerView extends React.Component<ComposerViewProps, Com | |
| </> | ||
| )} | ||
|
|
||
| <AttachmentsArea draft={draft} /> | ||
| <AttachmentsArea draft={draft} attachingThreadCount={this.state.attachingThreadCount} /> | ||
| </div> | ||
| <div className="composer-footer-region"> | ||
| <InjectedComponentSet | ||
|
|
@@ -278,7 +281,14 @@ export default class ComposerView extends React.Component<ComposerViewProps, Com | |
| const hasNativeFile = event.dataTransfer.types.includes('Files'); | ||
| const hasNonNativeFilePath = nonNativeFilePath !== null; | ||
|
|
||
| return hasNativeFile || hasNonNativeFilePath; | ||
| return hasNativeFile || hasNonNativeFilePath || this._hasThreadsForDrop(event); | ||
| }; | ||
|
|
||
| // Threads dragged out of the thread list carry their ids in a custom MIME | ||
| // type. Note that we can only look at `types` here — the payload itself is | ||
| // not readable until the drop actually happens. | ||
| _hasThreadsForDrop = (event: React.DragEvent<HTMLDivElement>) => { | ||
| return event.dataTransfer.types.includes('mailspring-threads-data'); | ||
|
Collaborator
Author
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. can you make |
||
| }; | ||
|
|
||
| _nonNativeFilePathForDrop = (event: React.DragEvent<HTMLDivElement>) => { | ||
|
|
@@ -313,6 +323,83 @@ export default class ComposerView extends React.Component<ComposerViewProps, Com | |
| this._onFileReceived(uri); | ||
| event.preventDefault(); | ||
| } | ||
|
|
||
| // Accept drops of threads from the thread list, attaching each one as a | ||
| // .eml file. dataTransfer is only valid for the duration of this handler, | ||
| // so read the payload now and hand the ids off to an async worker. | ||
|
Collaborator
Author
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. please remove this comment it's self explanatory |
||
| if (this._hasThreadsForDrop(event)) { | ||
| this._onThreadsReceived(event.dataTransfer.getData('mailspring-threads-data')); | ||
| event.preventDefault(); | ||
| } | ||
| }; | ||
|
|
||
| _onThreadsReceived = async (json: string) => { | ||
| let threadIds: string[] = []; | ||
| try { | ||
| threadIds = JSON.parse(json).threadIds || []; | ||
| } catch (err) { | ||
| return; | ||
| } | ||
| if (!threadIds.length) { | ||
|
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.
The self-drop guard that was here in the first commit ( |
||
| return; | ||
| } | ||
|
|
||
| // Fetching the raw message from the sync engine is a remote round trip, so | ||
| // show a placeholder in the attachments area until the files land. | ||
| const dropCount = threadIds.length; | ||
| this.setState((state) => ({ attachingThreadCount: state.attachingThreadCount + dropCount })); | ||
|
|
||
| let staged: Array<{ filePath: string }> = []; | ||
| let unavailableThreadIds: string[] = []; | ||
| try { | ||
| ({ staged, unavailableThreadIds } = await EmlUtils.stageThreadsAsEml(threadIds)); | ||
| } catch (err) { | ||
| AppEnv.reportError(err); | ||
| } finally { | ||
| if (this._mounted) { | ||
| this.setState((state) => ({ | ||
| attachingThreadCount: state.attachingThreadCount - dropCount, | ||
| })); | ||
| } | ||
| } | ||
|
|
||
| if (!this._mounted) { | ||
| return; | ||
| } | ||
| for (const { filePath } of staged) { | ||
| Actions.addAttachment({ | ||
| filePath, | ||
| headerMessageId: this.props.draft.headerMessageId, | ||
| // The attachment store copies the file into its own directory before | ||
| // this fires, so the staged copy is free to go. | ||
| onCreated: () => EmlUtils.discardStagedEml(filePath), | ||
| }); | ||
| } | ||
|
|
||
| // A thread with nothing exportable in it and a fetch that failed are | ||
| // different problems, and only the second one is worth retrying. | ||
| if (unavailableThreadIds.length) { | ||
| AppEnv.showErrorDialog( | ||
| unavailableThreadIds.length === 1 | ||
| ? localized('This conversation has no message that can be attached.') | ||
| : localized( | ||
| '%1$@ of the conversations have no message that can be attached.', | ||
| unavailableThreadIds.length | ||
| ) | ||
|
Collaborator
Author
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. Can we make this say "one or more" so it's a single localized string, just to simplify the amount of localization required |
||
| ); | ||
| } | ||
|
|
||
| const failedCount = threadIds.length - unavailableThreadIds.length - staged.length; | ||
| if (failedCount > 0) { | ||
| AppEnv.showErrorDialog( | ||
| failedCount === 1 | ||
| ? localized('Could not download the original message. Please try again.') | ||
| : localized( | ||
| 'Could not download %1$@ of the original messages. Please try again.', | ||
| failedCount | ||
|
Collaborator
Author
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. same with this one, let's make it a single error dialog by rephrasing it to say one or more |
||
| ) | ||
| ); | ||
| } | ||
| }; | ||
|
|
||
| _onFileReceived = (filePath: string) => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -600,6 +600,20 @@ body.platform-win32 { | |
| } | ||
| } | ||
|
|
||
| // Placeholder shown while a thread dragged onto the composer is being fetched | ||
| // from the sync engine and written out as a .eml file. | ||
| .attaching-messages { | ||
|
Collaborator
Author
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. |
||
| display: flex; | ||
| align-items: center; | ||
| padding: @spacing-half 0; | ||
| color: @text-color-subtle; | ||
| font-size: @font-size-smaller; | ||
|
|
||
| img { | ||
| margin-right: @spacing-half; | ||
| } | ||
| } | ||
|
|
||
| // Overrides for the full-window popout composer | ||
| .composer-full-window { | ||
| width: 100%; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -165,30 +165,15 @@ export default class ThreadListContextMenu { | |
| label: localized('Forward as Attachment'), | ||
| click: async () => { | ||
| const thread = this.threads[0]; | ||
| const messages = await DatabaseStore.findAll<Message>(Message, { threadId: thread.id }) | ||
| .order(Message.attributes.date.descending()) | ||
| .limit(1); | ||
| const messages = await EmlUtils.newestExportableMessagesForThreadIds([thread.id]); | ||
| if (!messages.length) return; | ||
|
|
||
| const message = messages[0]; | ||
| const pathModule = require('path'); | ||
| const fs = require('fs'); | ||
| const tempDir = pathModule.join( | ||
| require('@electron/remote').app.getPath('temp'), | ||
| `mailspring-fwd-${message.id}` | ||
| ); | ||
| fs.mkdirSync(tempDir, { recursive: true }); | ||
| const tempPath = pathModule.join(tempDir, 'Forwarded Message.eml'); | ||
|
|
||
| const task = new GetMessageRFC2822Task({ | ||
| messageId: message.id, | ||
| accountId: message.accountId, | ||
| filepath: tempPath, | ||
| const staged = await EmlUtils.stageMessagesAsEml([message], { | ||
|
Collaborator
Author
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. can we write a wrapper
Collaborator
Author
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. edit: i think we actually want |
||
| filename: 'Forwarded Message.eml', | ||
| }); | ||
| Actions.queueTask(task); | ||
| await TaskQueue.waitForPerformRemote(task); | ||
|
|
||
| if (!fs.existsSync(tempPath)) { | ||
| if (!staged.length) { | ||
| AppEnv.showErrorDialog( | ||
| localized('Could not download the original message. Please try again.') | ||
| ); | ||
|
|
@@ -207,9 +192,10 @@ export default class ThreadListContextMenu { | |
| await TaskQueue.waitForPerformLocal(syncTask); | ||
|
|
||
| Actions.addAttachment({ | ||
| filePath: tempPath, | ||
| filePath: staged[0].filePath, | ||
| headerMessageId: draft.headerMessageId, | ||
| onCreated: () => { | ||
| EmlUtils.discardStagedEml(staged[0].filePath); | ||
| Actions.composePopoutDraft(draft.headerMessageId); | ||
| }, | ||
| }); | ||
|
|
@@ -340,9 +326,7 @@ export default class ThreadListContextMenu { | |
| click: async () => { | ||
| if (this.threadIds.length === 1) { | ||
| const thread = this.threads[0]; | ||
| const messages = await DatabaseStore.findAll<Message>(Message, { threadId: thread.id }) | ||
| .order(Message.attributes.date.descending()) | ||
| .limit(1); | ||
| const messages = await EmlUtils.newestExportableMessagesForThreadIds([thread.id]); | ||
| if (!messages.length) return; | ||
|
|
||
| const message = messages[0]; | ||
|
|
@@ -369,15 +353,11 @@ export default class ThreadListContextMenu { | |
| const outputDir = selected[0]; | ||
| const path = require('path'); | ||
|
|
||
| for (const thread of this.threads) { | ||
| const messages = await DatabaseStore.findAll<Message>(Message, { | ||
| threadId: thread.id, | ||
| }) | ||
| .order(Message.attributes.date.descending()) | ||
| .limit(1); | ||
| if (!messages.length) continue; | ||
| const messages = await EmlUtils.newestExportableMessagesForThreadIds( | ||
| this.threads.map((t) => t.id) | ||
| ); | ||
|
|
||
| const message = messages[0]; | ||
| for (const message of messages) { | ||
| const filename = EmlUtils.defaultEmlFilename(message.subject); | ||
|
|
||
| const task = new GetMessageRFC2822Task({ | ||
|
|
||

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.
This image looks like shit, can you use the Spinner component? In a follow-up we'll clean up other remaining uses of this. The problem is it doesn't look good in dark mode and can't be inverted.