-
Notifications
You must be signed in to change notification settings - Fork 542
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
8459 convo actions label read export #8467
Conversation
8b7a382
to
d756690
Compare
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.
I want to have a closer look but looks good so far!
src/mail-app/mail/model/MailModel.ts
Outdated
@@ -432,6 +433,11 @@ export class MailModel { | |||
return !this.logins.isEnabled(FeatureType.DisableMailExport) | |||
} | |||
|
|||
async loadAndMarkMails(mails: () => Promise<readonly Mail[]>, unread: boolean): Promise<void> { |
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.
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.
cd43326
to
f71566a
Compare
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.
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
2ee60b2
to
dfb4d04
Compare
3458d59
to
214d1c0
Compare
980e994
to
c0d7723
Compare
Makes it different from MailListView component so it is easier to search
214d1c0
to
3797817
Compare
No description provided.