Add AccountsChannelsModel
ClosedPublic

Authored by olivierjg on Mon, Mar 23, 1:12 AM.

Details

Summary

Simple model to forward accounts and channels as a tree.

Diff Detail

Repository
R865 Ruqola
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 24244
Build 24262: arc lint + arc unit
olivierjg requested review of this revision.Mon, Mar 23, 1:12 AM
olivierjg created this revision.

Hi,
I don't know if it works. But until this mode is optional it's not a problem. In kdab I have 43 channels, in my old kdab accoutn I have 7 channels, and in my openrochetchat 8 channels => 58 channels by default and I don't want to scroll all the time for showing where I have an useful info (new message, new ping etc.)

Be careful you mustn"t break ruqolaqml version
You need to find a method for allowing to change specific status for each account.
You need to rewrite logout/login page.
You need to add option in configure dialog box.
(Perhaps there is other feature that you need to adapt too, I can't see for the moment).

It's good to submit it when it's ok in a branch for avoiding to break all.

Thanks
Regards.

It's good to submit it when it's ok in a branch for avoiding to break all.

This is just the model + test, no modifications to the UI are included in this patch. The only change other than the file additions is to respond to Qt::DisplayRole in the accounts model. No change to the behavior of either ruqola or ruqolaqml.

Good start. There are some clever tricks in here :) I just have some suggestions for more readability.

Laurent wrote:

I don't want to scroll all the time for showing where I have an useful info (new message, new ping etc.)

That's what "Unread on Top" solves (at the expense of some jumping around, but I find it manageable).
Switch [to another account] + scroll isn't really better IMHO, takes even more time.

src/core/model/accountschannelsmodel.cpp
42

i is a row number in the accounts proxy model, right?

It gets a bit confusing with the i on line 44... Maybe this one could be rowNumber or proxyRow...
(of course they could both be, but the point is to use a different name, otherwise it looks like we're comparing roomsModel(i) with roomsModel(i)...)

44

I was confused when reading this because proxyModel() looks like a getter for a proxyModel.
Now I realize this is "proxy" as a verb, as in "please proxy this model".
But in fact the proxy *is* the first argument, it's not like the method is going to create it...

So I suggest setupRoomsModel or setupProxyModel or something like that.

That's what "Unread on Top" solves (at the expense of some jumping around, but I find it manageable).
Switch [to another account] + scroll isn't really better IMHO, takes even more time.

So we will have:
"unread on top:

  • > account 1
    • > channel 1
    • > channel 2 -> ...
  • > account 2
    • > channel 1
    • > channel 2 -> ...
  • > account 3
    • > channel 1
    • > channel 2 -> ...

Account 1

    • > favorite
      • > channel 1
      • > channel 2
  • > direct message
    • > channel 1
    • > channel 2
  • > channel
    • > channel 1
    • > channel 2
  • > discussion
    • > channel 1
    • > channel 2

Account 2

    • > favorite
      • > channel 1
      • > channel 2
  • > direct message
    • > channel 1
    • > channel 2
  • > channel
    • > channel 1
    • > channel 2
  • > discussion
    • > channel 1
    • > channel 2

Account 3

    • > favorite
      • > channel 1
      • > channel 2
  • > direct message
    • > channel 1
    • > channel 2
  • > channel
    • > channel 1
    • > channel 2
  • > discussion
    • > channel 1
    • > channel 2

So we will need to implement "Account order" too as people will want to see Account 3 in first for example.

And when we will press ESC in channel which was in "unread on top" (for example from account3) will jump in the end of list.
"Unread on top" can be useful when we want to see each channel where there are unread message but it will be difficult to find specific channel when we have several account => we will scroll it.

It seems that you want it, ok but as optional.

olivierjg updated this revision to Diff 78511.Thu, Mar 26, 1:22 AM

Naming changes

olivierjg marked 2 inline comments as done.Thu, Mar 26, 1:23 AM
dfaure accepted this revision.Sat, Mar 28, 10:03 AM

Thanks!

Laurent: I don't see how having to scroll down such a tree is any different from the KMail folder tree :-)
Similar to KMail, I think this means we'll need to move the favorites to the top (though below the "unread" block).

This revision is now accepted and ready to land.Sat, Mar 28, 10:03 AM
olivierjg closed this revision.Sat, Mar 28, 11:31 PM