Move accounts overview to the LHS
AbandonedPublic

Authored by olivierjg on Feb 8 2020, 5:13 AM.

Details

Reviewers
dfaure
mlaurent
Summary

First step towards using an item view for the accounts overview.

Diff Detail

Repository
R865 Ruqola
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 22206
Build 22224: arc lint + arc unit
olivierjg requested review of this revision.Feb 8 2020, 5:13 AM
olivierjg created this revision.
mlaurent requested changes to this revision.Feb 8 2020, 7:55 AM
mlaurent added inline comments.
src/widgets/misc/accountsoverviewwidget.cpp
200

it's not the same commit as removing class no ?

> 2 differents commit

207

accountNumberChanged is not enough ?
it's called when we add/remove account.
I tested when I add/remove accounts.

src/widgets/ruqolacentralwidget.cpp
41

I created a separate class for allowing to test it.
So if you move here you need to readd autotest
Thanks

src/widgets/ruqolamainwindow.cpp
91

So now that you removed mAccountOverviewWidget which class will use it ?

389

Another commit ?

This revision now requires changes to proceed.Feb 8 2020, 7:55 AM
dfaure added inline comments.Feb 8 2020, 9:27 AM
src/widgets/ruqolacentralwidget.cpp
48

Laurent: it's used here.

src/widgets/ruqolamainwindow.cpp
389

I think the idea here (using QPointer) was to fix https://blogs.kde.org/node/3919
With QScopedPointer (or the dialog on the stack, same thing) you can end up double-deleting the dialog.

mlaurent added inline comments.Feb 8 2020, 12:48 PM
src/widgets/ruqolacentralwidget.cpp
48

Oh on top ? Could you create a screenshot please ?
(I prefere on statusbar. why on top ?)

Sorry, but indeed I prefer account in statusbar.
Splitter can be increase when we have account status changed or with different language

regards

dfaure added a comment.Feb 9 2020, 9:31 AM

This looks like it will be jumpy indeed.
Maybe "just icons" like in RC+ is a better solution?
With an overlaid number for unread messages, but also overlay icons for "successfully logged in" and "connection/login failed". Or that part could just use the "disabled" mode of painting icons, possibly.

Is using an accounts overview item view in the top-left acceptable? I really don't want this in the statusbar.

src/widgets/misc/accountsoverviewwidget.cpp
200

Not about removing the class, about putting this widget in the TL

src/widgets/ruqolacentralwidget.cpp
48

I want this to be an item view, not buttons, this is a "usable intermediate state"

src/widgets/ruqolamainwindow.cpp
91

The central widget.

389

Ok, sounds like a job for WA_DeleteOnClose but I'll just minimize changes

389

Well, the removal applies to the accounts overview widget, but I changed too much indeed.

mwolff added a subscriber: mwolff.Feb 20 2020, 1:09 PM

Personally, I would like to get rid of the account list altogether. Rather, I would prefer if all accounts could get merged, similar to konversation.

Account 1
|- channel 1
|- channel 2
Account 2
|- channel 1
|- channel 2

Once we get proper grouping support, this would ideally cover all accounts too. Such that we could get something like:

Unread
|- Account 1
  |- channel 1
|- Account 2
  |- channel 3
Favorites
|- Account 1
  |- Channel 2
...

Once we have a central model for this, it would also be trivial to implement a "quick jump" feature to switch chat rooms across different account instances

I like Milian's idea -- @dfaure @mlaurent are you ok with that?

Yes, I like it too.

With a single account, it should skip the account level and become
Unread

  • channel 1
  • channel 2

Favorites

  • channel 1

(without indentation, so that these look like headers rather than parents)

And indeed with multiple accounts, this would be greatly superior UI compared to RC+ (where I always forgot to have a look at other accounts).

Note that the model already orders together all unread, all favorites, etc. -- so the to-be-written tree model can easily use that information too.

Really I don't like to merge all accounts in one list.
I have 3 differents accounts I don't want to have all in one list.

"Unread

- Account 1
- channel 1
- Account 2
- channel 3

Favorites

- Account 1
- Channel 2

" we will have a lot of duplicate "Account X" so the list will be big.

For example I have an account kdab, an account openrocketchat etc, I don't want that if an user comes behind me he will see all my rooms. I can't hide them if I am all accounts merged.

It will be more difficult to know which rooms comes from with account.
For example in kdab and kdab 'old kdab' account I can have two rooms named "kevin ottens" etc. With current UI we can know that we are in kdab current account and not "old kdab" account etc.

How about we do this more like in KMail ?

Account 1

  • Unread
  • Favorites
  • Channels
  • Private Messages

Account 2

  • Unread
  • Favorites
  • Channels
  • Private Messages

?

This also works better for the single-account use case.

"kmail UI" has a better UI as have a separate channels.

but now we need to define a new UI for changing "status" as we don't want to change status for each account.

For info, each account can have different settings (so we must disable actions when we switch account, we need to update emoji too when switch account etc.)

When we click on "search channel" we need to be sure to be in specific account etc.

"kmail UI" has a better UI as have a separate channels.

but now we need to define a new UI for changing "status" as we don't want to change status for each account.

For info, each account can have different settings (so we must disable actions when we switch account, we need to update emoji too when switch account etc.)

All of that also applies to KMail, Konversation and other tools. There are ways to handle this, e.g. to apply the actions to the account for the currently selected channel.

When we click on "search channel" we need to be sure to be in specific account etc.

Optionally, maybe, but otherwise it would be fine to always search in all accounts by default imo.

olivierjg abandoned this revision.Mar 23 2020, 1:18 AM

Abandoning this UI change in favor of implementing account/channel tree model discussed above. See https://phabricator.kde.org/D28210