Skip to content
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

8459 convo actions label read export #8467

Merged

Conversation

wrdhub
Copy link
Contributor

@wrdhub wrdhub commented Feb 4, 2025

No description provided.

@wrdhub wrdhub linked an issue Feb 4, 2025 that may be closed by this pull request
22 tasks
@wrdhub wrdhub changed the base branch from dev-mail to 8223-conversation-list-model February 4, 2025 15:01
@wrdhub wrdhub force-pushed the 8459-convo-actions-label-read-export branch 2 times, most recently from 8b7a382 to d756690 Compare February 4, 2025 16:49
Copy link
Contributor

@charlag charlag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to have a closer look but looks good so far!

@@ -432,6 +433,11 @@ export class MailModel {
return !this.logins.isEnabled(FeatureType.DisableMailExport)
}

async loadAndMarkMails(mails: () => Promise<readonly Mail[]>, unread: boolean): Promise<void> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

load methods seems like they should not be on the MailModel. I know you are probably trying to reduce duplication but we should think about it again I guess.

@paw-hub paw-hub force-pushed the 8223-conversation-list-model branch 5 times, most recently from cd43326 to f71566a Compare February 6, 2025 09:50
Copy link
Contributor

@charlag charlag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, this was a lot of work and you did all the heavy lifting for actions already! I have mostly nitpicks and one small concern but I think it's really close now

src/mail-app/mail/model/MailModel.ts Outdated Show resolved Hide resolved
src/mail-app/mail/model/MailModel.ts Outdated Show resolved Hide resolved
src/mail-app/mail/view/ConversationViewModel.ts Outdated Show resolved Hide resolved
src/mail-app/mail/view/MailView.ts Outdated Show resolved Hide resolved
src/mail-app/mail/view/MailGuiUtils.ts Outdated Show resolved Hide resolved
src/mail-app/mail/view/MailViewModel.ts Show resolved Hide resolved
src/mail-app/mail/view/MailViewModel.ts Outdated Show resolved Hide resolved
src/mail-app/mail/view/MailViewModel.ts Outdated Show resolved Hide resolved
src/mail-app/mail/view/MailViewerToolbar.ts Outdated Show resolved Hide resolved
src/mail-app/mail/view/MailViewerToolbar.ts Outdated Show resolved Hide resolved
@charlag charlag force-pushed the 8223-conversation-list-model branch from 2ee60b2 to dfb4d04 Compare February 6, 2025 15:10
@wrdhub wrdhub force-pushed the 8459-convo-actions-label-read-export branch 2 times, most recently from 3458d59 to 214d1c0 Compare February 6, 2025 16:13
@charlag charlag force-pushed the 8223-conversation-list-model branch from 980e994 to c0d7723 Compare February 6, 2025 16:18
Base automatically changed from 8223-conversation-list-model to conversation-integration February 6, 2025 16:29
@charlag charlag force-pushed the 8459-convo-actions-label-read-export branch from 214d1c0 to 3797817 Compare February 6, 2025 16:31
@charlag charlag merged commit 3797817 into conversation-integration Feb 6, 2025
6 checks passed
@charlag charlag deleted the 8459-convo-actions-label-read-export branch February 6, 2025 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Conversation actions: Labels, Read/Unread, and Export
2 participants