-
Notifications
You must be signed in to change notification settings - Fork 1k
fix for the list not being updated after emails were deleted in search bar #2611
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 7 commits
0f948f0
cc12f26
d69c253
6fff514
690486b
c3dd903
483da65
ecf0aaa
846c678
9edca2a
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 |
|---|---|---|
|
|
@@ -102,6 +102,28 @@ export class TrashButton extends React.Component<{ items: Thread[] }> { | |
| class HiddenGenericRemoveButton extends React.Component<{ items: Thread[] }> { | ||
| static displayName = 'HiddenGenericRemoveButton'; | ||
|
|
||
| _itemsForRemove = () => { | ||
| if (this.props.items && this.props.items.length > 0) { | ||
| return this.props.items; | ||
| } | ||
|
|
||
| const dataSource = ThreadListStore.dataSource(); | ||
| if (!dataSource) { | ||
| return []; | ||
| } | ||
|
|
||
| const focused = FocusedContentStore.focused('thread') as Thread; | ||
| if (focused) { | ||
| return [focused]; | ||
| } | ||
|
|
||
| if (dataSource.selection && dataSource.selection.count() > 0) { | ||
| return dataSource.selection.items() as Thread[]; | ||
| } | ||
|
|
||
| return []; | ||
|
Collaborator
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. Hmm this is an interesting change, it seems like the thinking here is that if you have both a selection and a focused item, the selection should take precedence? It'd be helpful to have a repro case if you had one in mind - I wonder if this makes more sense in the vertical split view? I think that this might be a good behavior improvement but it seems odd to do it for just the generic remove action and not the |
||
| }; | ||
|
|
||
| _onRemoveAndShift = ({ offset }) => { | ||
| const dataSource = ThreadListStore.dataSource(); | ||
| const focusedId = FocusedContentStore.focusedId('thread'); | ||
|
|
@@ -117,8 +139,13 @@ class HiddenGenericRemoveButton extends React.Component<{ items: Thread[] }> { | |
| }; | ||
|
|
||
| _onRemoveFromView = () => { | ||
| const items = this._itemsForRemove(); | ||
| if (items.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| const current = FocusedPerspectiveStore.current(); | ||
| const tasks = current.tasksForRemovingItems(this.props.items, 'Keyboard Shortcut'); | ||
| const tasks = current.tasksForRemovingItems(items, 'Keyboard Shortcut'); | ||
| Actions.queueTasks(tasks); | ||
| Actions.popSheet(); | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,6 @@ | ||
| import React from 'react'; | ||
| import { | ||
| localized, | ||
| Folder, | ||
| ChangeLabelsTask, | ||
| ChangeFolderTask, | ||
| AccountStore, | ||
| CategoryStore, | ||
| TaskFactory, | ||
| MailboxPerspective, | ||
| Actions, | ||
|
|
@@ -82,31 +77,9 @@ class SearchMailboxPerspective extends MailboxPerspective { | |
| } | ||
|
|
||
| tasksForRemovingItems(threads, source?: string) { | ||
|
Contributor
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. Behavioral change for Gmail users: The previous implementation checked
Collaborator
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. Hmm I think this comment is correct, there are sort of separate terms - "remove" vs "trash" vs "archive", and in places where a key binding or helper is tied to "remove", the behavior is meant to be configured by I think for most users clicking the "Backspace" key in a gmail search result list should archive messages and not move them to the trash. |
||
| return TaskFactory.tasksForThreadsByAccountId(threads, (accountThreads, accountId) => { | ||
| const account = AccountStore.accountForId(accountId); | ||
| if (!account) { | ||
| return []; | ||
| } | ||
| const dest = account.preferredRemovalDestination(); | ||
| if (!dest) { | ||
| return []; | ||
| } | ||
| if (dest instanceof Folder) { | ||
| return new ChangeFolderTask({ | ||
| threads: accountThreads, | ||
| source: 'Dragged out of list', | ||
| folder: dest, | ||
| }); | ||
| } | ||
| if (dest.role === 'all') { | ||
| // if you're searching and archive something, it really just removes the inbox label | ||
| return new ChangeLabelsTask({ | ||
| threads: accountThreads, | ||
| source: 'Dragged out of list', | ||
| labelsToRemove: [CategoryStore.getInboxCategory(accountId)], | ||
| }); | ||
| } | ||
| throw new Error('Unexpected destination returned from preferredRemovalDestination()'); | ||
| return TaskFactory.tasksForMovingToTrash({ | ||
| threads, | ||
| source: source || 'Keyboard Shortcut', | ||
| }); | ||
| } | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.