Add possibility to add the messages account/folder as a theme content
item. Additionally, show the path in the messages tool tip.
FEATURE: 420644
FIXED-IN:
dvratil |
VDG |
Add possibility to add the messages account/folder as a theme content
item. Additionally, show the path in the messages tool tip.
FEATURE: 420644
FIXED-IN:
New tool tip:
New "Folder" item in content items
Message list:
No Linters Available |
No Unit Test Coverage |
Buildable 26972 | |
Build 26990: arc lint + arc unit |
I just noticed that my original goal, showing the "real" folder of an item inside a smart list or search, does not work like this. As the the "parentCollection" actually is the smart list/search, it will not show the original folder :-(
You could still do it but it's a bit more complex: you can use Item::storageCollectionId() to get ID of the real parent collection. You would then need to use the ID against EntityTreeModel to resolve the /actual/ Collection with displayName populated.
Maybe "StorageModel::collectionFromId()" rather than "StorageModel::collectionForId()" would be better?
collectionForId is a good name, I think.
messagelist/src/storagemodel.cpp | ||
---|---|---|
263 | This will use a ton of memory and will make populating the model slow - for each single email you will need to resolve the real parent collection, traverse the collection parent chain and assemble the string. I suggest you add a QHash<Collection::Id, QString> lookup table into the StorageModel and try to look up the path by the collection ID there first, then fallback to building the path and inserting it into the hash map. This way we will make use of the implicit QString sharing so all items from the same collection will share the same single string with the path reducing memory footprint, and we will avoid assembling the string every time, which is more costly than a hash map lookup. Remember to clear the hash map when the selected folder is changed. | |
506 | You can pass the d->mChildrenFilterModel here instead of the etm, the code inside is clever enough to find the ETM source model. |
messagelist/src/storagemodel.cpp | ||
---|---|---|
263 | What do you mean by "selected folder"? I thought the collection ID is globally unique? Why can it not be re-used independent from any selection? | |
506 | Actually, this does not work. Neither passing "d->mChildrenFilterModel" nor "static_cast<QAbstractProxyModel *>(d->mChildrenFilterModel)" will produce a result, "idx" is invalid. |
messagelist/src/storagemodel.cpp | ||
---|---|---|
262 | I would trim the trailing '/' from the folder path here, so that it looks like "Local Folders/Inbox" rather than "Local Folders/Inbox/" | |
263 | Collection ID is globally unique, this is about saving resources. Note that you are touching some very very performance and memory-sensitive parts of code here. If you open a virtual search folder, each item can be from a different folder, so you will populate the hashtable with many mappings. However, when you then switch to a regular folder, you only need a single mapping, because all Items there belong to a single collection. Without clearing the hashtable on folder switch, you would be
Since majority of usecases is browsing regular folders, where there will be only single mapping, you should optimize for that. | |
506 | Interesting, but very well, then. |
Thanks for the changes. One more change/optimization, please.
messagelist/src/storagemodel.cpp | ||
---|---|---|
259 | This requires two lookups: first in the contains() and then in the value(). Use find() instead, which returns an iterator. When the iterator is invalid, assemble the string and insert it, otherwise get the value from the iterator. |
Tried it locally and it's pretty cool. Just some nitpicks in the recent changes.
messagelist/src/storagemodel.cpp | ||
---|---|---|
260 ↗ | (On Diff #82898) | Use auto instead of typing out the entire iterator type, please. |
270 ↗ | (On Diff #82898) | This does an unnecessary copy of the substring, I'd propose to do folder.chop(1) after the while loop, which just shrinks the current string, then you can insert it directly into the hash table. |
Abandon the other one and try updating this one again. You can use arc diff --update D29602 to make sure it updates this one.
Looks good to me know. Thanks for this patch, it's a pretty cool feature, especially for the "Unified mailboxes" agent 👍
@ngraham: this is a feature for the weekly report. What is the proposed way to ping you for things to blog about?