GNOME Bugzilla – Bug 533638
Topic by expander
Last modified: 2010-04-02 08:59:54 UTC
Hello, I'm trying empathy 0.22.1 for IRC. I'm used to color nicknames and I find difficult to follow a group conversation without it. Something like assigning a random color for a nick that says something and stick with this color for this nick. Sometimes the topic doesn't appears completetely, I think that something like an expander with a text view for the topic would be more useful, the text could handle hypertext as many channels put here references to content on the web. Also the expander label could contain the info from the topic author and date, as this comes from the IRC protocol if it is set, or a message like "No topic set". I hope this help you improve this application.
Created attachment 113816 [details] [review] Use a GtkEntry instead of GtkLabel to display the topic I had a stab at "fixing" the not being able to see the entire topic problem. I've simply changed the topic from a label to an entry. This is what X-Chat does, I think. Please check out the topic-entry branch at git://git.collabora.co.uk/git/user/jonny/empathy.git I've attached the patch for your convenience.
Juan: Could you please open one bug per feature request? I'm splitting this bug, keep the expander conversation here and the nick color at bug #541632. Thanks.
About the patch: That could make the job, but maybe you could set the entry editable so the user can set a new topic? The problem is I don't know how/if we can know if the user has the permission to set the topic. Seems there is nothing in the spec to know that.
Having that entry editable and using it to set the topic is a great idea, but if there's no way of checking whether the user is allowed to do so, it's not really worth thinking about, is it? Nevertheless, my patch successfully fixes *this* bug, does it not? After all, it does now allow viewing the whole topic.. :-)
Created attachment 114096 [details] xchat example
First, thank you for the job with this bug. I think the topic still doesn't appears completely, the user would need to scroll and there is no possibility of make links from urls with this scheme, something possible with a textview, and appreciable. As I stated I think an expander with a textview inside is a good solution, the GUI toolkit provides it, so we can use it. As an example implementation xchat-gnome provides something similar to what I'm talking about. But I don't like the cutted topic with "..." I think that the 'Topic set by "nickauthor" at "timestamp"' scheme (data provided by the irc protocol once you join a channel) is a good use of the irc protocol for the expander label. Also the user can expand or unexpand the topic when needed, it will be always here and not consuming a lot of space.
I agree with Juan that an expander with a textview inside looks better. However I don't agree that the expander label should be the nickname of the guy who set the topic, that information is not important. I like xchat-gnome's way to elipse the topic with "..." and show the full topic if expanded. I lots of channels the topic can fit without the expander anyway.
Created attachment 114328 [details] [review] Pull in xchat-gnome's TopicLabel and use it instead of GtkLabel to display the channel topic. Okay, you got it. I pulled xchat-gnome's TopicLabel widget and plugged it into Empathy. I simply know you're going to love this. The results are a few commits in my "topic-entry" (same name, different meaning) branch at git://git.collabora.co.uk/git/user/jonny/empathy.git I've attached a big patch of the commits merged together.
Created attachment 114337 [details] [review] Remove need for libsexy This additional patch removes the need for libsexy. However, it doesn't quite work properly. The following image attached shows what happens. This patch is in my Git branch.
Created attachment 114338 [details] Topic label not filling.
Thanks for the patch, comments: 1) We don't want to install topic-label.h, it should not be part of the empathy api/abi. Keep it private. So remove the header from libempathy_gtk_headers, it is alreadi in libempathy_gtk_la_SOURCES. 2) Did you copy topic-label.c/h as-is from xchat-gnome? Or did you add some changes? Otherwise it seems OK to me.
(In reply to comment #11) > 1) We don't want to install topic-label.h, it should not be part of the empathy > api/abi. Keep it private. So remove the header from libempathy_gtk_headers, it > is alreadi in libempathy_gtk_la_SOURCES. Oh, yes of course, sorry. This is fixed in the aforementioned branch. > 2) Did you copy topic-label.c/h as-is from xchat-gnome? Or did you add some > changes? Most of it is exactly the same, although I have removed large chunks of it. The original can be found at: http://svn.gnome.org/viewvc/xchat-gnome/trunk/src/fe-gnome/topic-label.c?view=markup There was a fair but of session stuff in the original code which is useless in Empathy. I didn't add much though. > Otherwise it seems OK to me. Cool. What do you think about the libsexy dep? Using a SexyUrlLabel might bring some other advantages.. *shrug* If you don't want to use libsexy though, I'm not sure why the label is showing up like in my previous screenshot..?
why is this bug closed?
I would like to avoid libsexy dep, and if possible I prefer to have a verbatim copy so it's easier to update if there is changes in x-g.
You shouldn't worry too much about that, x-g code didn't change since a while.
Oops, sorry I must have clicked on one of the auto-responders by accident! I'm cool with dropping with libsexy. Any hints on fixing that annoying label problem though?! :-) Correct me if I'm wrong, but I don't think that it's possible to keep an intact copy of the xchat-gnome code due to their session way of doing things, and if you don't want the libsexy dep, then of course a fair bit should change. I doubt their code will change any time soon though; it's not been touched for over fifteen months now.
libsexy should at least be an option build dep as it's not blessed by GNOME. I think it's not a problem to diverge from x-g, they won't change that file anymore. Otherwise the code seems OK, I didn't test it but of course we should sort out that filling problem.
*** Bug 565588 has been marked as a duplicate of this bug. ***
*** Bug 564260 has been marked as a duplicate of this bug. ***
*** Bug 586285 has been marked as a duplicate of this bug. ***
Created attachment 138518 [details] [review] topic_by_expander.patch This is not what we want but it's a start: - expander with a label inside - a label outside the expander (the normal ellipsized) which hides when the expander is expanded and shows when the expander is collapsed I don't know how to set the markup for URLs, if you tell my how I will do it now.
Created attachment 138519 [details] [review] topic_by_expander.patch minor tweaks. As someone was telling, the GtkLabel does not expand automatically. I can't find an option to do this. Even if the GtkExpander is set to expand in the GtkBox, the label is always of the same size
There's another aesthetic problem with my patch: when the expander is un-expanded the hbox size does not automatically shrink to only fit "Topic:" so the label with the topic is displayed starting in the center of the window. So it's worth to keep Jonny's patch. Sorry for wasting your time.
*** Bug 601166 has been marked as a duplicate of this bug. ***
Jonny: could you take care of this bug please? You've done some work on it so you're probably the best to review this patch. thanks!
Strictly speaking, this should be a separate bug, but since you're redesigning the topic appearance, I would add that currently Ctrl+C is not working on it (and it should!). Pietro
*** Bug 609255 has been marked as a duplicate of this bug. ***
I made a proper patch: http://git.collabora.co.uk/?p=user/xclaesse/empathy.git;a=shortlog;h=refs/heads/expand-topic It is better than xchat-gnome's topic label. xg's SexyUrlLabel request too much height when the topic is expanded, and it is using an hidden label that does not wrap/ellipsize just to know if the expander must be shown. My branch fix all this. Note that my branch is based on the one for bug #525576 to have clickable links in the topic too.
I didn't review the branch but that's something that should wait 2.31 as we are now in UI freeze.
Not sure if it's related to your branch but I got this crash while selecting the topic with the mouse:
+ Trace 220866
Already fixed, I forgot to rebase my branch on master. Now I did, just pull ;-)
Looks good. Code is a bit gruik but seems to work fine.
Branch pushed to master, will be fixed for 2.32/3.0.