Skip to content

Conversation

@ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst commented Sep 6, 2016

This is a little proof-of-concept work-in-progress change that I've had in mind for some time now. The basic idea is that we now have smarter data structures compared to the code base 1-2 years ago. Since we now basically have most of the information organized nicely in Backbone models and collections, the client-side cache becomes more and more useless.

Small summary of what I've done so far

  • remove buggy cache module (cache.js)
  • do not clear the message collection, then we don't have to load it from cache (was kind of silly that we did it 🙈)
  • when fetching the content of a message, the data can be seen as some extra attributes of a message that have not been loaded so far. Therefore we can simply merge that data into the message model.
  • do not abort previous ajax requests as they won't cause concurrency issues anymore. Even better, we can use the data of the other request later because we store it in the Backbone model/collection and won't throw it away.

Bugs detected while developing:

  • the load-more-functionality seems to be broken
  • the background-fetcher, which preloads 10 message bodies in background, was temporarily disabled
  • the message list loading spinner looks a bit strange, maybe related to the bug above
  • multiple messages are shown as active
  • when reloading the message list, deleted messages are not removed from the list
  • when reloading the message list, the currently opened message loses its composer view

cc @nextcloud/mail feedback appreciated :-)

Note: While the original motivation to touch that code was only to make it more stable and/or remove some bad hacks, the user interface felt a lot faster (at least on FF). Maybe the code was too comlplex/inefficient before, but I think some better responsiveness of the UI would be a nice side effect of this cleanup 🚀

@jancborchardt
Copy link
Member

Oh yes dude! I’ve had lots of annoying problems with cache.js lately …

@jancborchardt
Copy link
Member

Check the »from« dropdown in the »Sent« folder or elsewhere. Weird »from <>« entries and other things. ;)

@ChristophWurst
Copy link
Member Author

Yeah, there are unfortunately some odd bugs with this PR. Therefore I'm planning to split it up in smaller, easier to review/test PRs

@ChristophWurst ChristophWurst modified the milestones: 0.6, 0.6.1 Sep 20, 2016
@ChristophWurst
Copy link
Member Author

I'll try to split the changes I made into small PRs. Closing this now

@lock
Copy link

lock bot commented Nov 21, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and questions.

@lock lock bot locked and limited conversation to collaborators Nov 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants