nurupo / ProjectTox-Qt-GUI

A cross-platform front end for ProjectTox Core library, written in C++11 with use of Qt5
GNU General Public License v3.0
342 stars 116 forks source link

New friend request widget #60

Open Schlumpf opened 11 years ago

Schlumpf commented 11 years ago

I thought about the friend requests of the mockup. Here is a first idea. I replaced the menu by a new button, which comes visible if a new request arrives. A click on the button shows a popup menu with the client ID, the message and two buttons to accept or reject the request.

The next step would be to extract the status button from the OurUserItemWidget, to get closer to the mockup.

nurupo commented 11 years ago

The next step would be to extract the status button from the OurUserItemWidget, to get closer to the mockup.

I had a talk with the mockup designer about that. Basically, that "Online" button in the mockup is not a button that changes your status, it's a button that changes the way your friend list is sorted. The one that changes your status should somehow appear when you press on the arrow that is right above friend list's scroll bar. cggcsvz

I find it confusing, and as I said before, we don't have to follow the mockup pixel to pixel. For example, the change of message display widget from QTextBrowser-based to QLabel-based, which was done based on the mockup having a custom QWidget, the file transferring widget, in the message display, probably shouldn't have been done. The new implementation creates 3 QLabels (name, message, time) for a single message, which is quite memory costly, and the layout management of those QLabels takes quite a lot of cpu cycles when number of messages grows. You can check it yourself. For memory: just keep Enter pressed in the input widget and have something that displays memory usage of the application opened on a side of the screen. For GUI freezes: spawn 800-1000 messages in a single message display, try switching focus to some other application and making the Tox GUI active again — the GUI will freeze for several seconds, consuming a whole cpu core, because QLayout will be doing some calculations on all of those numerous QLabels. Intense memory consumption and freezing doesn't seem to be a critical issue right now, but it will come up sometime in the future. Also, with QLabel you can't access rich text selected by mouse in order to copy a part of a message with smileys in it (QLabel doesn't expose QTextCursor, I have submitted a feature request to Qt regarding this), but with QTextBrowser you can. QTextBrowser (derives from QTextEdit) shouldn't consume so much memory and freeze the whole GUI thread just because of 1000 messages, it's said to be optimized for having a lot of text, but file transfers will have to be shown in some separate thing, not in the message display.

QTextEdit is an advanced WYSIWYG viewer/editor supporting rich text formatting using HTML-style tags. It is optimized to handle large documents and to respond quickly to user input.

http://qt-project.org/doc/qt-5.0/qtwidgets/qtextedit.html#details

Actually, instead of using QTextBrowser, it would be better to take a look at (check if it supports copying of selected rich text or if it's implementable), and even use, what the free and open source Qt IRC client Quassel came up with for displaying messages. I haven't looked the code yet, but visually it looks like what we want, except it seems to be for text only, it won't accept custom QWidgets for file transferring, just like the QTextBrowser, which, in my opinion is fine, we will have to make a separate thingy where file transfers would be displayed.

We can make mockup-like message display based on something like QTextBrowser which, in contrast to QtextBrowser, supports "custom widgets" (html5 + js), but that would require 30mb QWebKit library as a dependency. I'm neither for or against this idea (neutral) and haven't worked with QWebKit, I don't know how it will work out.

Schlumpf commented 11 years ago

Basically, that "Online" button in the mockup is not a button that changes your status, it's a button that changes the way your friend list is sorted.

That would be really confusing... I agree, it should be changed.

Message display Yes, we have to change the message display. I never tried to add so many messages at once, but you're right. I would prefer to split the message display and the file transfers. A QTextEdit (readonly) for the messages and a seperate widget for file transfers. QTextbrowser with webkit is to much, I think. We don't need the browsing functionality (page back and so on) and another 30 MB for a simple chat client is much, especially for windows users. The most linux users will have the libraries installed anyway. With QTextEdit and a large html-table for the messages inside, it would be easily possible to copy text over the lines, too.

Quassel goes another interesting way: QGraphicsView. Optimized for large amount of items and full control about the way the messages should look like. Sounds nice. And, QGraphicsView would allow us to have the file transfer inside messages view. The only thing I don't know is rich text support, and that could be a problem for us.

nurupo commented 11 years ago

Totally agreed. Also, the only reason I want to use QTextBrowser, which is derived from QTextEdit class, is because it suppors URLs (coloring, underlining. opening in the default browser on click) out of the box. We can make our class derived from QTextEdit and implement support of URLs ourselves, but I don't think it's worth effort when there is QTextBrowser, unless QTextBrowser has a lot of overhead.

I prefer adopting the Quassel message display over using the QTextBrowser, because it offers more functionality. The ability to resize name, message and time columns, ability to prepend messages and already working text search, which actually a little advanced than just a plain text search, and font changing functionality are nice, and using Quassel every day on Windows and Linux I never had any problem with the way message display is done there :)

And, QGraphicsView would allow us to have the file transfer inside messages view.

Do we want file transfer to be shown inside the message view? Oh, wrong question. I mean, would users want the file transfer to be shown inside the message view instead of a separate widget for files?

The only thing I don't know is rich text support, and that could be a problem for us.

Rich text (html) is pretty broad, isn't all we want is just an ability to insert images and change them when different smiley pack is picked? If we want full rich text support, we are likely to end up with QLabels again.

nurupo commented 11 years ago

That would be really confusing... I agree, it should be changed.

In the mockup you can visually see how what I call "Our Own Widget Item" (named after Friend Widget Item. I need to become better at naming things) is different in background color from Friend List, so even if the button changed your status, Friend List is definitely not a place for it, the button should be somewhere inside Our Own Widget Item. That's basically why I asked the designer what that button was doing there, since I didn't know that it was for sorting and assumed it was for status change just as you did.

Schlumpf commented 11 years ago

Yeah, QGraphicsView could be the best option (and we can keep the effects, too). About the rich text, sorry I thought that QGraphicsItem only allow plaintext, but it doesn't. Smilies should work :)

I mean, would users want the file transfer to be shown inside the message view instead of a separate widget for files?

I don't think so. Imagine you're chatting, sending a (larger) file and continue chatting. Than you have to scroll up to see if your download is done? No, I think a seperate file transfer widget is better for the usability.

Our Own Widget Item

OK, I can see. Do we really need such a sorting button? I never used a button like that. If we keep that feature, maybe it could be a context menu action. The button is very large in the mockup and not a "use every day" feature.

By the way: What's about merging the "Our Own Widget Item" and "Friend Widget Item"? They're doing almost the same: display name, status text and avatar. The differences are showing a dropdown menu for status change at "Own Widget" and showing 2 buttons for calling and video at "Friend Widget".

nurupo commented 11 years ago

I don't think so. Imagine you're chatting, sending a (larger) file and continue chatting. Than you have to scroll up to see if your download is done? No, I think a seperate file transfer widget is better for the usability.

Then, do we want a global list of file transfers, or a separate list for every friend? We could make separate lists first, then make a global by just merging all the lists in one later.

OK, I can see. Do we really need such a sorting button? I never used a button like that. If we keep that feature, maybe it could be a context menu action. The button is very large in the mockup and not a "use every day" feature.

Don't think we need it. Personally, I won't ever use it. I'm happy with the way how the sorting is done now. If to describe the end result of the current sorting, you can imagine the friend list being sorted "alphabetically" by name using a local aware comparison first and then sorted by status (value of the Status enum), so that you see Online people sorted alphabetically first, then Away people sorted alphabetically, then the same for Busy and Offline.

By the way: What's about merging the "Our Own Widget Item" and "Friend Widget Item"? They're doing almost the same: display name, status text and avatar. The differences are showing a dropdown menu for status change at "Own Widget" and showing 2 buttons for calling and video at "Friend Widget".

Just noticed that it's actually called OurUserItemWidget. When writing them I thought of making a common base class that they would share, but currently Our User Item Widget and Friend Item Widget are quite different. One simply displays a status in a QLabel, another displays a status in a QToolButton that allows to change the status. Also, while Friend Item Widget uses a QLabel to display a text, Our User Item Widget uses QStackedWidget with QLabel and QLineEdit inside, so that you could show a QLineEdit in place of the QLabel when a user wants to change their name. They are just so different that there is nothing to factor out into a base class or make them a single class.

Schlumpf commented 11 years ago

OK, let's summarize:

So one question is still unanswered: How to display the incoming friend requests?

My first idea was like a hidden toolbutton, what comes visible if a request arrives. A click on the button shows a popup with the request info (this push request). But now, if we drop the sorting button we could write a new widget, which extends if the user clicks on it. Maybe the better solution.

nurupo commented 11 years ago

We write a new messages display, based on QGraphicsView like Quassel.

You mean writing our own message display based on QGraphicsView from scratch? We could just take Quassel's message display and try to integrate it here, make it work for our purpose, since the license allows that.

Current file transfers will go to separate lists (own widgets) for each friend below the message display.

I thought of putting the file transfer list between Friend Item Widget and message display and making it look like some kind of wide button with "File History" on it when there is no transfer going on, or a general file transfer information when something is transferring, and when it's clicked, it would expand down, taking some space away from the message display, and show a list of file transfers. Something like that when the file list is not expanded, although I went a little overboard with the amount of the information shown when a file transfer is occurring:

cggcsvfgdfgfg

cggcsvgdfg

If we put it the way you described, right below the message display, it will be getting in the way between the input widget and message display when users are texting.

Main file transfer list for all users in one (combination of the lists), with history?

Supposedly a separate window that can be opened from the cogwheel menu.

Our Own Widget and Friend Widget keep different widgets.

Correct.

We drop the sorting button from the mockup

Correct.

How to display the incoming friend requests?

What about doing it the way it's done in the mockup (see the two images) but with removed sorting button and friend request taking its space, and with arrows on the friend request in order to navigate from one friend request to another?

Schlumpf commented 11 years ago

I agree in all points.

We could just take Quassel's message display and try to integrate it here, make it work for our purpose, since the license allows that.

I didn't looik into the code so deeply jet, maybe we could...

But one more question. What does the 3. button in the request widget do?

nurupo commented 11 years ago

But one more question. What does the 3. button in the request widget do?

No idea, have to ask the designer. Maybe it adds the person into a black list. We can go with two buttons for now :)

nurupo commented 11 years ago

Wow, that's ugly. At least with native UI looks.

gdfgdfgdfg

The button stretched the layout and dock.

gdfgdfgdfghhh

gdfgdfgdfghhhhh

Current default UI size

gdfgdfgdfggghhhhh

Resized UI to about mockup size

Those Client Id and friend request message are so cramped by buttons above and below, not to mention the width of the dock. If a friend request message will be lengthy, it will take a lot of vertical space from the friend list. Friend request widget looks as if someone squeezed it and forcefully placed it in there. It might be better if all friend requests with their messages were shown somewhere with more space — in place of chat page widget (the right part of the window).

And yeah, it loos like the sooner we start customizing UI — the better, even if it's just little by little.

aitjcize commented 10 years ago

The UI design mockup is very nice!