From 0134c8a4b3d90c765ef9e26d0ed4292a76c1b5a5 Mon Sep 17 00:00:00 2001 From: Ben Gotow Date: Sun, 23 Jun 2019 00:24:24 -0500 Subject: [PATCH] =?UTF-8?q?Sentry=20fix:=20Don=E2=80=99t=20allow=20labels?= =?UTF-8?q?=20to=20be=20selected=20as=20the=20Gmail=20trash=20folder,=20se?= =?UTF-8?q?e=20description?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is neat but Gmail has very specific semantics around labels vs the “spam”, “trash” and “all mail” folders. If you choose a label as your Trash the email still appears in all mail and Mailspring freaks out in several places. We could fix support for this scenarrio, but it’s unlikely this is what users actually want. --- .../category-mapper/lib/category-selection.tsx | 6 +++--- .../lib/preferences-category-mapper.tsx | 10 ++++++++-- app/src/mailbox-perspective.ts | 14 +++++++------- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/app/internal_packages/category-mapper/lib/category-selection.tsx b/app/internal_packages/category-mapper/lib/category-selection.tsx index 5bd6a515b..2d73be1f2 100644 --- a/app/internal_packages/category-mapper/lib/category-selection.tsx +++ b/app/internal_packages/category-mapper/lib/category-selection.tsx @@ -9,7 +9,7 @@ import { import { localized, Label, Utils, PropTypes } from 'mailspring-exports'; interface CategorySelectionProps { - accountUsesLabels: boolean; + allowLabels: boolean; all: CategoryItem[]; current: CategoryItem; onSelect: (item: CategoryItem) => void; @@ -31,7 +31,7 @@ export default class CategorySelection extends React.Component< CategorySelectionState > { static propTypes = { - accountUsesLabels: PropTypes.bool, + allowLabels: PropTypes.bool, all: PropTypes.array, current: PropTypes.object, onSelect: PropTypes.func, @@ -95,7 +95,7 @@ export default class CategorySelection extends React.Component< }; render() { - const placeholder = this.props.accountUsesLabels + const placeholder = this.props.allowLabels ? localized('Choose folder or label') : localized('Choose folder'); diff --git a/app/internal_packages/category-mapper/lib/preferences-category-mapper.tsx b/app/internal_packages/category-mapper/lib/preferences-category-mapper.tsx index 291fcf8b0..b54891d5c 100644 --- a/app/internal_packages/category-mapper/lib/preferences-category-mapper.tsx +++ b/app/internal_packages/category-mapper/lib/preferences-category-mapper.tsx @@ -5,6 +5,7 @@ import { Category, Actions, ChangeRoleMappingTask, + Folder, } from 'mailspring-exports'; import CategorySelection from './category-selection'; @@ -79,15 +80,20 @@ export default class PreferencesCategoryMapper extends React.Component<{}, State if (account.provider === 'gmail' && role === 'archive') { return false; } + + let all = this.state.all[account.id]; + const allowLabels = account.usesLabels() && role !== 'trash' && role !== 'spam'; + if (!allowLabels) all = all.filter(c => c instanceof Folder); + return (
{Category.LocalizedStringForRole[role]}:
this._onCategorySelection(account, role, category)} - accountUsesLabels={account.usesLabels()} + allowLabels={allowLabels} />
diff --git a/app/src/mailbox-perspective.ts b/app/src/mailbox-perspective.ts index 3c691ec49..592643060 100644 --- a/app/src/mailbox-perspective.ts +++ b/app/src/mailbox-perspective.ts @@ -536,15 +536,15 @@ class CategoryMailboxPerspective extends MailboxPerspective { ChangeFolderTask = ChangeFolderTask || require('./flux/tasks/change-folder-task').ChangeFolderTask; - // TODO this is an awful hack - const role = this.isArchive() ? 'archive' : this.categoriesSharedRole(); - - if (role === 'spam' || role === 'trash') { - return []; + // If you are viewing the archive, "remove" goes to the trash, since obeying + // your rpreference would mean possibly doing nothing (if you default to archive.) + if (this.isArchive()) { + return TaskFactory.tasksForMovingToTrash({ threads, source }); } - if (role === 'archive') { - return TaskFactory.tasksForMovingToTrash({ threads, source }); + // If you are viewing spam or trash, "remove" does nothing + if (['spam', 'trash'].includes(this.categoriesSharedRole())) { + return []; } return TaskFactory.tasksForThreadsByAccountId(threads, (accountThreads, accountId) => {