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.

Sign up for GitHub

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

Jump to bottom

Conversation list model #8351

Open
wants to merge 41 commits into
base: dev-mail
Choose a base branch
from
Open

Conversation list model #8351

wants to merge 41 commits into from

Conversation

paw-hub
Copy link
Contributor

@paw-hub paw-hub commented Jan 22, 2025

No description provided.

@paw-hub paw-hub force-pushed the 8223-conversation-list-model branch from 20ba81d to 72a61ad Compare January 23, 2025 17:41
@BijinDev BijinDev force-pushed the 8223-conversation-list-model branch from 1ded82d to 0367ecb Compare January 24, 2025 09:03
@BijinDev BijinDev force-pushed the 8223-conversation-list-model branch from 4d5eaf9 to a3c9c89 Compare January 27, 2025 08:24
@BijinDev BijinDev marked this pull request as ready for review January 27, 2025 08:26
@BijinDev BijinDev linked an issue Jan 27, 2025 that may be closed by this pull request
Conversation-based list model #8223
Open
40 tasks
@BijinDev BijinDev force-pushed the 8223-conversation-list-model branch from a3c9c89 to 2e54f1d Compare January 28, 2025 13:41
charlag
charlag reviewed Jan 30, 2025
View reviewed changes
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 still didn't get everything but it's already a great effort!

src/mail-app/mail/model/ConversationListModel.ts Show resolved Hide resolved
src/mail-app/mail/model/ConversationListModel.ts Outdated Show resolved Hide resolved
src/mail-app/mail/model/ConversationListModel.ts Outdated Show resolved Hide resolved
src/mail-app/mail/model/ConversationListModel.ts Outdated Show resolved Hide resolved
src/mail-app/mail/model/ConversationListModel.ts Outdated Show resolved Hide resolved
charlag
charlag reviewed Feb 3, 2025
View reviewed changes
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.

idk why my comments are pending sorry

src/mail-app/mail/model/MailSetListModel.ts Outdated Show resolved Hide resolved
src/mail-app/mail/model/ConversationListModel.ts Outdated Show resolved Hide resolved
@wrdhub
Copy link
Contributor

wrdhub commented Feb 3, 2025

Another thing I noticed: when you apply or remove a label this change is not shown in the mail list immediately. You have to reload to see it.

paw-hub and others added 11 commits February 3, 2025 13:36
@paw-hub @BijinDev
Add ConversationListModel
2bcb859 
Co-authored-by: bir <[email protected]>
@BijinDev @paw-hub
Fix entity updates
ccf3679 
Co-authored-by: paw <[email protected]>

close: #8223
@paw-hub @BijinDev
WIP
8811653 
Co-authored-by: bir <[email protected]>
@BijinDev @paw-hub
Remove getMailSetEntry
294e88d 
Co-authored-by: paw-hub <[email protected]>
@BijinDev @paw-hub
Do not replace the mail in-place when mailsetentry is deleted
23459c7 
After some discussion, we decided it is preferred to have the
conversation jump around in the list if the tail of it is removed from
the list (but older mails in the conversation are still in it).

Co-authored-by: paw-hub <[email protected]>
@BijinDev @paw-hub
Do not use conversation list for draft/sent folders
e015839 
Co-authored-by: paw-hub <[email protected]>
@BijinDev @paw-hub
Add setting for whether to use the conversation view model
953d9e8 
Still needs translation!

Co-authored-by: paw-hub <[email protected]>
@BijinDev @paw-hub
Fix Url->list interaction
c63dc5a 
Co-authored-by: paw <[email protected]>

close: #8223
@BijinDev @paw-hub
Extend memoized() to handle multiple arguments
0ebdf4c
@BijinDev @paw-hub
Rename conversation in list pref, fix a bug when switching pref
e1d15ce
@paw-hub
Look in mailMap for the latest entry
801cffb 
We already store older mails, but we weren't using them for some reason.
@paw-hub paw-hub force-pushed the 8223-conversation-list-model branch from d63b06b to 801cffb Compare February 3, 2025 12:59
@paw-hub paw-hub force-pushed the 8223-conversation-list-model branch from 0175167 to fbb87ef Compare February 3, 2025 14:00
@paw-hub
Copy link
Contributor Author

paw-hub commented Feb 3, 2025

Another thing I noticed: when you apply or remove a label this change is not shown in the mail list immediately. You have to reload to see it.

This should now be fixed. Thanks for reporting this!

paw-hub and others added 26 commits February 3, 2025 15:07
@paw-hub @BijinDev
Use groupByAndMap for provideAllMails
fc090a7 
Co-authored-by: bir <[email protected]>
@paw-hub @BijinDev
Make LoadedConversation readonly
49fe12a 
Co-authored-by: bir <[email protected]>
@paw-hub @BijinDev
Fix another potential race condition
a091a62 
Co-authored-by: bir <[email protected]>
@paw-hub @BijinDev
Cache items() and getSelectedAsArray() with memoized
0404590 
Co-authored-by: bir <[email protected]>
@paw-hub
Items should return ReadonlyArray
8d20592
@paw-hub @BijinDev
Document MailSetListModel
8693c83 
Co-authored-by: bir <[email protected]>
@paw-hub @BijinDev
Fix DeviceConfigTest
e9db464 
Co-authored-by: bir <[email protected]>
@paw-hub @BijinDev
Fix update selecting the same mail
ec56676 
oops

Co-authored-by: bir <[email protected]>
@paw-hub @BijinDev
Don't change state until after the await
a1cf3ab 
Co-authored-by: bir <[email protected]>
@paw-hub @BijinDev
Fix getLabelsForMail throwing???
434481f 
This seems to be caused when a mail is removed from the
list.

Is this actually a fix, though? This doesn't seem right.

Co-authored-by: bir <[email protected]>
@paw-hub @BijinDev
Fix updating the conversation
1701fd1 
Co-authored-by: bir <[email protected]>
@paw-hub @BijinDev
Remove old behavior accidentally left in
25a2fe5 
This caused mails to be removed from the map prematurely on update.

Co-authored-by: bir <[email protected]>
@paw-hub @BijinDev
It's probably fine
9d9196f 
We looked at this one with ivk

Co-authored-by: bir <[email protected]>
@paw-hub
WIP: Readd fixCounterIfNeeded
6b45c7a 
Co-authored-by: bir bir<@tutao.de>
@paw-hub @BijinDev
Use a ReadonlyMap for the mail map
a6e75a0 
Co-authored-by: bir <[email protected]>
@paw-hub @BijinDev
Add translation keys
aa2327e 
Co-authored-by: bir <[email protected]>
@paw-hub @BijinDev
Map is in insertion order
0871228 
Co-authored-by: bir <[email protected]>
@paw-hub @BijinDev
WIP: Copy MailListModel tests to ConversationListModel
e778b2d 
Co-authored-by: bir <[email protected]>
@paw-hub @BijinDev
Note in MailSetListModel that mails is not ordered
2c175f9 
Co-authored-by: bir <[email protected]>
@paw-hub @BijinDev
Fix duplicate conversations in list caught by WIP tests
3b5042f 
Co-authored-by: bir <[email protected]>
@paw-hub @BijinDev
WIP: Unit tests for ConversationListModel
6fad262 
Co-authored-by: bir <[email protected]>
@paw-hub
mapWithout
33baced 
Thanks ivk
@paw-hub @BijinDev
Split loadMore tests
e3674fd 
Co-authored-by: bir <[email protected]>
@paw-hub @BijinDev
How did this get deleted???
9bfbb02 
Co-authored-by: bir <[email protected]>
@paw-hub @BijinDev
Add unit tests for ConversationListModel
b3b65eb 
Co-authored-by: bir <[email protected]>
@paw-hub @BijinDev
Oops
337048c 
Co-authored-by: bir <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@charlag charlag charlag left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Conversation-based list model
4 participants
@paw-hub @wrdhub @charlag @BijinDev